Danielle/482 notifications needs a way to exit the modal #609

Danielle254 · 2024-10-11T19:20:23Z

Before

Changed background color to muted for better contrast
Added button to manually close the alert before it expires
Button is styled with ghost and standard icon size
Close button is accessible by keyboard and labeled

…alert

… close button

appwrite · 2024-10-11T19:20:27Z

Gridiron Survivor Application 6616ea581ef9f5521c7d

Function	ID	Status	Action
Your function deployment has failed. Please check the logs for more details and retry.

Project name: Gridiron Survivor Application
Project ID: 6616ea581ef9f5521c7d

Function	ID	Status	Action
userAuth	`6626fef885a9f630442b`	Failed	View Logs

Only deployments on the production branch are activated automatically. If you'd like to activate this deployment, navigate to your deployments. Learn more about Appwrite Function deployments.

💡 Did you know?
You can use Avatars API to generate QR code for any text or URLs

vercel · 2024-10-11T19:20:28Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
gridiron-survivor	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 16, 2024 4:26pm
gridiron-survivor-storybook	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 16, 2024 4:26pm

…n-survivor into danielle/482-notifications-needs-a-way-to-exit-the-modal

components/AlertNotification/AlertNotification.tsx

…n-survivor into danielle/482-notifications-needs-a-way-to-exit-the-modal

…d syntax (label prop)

…tton

shashilo · 2024-10-14T17:09:03Z

Remove the orange hover background. Just make the icon color 50% lighter on hover.

shashilo · 2024-10-14T17:09:50Z

components/Alert/Alert.tsx

@@ -5,17 +5,17 @@ import * as React from 'react';
 import { cva, type VariantProps } from 'class-variance-authority';

 const alertVariants = cva(
-  'relative w-full rounded-lg border p-4 [&>svg~*]:pl-7 [&>svg+div]:translate-y-[-3px] [&>svg]:absolute [&>svg]:left-4 [&>svg]:top-4 [&>svg]:text-foreground',
+  'relative w-full rounded-lg border p-4 [&>svg~*]:ml-7 [&>svg+div]:translate-y-[-3px] [&>svg]:absolute [&>svg]:left-4 [&>svg]:top-4 [&>svg]:text-foreground',


How come these styles were updated? This ticket only pertains towards the close button.

@shashilo this styling was causing the button to be rendered distorted. I could not find another workaround.

@shashilo this is now fixed

components/AlertNotification/AlertNotification.test.tsx

components/AlertNotification/AlertNotification.tsx

shashilo · 2024-10-14T17:20:34Z

components/AlertNotification/AlertNotification.test.tsx

+      );
+      const dismissButton = screen.getByTestId('dismiss-alert-btn');      
+      fireEvent.click(dismissButton);      
+      expect(spyToast).toHaveBeenCalled();


great job on the spy!

ryandotfurrer

Braydon and Shashi covered my other comments, so I've just left a couple of nits. Once the other requested changes from Braydon and Shashi are taken care of, I will approve this PR regardless of the status of my "nits".

components/AlertNotification/AlertNotification.test.tsx

components/AlertNotification/AlertNotification.tsx

Danielle254 · 2024-10-15T14:14:09Z

Remove the orange hover background. Just make the icon color 50% lighter on hover.

@shashilo this would involve creating a new button style rather than using an existing ("ghost") style. Are you OK with that?

ryandotfurrer · 2024-10-15T16:17:31Z

Remove the orange hover background. Just make the icon color 50% lighter on hover.

@shashilo this would involve creating a new button style rather than using an existing ("ghost") style. Are you OK with that?

@Danielle254 the buttons can accept additional classNames where defined.

Since this is only being used in one place (currently) I'd just adjust the button itself. Or if @shashilo prefers, we can edit the icon variety.

…n-survivor into danielle/482-notifications-needs-a-way-to-exit-the-modal

…dding left on svg inside button

components/AlertNotification/AlertNotification.test.tsx

ryandotfurrer

LGTM 🚀

Danielle254 added 5 commits October 11, 2024 08:01

update background color from primary to muted for better contrast on …

a550d65

…alert

add button component to alert

3d2c998

fix - change padding left to margin left because it was impacting the…

722d4a3

… close button

fix - alert disappears on clicking the close button

9008d93

fix - add accessibility for close button

186b408

Danielle254 requested review from shashilo and mhchen as code owners October 11, 2024 19:20

Danielle254 added 2 commits October 11, 2024 13:25

fix - eslint error. remove autoFocus property

cb4603a

Merge branch 'develop' of https://github.com/LetsGetTechnical/gridiro…

da2345e

…n-survivor into danielle/482-notifications-needs-a-way-to-exit-the-modal

vercel bot deployed to Preview – gridiron-survivor-storybook October 11, 2024 19:26 View deployment

vercel bot deployed to Preview – gridiron-survivor October 11, 2024 19:27 View deployment

Danielle254 requested review from ryandotfurrer and alexappleget October 11, 2024 19:27

braydoncoyer reviewed Oct 12, 2024

View reviewed changes

components/AlertNotification/AlertNotification.tsx Show resolved Hide resolved

braydoncoyer mentioned this pull request Oct 12, 2024

Cody/refine toast styling #607

Merged

Danielle254 added 2 commits October 13, 2024 07:47

Merge branch 'develop' of https://github.com/LetsGetTechnical/gridiro…

402bb33

…n-survivor into danielle/482-notifications-needs-a-way-to-exit-the-modal

fix: udpate Button component structure to match previously establishe…

ed97bfe

…d syntax (label prop)

