From a550d65d67794ef0b2e3bac77a83fbc2c355e8d3 Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Fri, 11 Oct 2024 08:01:42 -0600 Subject: [PATCH 01/17] update background color from primary to muted for better contrast on alert --- components/Alert/Alert.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/Alert/Alert.tsx b/components/Alert/Alert.tsx index 80fc809f..fff2d4b8 100644 --- a/components/Alert/Alert.tsx +++ b/components/Alert/Alert.tsx @@ -9,13 +9,13 @@ const alertVariants = cva( { variants: { variant: { - default: 'bg-background text-foreground', + default: 'bg-muted text-muted-foreground', error: - 'bg-background text-error [&>svg]:text-error text-error border-error', + 'bg-muted text-error [&>svg]:text-error text-error border-error', warning: - 'bg-background text-warning [&>svg]:text-warning text-warning border-warning', + 'bg-muted text-warning [&>svg]:text-warning text-warning border-warning', success: - 'bg-background text-success [&>svg]:text-success text-success border-success', + 'bg-muted text-success [&>svg]:text-success text-success border-success', }, }, defaultVariants: { From 3d2c998fe31a3812082c2ae2eb085e829f26d209 Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Fri, 11 Oct 2024 12:06:49 -0600 Subject: [PATCH 02/17] add button component to alert --- components/AlertNotification/AlertNotification.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/components/AlertNotification/AlertNotification.tsx b/components/AlertNotification/AlertNotification.tsx index fb07b49b..330936ae 100644 --- a/components/AlertNotification/AlertNotification.tsx +++ b/components/AlertNotification/AlertNotification.tsx @@ -5,9 +5,10 @@ import { Alert as AlertDefault } from '../Alert/Alert'; import { AlertTitle } from '../AlertTItle/AlertTitle'; import { AlertDescription } from '../AlertDescription/AlertDescription'; import { JSX } from 'react'; -import { CheckCircle, XCircle, Info, AlertTriangle } from 'lucide-react'; +import { CheckCircle, XCircle, Info, AlertTriangle, X } from 'lucide-react'; import { IAlertNotification } from './AlertNotification.interface'; import { AlertVariants } from './Alerts.enum'; +import { Button } from '../Button/Button'; const variantConfig = { success: { @@ -46,6 +47,14 @@ const Alert = ({ + ); }; From 722d4a399c72c458714b1a44e133d6760502e712 Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Fri, 11 Oct 2024 12:23:57 -0600 Subject: [PATCH 03/17] fix - change padding left to margin left because it was impacting the close button --- components/Alert/Alert.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/Alert/Alert.tsx b/components/Alert/Alert.tsx index fff2d4b8..8b31f7f4 100644 --- a/components/Alert/Alert.tsx +++ b/components/Alert/Alert.tsx @@ -5,7 +5,7 @@ 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', { variants: { variant: { From 9008d934078b28ed23ad6d1b8321081ac78dd9a3 Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Fri, 11 Oct 2024 13:11:08 -0600 Subject: [PATCH 04/17] fix - alert disappears on clicking the close button --- components/AlertNotification/AlertNotification.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/AlertNotification/AlertNotification.tsx b/components/AlertNotification/AlertNotification.tsx index 330936ae..15972b59 100644 --- a/components/AlertNotification/AlertNotification.tsx +++ b/components/AlertNotification/AlertNotification.tsx @@ -9,6 +9,7 @@ import { CheckCircle, XCircle, Info, AlertTriangle, X } from 'lucide-react'; import { IAlertNotification } from './AlertNotification.interface'; import { AlertVariants } from './Alerts.enum'; import { Button } from '../Button/Button'; +import toast from 'react-hot-toast'; const variantConfig = { success: { @@ -52,6 +53,7 @@ const Alert = ({ size="icon" className="absolute right-4 top-2" data-testid="dismiss-alert-btn" + onClick={() => toast.remove()} > From 186b408a7fdbc2bf68e051946dcf264a71e257f1 Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Fri, 11 Oct 2024 13:17:05 -0600 Subject: [PATCH 05/17] fix - add accessibility for close button --- components/AlertNotification/AlertNotification.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/AlertNotification/AlertNotification.tsx b/components/AlertNotification/AlertNotification.tsx index 15972b59..d9fe4068 100644 --- a/components/AlertNotification/AlertNotification.tsx +++ b/components/AlertNotification/AlertNotification.tsx @@ -54,6 +54,8 @@ const Alert = ({ className="absolute right-4 top-2" data-testid="dismiss-alert-btn" onClick={() => toast.remove()} + autoFocus + aria-label='close notification' > From cb4603ad4cb58896f8d2e7e69a4ebd25a0a53546 Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Fri, 11 Oct 2024 13:25:13 -0600 Subject: [PATCH 06/17] fix - eslint error. remove autoFocus property --- components/AlertNotification/AlertNotification.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/components/AlertNotification/AlertNotification.tsx b/components/AlertNotification/AlertNotification.tsx index d9fe4068..e8f34602 100644 --- a/components/AlertNotification/AlertNotification.tsx +++ b/components/AlertNotification/AlertNotification.tsx @@ -54,7 +54,6 @@ const Alert = ({ className="absolute right-4 top-2" data-testid="dismiss-alert-btn" onClick={() => toast.remove()} - autoFocus aria-label='close notification' > From ed97bfe94d07492fe4de511aa5bb1e8c2732a4d9 Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Sun, 13 Oct 2024 08:21:28 -0600 Subject: [PATCH 07/17] fix: udpate Button component structure to match previously established syntax (label prop) --- components/AlertNotification/AlertNotification.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/components/AlertNotification/AlertNotification.tsx b/components/AlertNotification/AlertNotification.tsx index e8f34602..885993ec 100644 --- a/components/AlertNotification/AlertNotification.tsx +++ b/components/AlertNotification/AlertNotification.tsx @@ -55,9 +55,8 @@ const Alert = ({ data-testid="dismiss-alert-btn" onClick={() => toast.remove()} aria-label='close notification' - > - - + label={} + /> ); }; From 0bb4542872bea56ae6a1529406460d16f8d86bdc Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Sun, 13 Oct 2024 08:46:34 -0600 Subject: [PATCH 08/17] fix: address TypeScript error and begin drafting tests for dismiss button --- .../AlertNotification.test.tsx | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/components/AlertNotification/AlertNotification.test.tsx b/components/AlertNotification/AlertNotification.test.tsx index 0d307594..b8f2f2b9 100644 --- a/components/AlertNotification/AlertNotification.test.tsx +++ b/components/AlertNotification/AlertNotification.test.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { render } from '@testing-library/react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import Alert from './AlertNotification'; import { AlertVariants } from './Alerts.enum'; import { CheckCircle, XCircle, Info, AlertTriangle } from 'lucide-react'; @@ -30,7 +30,28 @@ const variantTestCases = { describe('AlertNotification', () => { for (const [key, value] of Object.entries(variantTestCases)) { it(`renders the correct variant ${key}`, () => { - render(); + render( + , + ); }); - } + }; + + it('should render the dismiss button', () => { + // do I need to render the Alert first? + const dismissButton = screen.getByTestId('dismiss-alert-btn'); + expect(dismissButton).toBeInTheDocument(); + }); + + it('should close the notification upon clicking the dismiss button', + // do I need to render the Alert first? + async () => { + const dismissButton = screen.getByTestId('dismiss-alert-btn'); + fireEvent.click(dismissButton); + await waitFor(() => { + expect(dismissButton.getAttribute('data-state')).toBe('closed'); // how do I tell if Alert is present or not? + }); + }) }); From d5ce353e6c3835861ec7fe873602aaa9d6f77b92 Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Sun, 13 Oct 2024 08:46:56 -0600 Subject: [PATCH 09/17] fix: update aria-label to be more descriptive --- components/AlertNotification/AlertNotification.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/AlertNotification/AlertNotification.tsx b/components/AlertNotification/AlertNotification.tsx index 885993ec..49050cdc 100644 --- a/components/AlertNotification/AlertNotification.tsx +++ b/components/AlertNotification/AlertNotification.tsx @@ -54,7 +54,7 @@ const Alert = ({ className="absolute right-4 top-2" data-testid="dismiss-alert-btn" onClick={() => toast.remove()} - aria-label='close notification' + aria-label='dismiss notification' label={} /> From 03aa273aa9812c37b8ce0259dd9f7687903a546f Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Sun, 13 Oct 2024 09:18:50 -0600 Subject: [PATCH 10/17] fix: test that dismiss button renders for each type of alert --- .../AlertNotification.test.tsx | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/components/AlertNotification/AlertNotification.test.tsx b/components/AlertNotification/AlertNotification.test.tsx index b8f2f2b9..bef50837 100644 --- a/components/AlertNotification/AlertNotification.test.tsx +++ b/components/AlertNotification/AlertNotification.test.tsx @@ -29,29 +29,30 @@ const variantTestCases = { describe('AlertNotification', () => { for (const [key, value] of Object.entries(variantTestCases)) { - it(`renders the correct variant ${key}`, () => { + it(`renders the correct variant ${key} and a dismiss button`, () => { render( , ); + const dismissButton = screen.getByTestId('dismiss-alert-btn'); + expect(dismissButton).toBeInTheDocument(); }); }; - it('should render the dismiss button', () => { - // do I need to render the Alert first? - const dismissButton = screen.getByTestId('dismiss-alert-btn'); - expect(dismissButton).toBeInTheDocument(); - }); - - it('should close the notification upon clicking the dismiss button', - // do I need to render the Alert first? + /* it('should close the notification upon clicking the dismiss button', async () => { + render( + + ); const dismissButton = screen.getByTestId('dismiss-alert-btn'); fireEvent.click(dismissButton); await waitFor(() => { - expect(dismissButton.getAttribute('data-state')).toBe('closed'); // how do I tell if Alert is present or not? + expect(dismissButton).not.toBeInTheDocument(); }); - }) + }); */ }); From 58a1853b8f26c65f12da5ece531128952f520976 Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Sun, 13 Oct 2024 11:30:52 -0600 Subject: [PATCH 11/17] fix: make test for button render its own separate test --- .../AlertNotification.test.tsx | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/components/AlertNotification/AlertNotification.test.tsx b/components/AlertNotification/AlertNotification.test.tsx index bef50837..b1c07322 100644 --- a/components/AlertNotification/AlertNotification.test.tsx +++ b/components/AlertNotification/AlertNotification.test.tsx @@ -29,30 +29,27 @@ const variantTestCases = { describe('AlertNotification', () => { for (const [key, value] of Object.entries(variantTestCases)) { - it(`renders the correct variant ${key} and a dismiss button`, () => { + it(`renders the correct variant ${key}`, () => { render( , ); - const dismissButton = screen.getByTestId('dismiss-alert-btn'); - expect(dismissButton).toBeInTheDocument(); }); }; - /* it('should close the notification upon clicking the dismiss button', - async () => { + for (const [key, value] of Object.entries(variantTestCases)) { + it('should render the dismiss button on each alert type', () => { render( + variant={AlertVariants[key as keyof typeof AlertVariants]} + message={value.message} + />, ); const dismissButton = screen.getByTestId('dismiss-alert-btn'); - fireEvent.click(dismissButton); - await waitFor(() => { - expect(dismissButton).not.toBeInTheDocument(); - }); - }); */ + expect(dismissButton).toBeInTheDocument(); + }); + }; + }); From 1a86e1c4fbcca182bf483b044de8001a88db0614 Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Sun, 13 Oct 2024 11:36:52 -0600 Subject: [PATCH 12/17] fix: remove unneeded imports --- components/AlertNotification/AlertNotification.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/AlertNotification/AlertNotification.test.tsx b/components/AlertNotification/AlertNotification.test.tsx index b1c07322..1663dff1 100644 --- a/components/AlertNotification/AlertNotification.test.tsx +++ b/components/AlertNotification/AlertNotification.test.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { render, screen} from '@testing-library/react'; import Alert from './AlertNotification'; import { AlertVariants } from './Alerts.enum'; import { CheckCircle, XCircle, Info, AlertTriangle } from 'lucide-react'; From d254e7f55d5a549a6d5a08a7839054d4e1f854ff Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Sun, 13 Oct 2024 15:05:15 -0600 Subject: [PATCH 13/17] fix: nested all tests inside of for loop to run each test for all possible variants of alerts --- .../AlertNotification.test.tsx | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/components/AlertNotification/AlertNotification.test.tsx b/components/AlertNotification/AlertNotification.test.tsx index 1663dff1..738552ff 100644 --- a/components/AlertNotification/AlertNotification.test.tsx +++ b/components/AlertNotification/AlertNotification.test.tsx @@ -1,8 +1,9 @@ import React from 'react'; -import { render, screen} from '@testing-library/react'; +import { render, screen, fireEvent} from '@testing-library/react'; import Alert from './AlertNotification'; import { AlertVariants } from './Alerts.enum'; import { CheckCircle, XCircle, Info, AlertTriangle } from 'lucide-react'; +import toast from 'react-hot-toast'; const variantTestCases = { [AlertVariants.Success]: { @@ -37,9 +38,7 @@ describe('AlertNotification', () => { />, ); }); - }; - for (const [key, value] of Object.entries(variantTestCases)) { it('should render the dismiss button on each alert type', () => { render( { const dismissButton = screen.getByTestId('dismiss-alert-btn'); expect(dismissButton).toBeInTheDocument(); }); - }; + test('should fire the toast.remove() function when dismiss button is clicked', async () => { + const spyToast = jest.spyOn(toast, 'remove'); + render( + , + ); + const dismissButton = screen.getByTestId('dismiss-alert-btn'); + fireEvent.click(dismissButton); + expect(spyToast).toHaveBeenCalled(); + }); + }; }); From deb61d1094a401854c9761ea9ed5b0f2345d1275 Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Wed, 16 Oct 2024 07:55:14 -0600 Subject: [PATCH 14/17] fix: sort imports alphabetically --- components/AlertNotification/AlertNotification.test.tsx | 4 ++-- components/AlertNotification/AlertNotification.tsx | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/components/AlertNotification/AlertNotification.test.tsx b/components/AlertNotification/AlertNotification.test.tsx index 738552ff..b400ced9 100644 --- a/components/AlertNotification/AlertNotification.test.tsx +++ b/components/AlertNotification/AlertNotification.test.tsx @@ -1,8 +1,8 @@ -import React from 'react'; -import { render, screen, fireEvent} from '@testing-library/react'; import Alert from './AlertNotification'; import { AlertVariants } from './Alerts.enum'; import { CheckCircle, XCircle, Info, AlertTriangle } from 'lucide-react'; +import React from 'react'; +import { render, screen, fireEvent} from '@testing-library/react'; import toast from 'react-hot-toast'; const variantTestCases = { diff --git a/components/AlertNotification/AlertNotification.tsx b/components/AlertNotification/AlertNotification.tsx index 49050cdc..39199422 100644 --- a/components/AlertNotification/AlertNotification.tsx +++ b/components/AlertNotification/AlertNotification.tsx @@ -2,13 +2,13 @@ // Licensed under the MIT License. import { Alert as AlertDefault } from '../Alert/Alert'; -import { AlertTitle } from '../AlertTItle/AlertTitle'; import { AlertDescription } from '../AlertDescription/AlertDescription'; -import { JSX } from 'react'; -import { CheckCircle, XCircle, Info, AlertTriangle, X } from 'lucide-react'; -import { IAlertNotification } from './AlertNotification.interface'; +import { AlertTitle } from '../AlertTItle/AlertTitle'; import { AlertVariants } from './Alerts.enum'; import { Button } from '../Button/Button'; +import { CheckCircle, XCircle, Info, AlertTriangle, X } from 'lucide-react'; +import { IAlertNotification } from './AlertNotification.interface'; +import { JSX } from 'react'; import toast from 'react-hot-toast'; const variantConfig = { From 4b93b1c11632b957b6b9d90a3af3280ed58f718d Mon Sep 17 00:00:00 2001 From: Danielle Lindblom Date: Wed, 16 Oct 2024 08:18:04 -0600 Subject: [PATCH 15/17] fix: update hover styling for button as requested in PR 609 --- components/AlertNotification/AlertNotification.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/AlertNotification/AlertNotification.tsx b/components/AlertNotification/AlertNotification.tsx index 39199422..7092759f 100644 --- a/components/AlertNotification/AlertNotification.tsx +++ b/components/AlertNotification/AlertNotification.tsx @@ -49,9 +49,9 @@ const Alert = ({