vercel bot deployed to Preview – gridiron-survivor-storybook October 13, 2024 14:22 View deployment

vercel bot deployed to Preview – gridiron-survivor October 13, 2024 14:23 View deployment

Danielle254 added 2 commits October 13, 2024 08:46

fix: address TypeScript error and begin drafting tests for dismiss bu…

0bb4542

…tton

fix: update aria-label to be more descriptive

d5ce353

vercel bot deployed to Preview – gridiron-survivor October 13, 2024 14:48 View deployment

vercel bot deployed to Preview – gridiron-survivor-storybook October 13, 2024 14:49 View deployment

fix: test that dismiss button renders for each type of alert

03aa273

vercel bot deployed to Preview – gridiron-survivor-storybook October 13, 2024 15:19 View deployment

vercel bot deployed to Preview – gridiron-survivor October 13, 2024 15:19 View deployment

fix: make test for button render its own separate test

58a1853

vercel bot deployed to Preview – gridiron-survivor-storybook October 13, 2024 17:31 View deployment

vercel bot deployed to Preview – gridiron-survivor-storybook October 14, 2024 17:07 View deployment

shashilo reviewed Oct 14, 2024

View reviewed changes

ryandotfurrer requested changes Oct 14, 2024

View reviewed changes

components/AlertNotification/AlertNotification.test.tsx Show resolved Hide resolved

components/AlertNotification/AlertNotification.tsx Show resolved Hide resolved

vercel bot deployed to Preview – gridiron-survivor-storybook October 15, 2024 16:24 View deployment

vercel bot deployed to Preview – gridiron-survivor October 15, 2024 16:25 View deployment

Danielle254 added 2 commits October 16, 2024 07:55

fix: sort imports alphabetically

deb61d1

Merge branch 'develop' of https://github.com/LetsGetTechnical/gridiro…

7453829

…n-survivor into danielle/482-notifications-needs-a-way-to-exit-the-modal

Danielle254 force-pushed the danielle/482-notifications-needs-a-way-to-exit-the-modal branch from 143ba37 to 7453829 Compare October 16, 2024 13:58

vercel bot deployed to Preview – gridiron-survivor-storybook October 16, 2024 13:59 View deployment

vercel bot deployed to Preview – gridiron-survivor October 16, 2024 14:00 View deployment

fix: update hover styling for button as requested in PR 609

4b93b1c

vercel bot deployed to Preview – gridiron-survivor-storybook October 16, 2024 14:18 View deployment

vercel bot deployed to Preview – gridiron-survivor October 16, 2024 14:20 View deployment

fix: undo change to Alert styling and accommodate needed change to pa…

cb82b81

…dding left on svg inside button

vercel bot deployed to Preview – gridiron-survivor October 16, 2024 15:04 View deployment

vercel bot deployed to Preview – gridiron-survivor-storybook October 16, 2024 15:05 View deployment

shashilo requested changes Oct 16, 2024

View reviewed changes

components/AlertNotification/AlertNotification.test.tsx Outdated Show resolved Hide resolved

shashilo requested review from braydoncoyer, shashilo and ryandotfurrer October 16, 2024 15:53

fix: change 'test' to 'it' in unit test

5181c17

vercel bot deployed to Preview – gridiron-survivor October 16, 2024 16:25 View deployment

vercel bot deployed to Preview – gridiron-survivor-storybook October 16, 2024 16:26 View deployment

ryandotfurrer approved these changes Oct 16, 2024

View reviewed changes

shashilo approved these changes Oct 16, 2024

View reviewed changes

Danielle254 merged commit 6bc5b39 into develop Oct 16, 2024
5 checks passed

Danielle254 deleted the danielle/482-notifications-needs-a-way-to-exit-the-modal branch October 16, 2024 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Danielle/482 notifications needs a way to exit the modal #609

Danielle/482 notifications needs a way to exit the modal #609

Danielle254 commented Oct 11, 2024 •

edited

Loading

appwrite bot commented Oct 11, 2024 •

edited

Loading

vercel bot commented Oct 11, 2024 •

edited

Loading

shashilo commented Oct 14, 2024 •

edited

Loading

shashilo Oct 14, 2024

Danielle254 Oct 15, 2024

Danielle254 Oct 16, 2024

shashilo Oct 14, 2024

ryandotfurrer left a comment

Danielle254 commented Oct 15, 2024

ryandotfurrer commented Oct 15, 2024

ryandotfurrer left a comment

Danielle/482 notifications needs a way to exit the modal #609

Danielle/482 notifications needs a way to exit the modal #609

Conversation

Danielle254 commented Oct 11, 2024 • edited Loading

appwrite bot commented Oct 11, 2024 • edited Loading

vercel bot commented Oct 11, 2024 • edited Loading

shashilo commented Oct 14, 2024 • edited Loading

shashilo Oct 14, 2024

Choose a reason for hiding this comment

Danielle254 Oct 15, 2024

Choose a reason for hiding this comment

Danielle254 Oct 16, 2024

Choose a reason for hiding this comment

shashilo Oct 14, 2024

Choose a reason for hiding this comment

ryandotfurrer left a comment

Choose a reason for hiding this comment

Danielle254 commented Oct 15, 2024

ryandotfurrer commented Oct 15, 2024

ryandotfurrer left a comment

Choose a reason for hiding this comment

Danielle254 commented Oct 11, 2024 •

edited

Loading

appwrite bot commented Oct 11, 2024 •

edited

Loading

vercel bot commented Oct 11, 2024 •

edited

Loading

shashilo commented Oct 14, 2024 •

edited

Loading