From 07728208244044272b8d45576cd0a33af69da2bc Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sat, 30 Mar 2024 10:04:31 +0100 Subject: [PATCH 01/11] Ensure all matching notifications are cleared rather than just the first one, and use atomic state updates --- .../manager-api/src/modules/notifications.ts | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/code/lib/manager-api/src/modules/notifications.ts b/code/lib/manager-api/src/modules/notifications.ts index 3358dd67a241..0196fb1fcbc6 100644 --- a/code/lib/manager-api/src/modules/notifications.ts +++ b/code/lib/manager-api/src/modules/notifications.ts @@ -1,4 +1,5 @@ import type { API_Notification } from '@storybook/types'; +import partition from 'lodash/partition.js'; import type { ModuleFn } from '../lib/types'; export interface SubState { @@ -25,26 +26,24 @@ export interface SubAPI { export const init: ModuleFn = ({ store }) => { const api: SubAPI = { - addNotification: (notification) => { - // Get rid of it if already exists - api.clearNotification(notification.id); - - const { notifications } = store.getState(); - - store.setState({ notifications: [...notifications, notification] }); + addNotification: (newNotification) => { + store.setState(({ notifications }) => { + const [existing, others] = partition(notifications, (n) => n.id === newNotification.id); + existing.forEach((notification) => { + if (notification.onClear) notification.onClear({ dismissed: false }); + }); + return { notifications: [...others, newNotification] }; + }); }, - clearNotification: (id) => { - const { notifications } = store.getState(); - - const notification = notifications.find((n) => n.id === id); - - if (notification) { - store.setState({ notifications: notifications.filter((n) => n.id !== id) }); - if (notification.onClear) { - notification.onClear({ dismissed: false }); - } - } + clearNotification: (notificationId) => { + store.setState(({ notifications }) => { + const [matching, others] = partition(notifications, (n) => n.id === notificationId); + matching.forEach((notification) => { + if (notification.onClear) notification.onClear({ dismissed: false }); + }); + return { notifications: others }; + }); }, }; From 123774cab7c29f909fe922eaa31d91110d2696d5 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sat, 30 Mar 2024 10:37:18 +0100 Subject: [PATCH 02/11] Improve NotificationItem stories --- .../NotificationItem.stories.tsx | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx b/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx index 5d58559b465b..dd9b351f222a 100644 --- a/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx +++ b/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx @@ -7,6 +7,7 @@ import { BookIcon as BookIconIcon, FaceHappyIcon, } from '@storybook/icons'; +import { fn } from '@storybook/test'; const meta = { component: NotificationItem, @@ -29,8 +30,8 @@ const meta = { export default meta; type Story = StoryObj; -const onClear = () => {}; -const onDismissNotification = () => {}; +const onClear = fn(); +const onDismissNotification = fn(); export const Simple: Story = { args: { @@ -40,7 +41,7 @@ export const Simple: Story = { content: { headline: 'Storybook cool!', }, - link: '/some/path', + link: undefined, }, onDismissNotification, }, @@ -55,7 +56,7 @@ export const LongHeadline: Story = { content: { headline: 'This is a long message that extends over two lines!', }, - link: '/some/path', + link: undefined, }, }, }; @@ -115,7 +116,7 @@ export const BookIcon: Story = { headline: 'Storybook has a book icon!', }, icon: , - link: '/some/path', + link: undefined, }, }, }; @@ -131,7 +132,7 @@ export const StrongSubHeadline: Story = { subHeadline: Strong subHeadline, }, icon: , - link: '/some/path', + link: undefined, }, }, }; @@ -151,7 +152,7 @@ export const StrongEmphasizedSubHeadline: Story = { ), }, icon: , - link: '/some/path', + link: undefined, }, }, }; @@ -167,7 +168,7 @@ export const BookIconSubHeadline: Story = { subHeadline: 'Find out more!', }, icon: , - link: '/some/path', + link: undefined, }, }, }; @@ -184,7 +185,7 @@ export const BookIconLongSubHeadline: Story = { 'Find out more! by clicking on on buttons and downloading some applications. Find out more! by clicking on buttons and downloading some applications', }, icon: , - link: '/some/path', + link: undefined, }, }, }; @@ -200,7 +201,7 @@ export const AccessibilityIcon: Story = { subHeadline: 'It is here!', }, icon: , - link: '/some/path', + link: undefined, }, }, }; @@ -216,7 +217,7 @@ export const AccessibilityGoldIcon: Story = { subHeadline: 'It is gold!', }, icon: , - link: '/some/path', + link: undefined, }, }, }; @@ -231,7 +232,7 @@ export const AccessibilityGoldIconLongHeadLineNoSubHeadline: Story = { headline: 'Storybook notifications has a accessibility icon it can be any color!', }, icon: , - link: '/some/path', + link: undefined, }, }, }; @@ -249,7 +250,7 @@ export const WithOldIconFormat: Story = { name: 'accessibility', color: 'gold', }, - link: '/some/path', + link: undefined, }, }, }; From e84cabef31b49a90e436be4b1c4edce73ff18dfe Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sat, 30 Mar 2024 14:24:34 +0100 Subject: [PATCH 03/11] Add duration property to Notification API, and show countdown bar --- code/lib/types/src/modules/api.ts | 9 +- .../NotificationItem.stories.tsx | 15 +++- .../notifications/NotificationItem.tsx | 85 +++++++++++++------ 3 files changed, 82 insertions(+), 27 deletions(-) diff --git a/code/lib/types/src/modules/api.ts b/code/lib/types/src/modules/api.ts index 1ccc98cc9e24..4c6893d35ad4 100644 --- a/code/lib/types/src/modules/api.ts +++ b/code/lib/types/src/modules/api.ts @@ -115,9 +115,13 @@ export interface API_SidebarOptions { interface OnClearOptions { /** - * True when the user dismissed the notification. + * True when the user manually dismissed the notification. */ dismissed: boolean; + /** + * True when the notification timed out after the set duration. + */ + timeout: boolean; } /** @@ -130,11 +134,12 @@ interface DeprecatedIconType { } export interface API_Notification { id: string; - link: string; content: { headline: string; subHeadline?: string | any; }; + duration?: number; + link?: string; // TODO: Remove DeprecatedIconType in 9.0 icon?: React.ReactNode | DeprecatedIconType; onClear?: (options: OnClearOptions) => void; diff --git a/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx b/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx index dd9b351f222a..210623ee6c8c 100644 --- a/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx +++ b/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx @@ -41,7 +41,20 @@ export const Simple: Story = { content: { headline: 'Storybook cool!', }, - link: undefined, + }, + onDismissNotification, + }, +}; + +export const Timeout: Story = { + args: { + notification: { + id: '1', + onClear, + content: { + headline: 'Storybook cool!', + }, + duration: 5000, }, onDismissNotification, }, diff --git a/code/ui/manager/src/components/notifications/NotificationItem.tsx b/code/ui/manager/src/components/notifications/NotificationItem.tsx index 0028f87749cf..20d736bd32bb 100644 --- a/code/ui/manager/src/components/notifications/NotificationItem.tsx +++ b/code/ui/manager/src/components/notifications/NotificationItem.tsx @@ -1,26 +1,51 @@ import type { FC, SyntheticEvent } from 'react'; -import React from 'react'; +import React, { useCallback, useEffect, useRef } from 'react'; import { type State } from '@storybook/manager-api'; import { Link } from '@storybook/router'; -import { styled, useTheme } from '@storybook/theming'; +import { keyframes, styled, useTheme } from '@storybook/theming'; import type { IconsProps } from '@storybook/components'; import { IconButton, Icons } from '@storybook/components'; import { transparentize } from 'polished'; import { CloseAltIcon } from '@storybook/icons'; -const Notification = styled.div(({ theme }) => ({ - position: 'relative', - display: 'flex', - padding: 15, - width: 280, - borderRadius: 4, - alignItems: 'center', +const grow = keyframes({ + '0%': { + width: '0%', + }, + '100%': { + width: '100%', + }, +}); - background: theme.base === 'light' ? 'hsla(203, 50%, 20%, .97)' : 'hsla(203, 30%, 95%, .97)', - boxShadow: `0 2px 5px 0 rgba(0,0,0,0.05), 0 5px 15px 0 rgba(0,0,0,0.1)`, - color: theme.color.inverseText, - textDecoration: 'none', -})); +const Notification = styled.div<{ duration?: number }>( + ({ theme }) => ({ + position: 'relative', + display: 'flex', + padding: 15, + width: 280, + borderRadius: 4, + alignItems: 'center', + + background: theme.base === 'light' ? 'hsla(203, 50%, 20%, .97)' : 'hsla(203, 30%, 95%, .97)', + boxShadow: `0 2px 5px 0 rgba(0,0,0,0.05), 0 5px 15px 0 rgba(0,0,0,0.1)`, + color: theme.color.inverseText, + textDecoration: 'none', + overflow: 'hidden', + }), + ({ duration, theme }) => + duration && { + '&::after': { + content: '""', + display: 'block', + position: 'absolute', + bottom: 0, + left: 0, + height: 3, + background: theme.color.secondary, + animation: `${grow} ${duration}ms linear forwards reverse`, + }, + } +); const NotificationWithInteractiveStates = styled(Notification)(() => ({ transition: 'all 150ms ease-out', @@ -135,22 +160,34 @@ export const NotificationItemSpacer = styled.div({ const NotificationItem: FC<{ notification: State['notifications'][0]; onDismissNotification: (id: string) => void; -}> = ({ notification: { content, link, onClear, id, icon }, onDismissNotification }) => { - const dismissNotificationItem = () => { +}> = ({ notification: { content, duration, link, onClear, id, icon }, onDismissNotification }) => { + const onTimeout = useCallback(() => { onDismissNotification(id); - if (onClear) { - onClear({ dismissed: true }); - } - }; + if (onClear) onClear({ dismissed: false, timeout: true }); + }, [onDismissNotification, onClear]); + + const timer = useRef(null); + useEffect(() => { + if (!duration) return; + timer.current = setTimeout(onTimeout, duration); + return () => clearTimeout(timer.current); + }, [duration, onTimeout]); + + const onDismiss = useCallback(() => { + clearTimeout(timer.current); + onDismissNotification(id); + if (onClear) onClear({ dismissed: true, timeout: false }); + }, [onDismissNotification, onClear]); + return link ? ( - + - + ) : ( - + - + ); }; From b00d4ee29bae443158807f5794519d34a9104cc8 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sat, 30 Mar 2024 20:27:20 +0100 Subject: [PATCH 04/11] Add slide in animation to notifications --- .../components/notifications/NotificationItem.tsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/code/ui/manager/src/components/notifications/NotificationItem.tsx b/code/ui/manager/src/components/notifications/NotificationItem.tsx index 20d736bd32bb..ca07e513422a 100644 --- a/code/ui/manager/src/components/notifications/NotificationItem.tsx +++ b/code/ui/manager/src/components/notifications/NotificationItem.tsx @@ -8,6 +8,17 @@ import { IconButton, Icons } from '@storybook/components'; import { transparentize } from 'polished'; import { CloseAltIcon } from '@storybook/icons'; +const slideIn = keyframes({ + '0%': { + opacity: 0, + transform: 'translateY(30px)', + }, + '100%': { + opacity: 1, + transform: 'translateY(0)', + }, +}); + const grow = keyframes({ '0%': { width: '0%', @@ -26,6 +37,7 @@ const Notification = styled.div<{ duration?: number }>( borderRadius: 4, alignItems: 'center', + animation: `${slideIn} 500ms`, background: theme.base === 'light' ? 'hsla(203, 50%, 20%, .97)' : 'hsla(203, 30%, 95%, .97)', boxShadow: `0 2px 5px 0 rgba(0,0,0,0.05), 0 5px 15px 0 rgba(0,0,0,0.1)`, color: theme.color.inverseText, From 7f1c4e0272ebd974e0250e15417b3f85d962a3ba Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sat, 30 Mar 2024 21:06:10 +0100 Subject: [PATCH 05/11] Fix logging onClear handler --- .../components/notifications/NotificationItem.stories.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx b/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx index 210623ee6c8c..254812ead9d9 100644 --- a/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx +++ b/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { action } from '@storybook/addon-actions'; import { LocationProvider } from '@storybook/router'; import type { Meta, StoryObj } from '@storybook/react'; import NotificationItem from './NotificationItem'; @@ -7,7 +8,6 @@ import { BookIcon as BookIconIcon, FaceHappyIcon, } from '@storybook/icons'; -import { fn } from '@storybook/test'; const meta = { component: NotificationItem, @@ -30,8 +30,8 @@ const meta = { export default meta; type Story = StoryObj; -const onClear = fn(); -const onDismissNotification = fn(); +const onClear = action('onClear'); +const onDismissNotification = () => {}; export const Simple: Story = { args: { From 8338f595b66f593b9b28cbb05257f7ad057b73d8 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sat, 30 Mar 2024 21:10:27 +0100 Subject: [PATCH 06/11] Add missing property --- code/lib/manager-api/src/modules/notifications.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/lib/manager-api/src/modules/notifications.ts b/code/lib/manager-api/src/modules/notifications.ts index 0196fb1fcbc6..a8ee3195b6b7 100644 --- a/code/lib/manager-api/src/modules/notifications.ts +++ b/code/lib/manager-api/src/modules/notifications.ts @@ -30,7 +30,7 @@ export const init: ModuleFn = ({ store }) => { store.setState(({ notifications }) => { const [existing, others] = partition(notifications, (n) => n.id === newNotification.id); existing.forEach((notification) => { - if (notification.onClear) notification.onClear({ dismissed: false }); + if (notification.onClear) notification.onClear({ dismissed: false, timeout: false }); }); return { notifications: [...others, newNotification] }; }); @@ -40,7 +40,7 @@ export const init: ModuleFn = ({ store }) => { store.setState(({ notifications }) => { const [matching, others] = partition(notifications, (n) => n.id === notificationId); matching.forEach((notification) => { - if (notification.onClear) notification.onClear({ dismissed: false }); + if (notification.onClear) notification.onClear({ dismissed: false, timeout: false }); }); return { notifications: others }; }); From 908e92ec2b159bbedc15386a8b6e271200432224 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sat, 30 Mar 2024 21:26:16 +0100 Subject: [PATCH 07/11] Add onClick property to notifications --- code/lib/types/src/modules/api.ts | 8 +++++ .../NotificationItem.stories.tsx | 35 ++++++++++--------- .../notifications/NotificationItem.tsx | 32 +++++++++++++---- 3 files changed, 51 insertions(+), 24 deletions(-) diff --git a/code/lib/types/src/modules/api.ts b/code/lib/types/src/modules/api.ts index 4c6893d35ad4..481139e71792 100644 --- a/code/lib/types/src/modules/api.ts +++ b/code/lib/types/src/modules/api.ts @@ -124,6 +124,13 @@ interface OnClearOptions { timeout: boolean; } +interface OnClickOptions { + /** + * Function to dismiss the notification. + */ + onDismiss: () => void; +} + /** * @deprecated Use ReactNode for the icon instead. * @see https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#icons-is-deprecated @@ -143,6 +150,7 @@ export interface API_Notification { // TODO: Remove DeprecatedIconType in 9.0 icon?: React.ReactNode | DeprecatedIconType; onClear?: (options: OnClearOptions) => void; + onClick?: (options: OnClickOptions) => void; } type API_Versions = Record; diff --git a/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx b/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx index 254812ead9d9..13b20e0f7d99 100644 --- a/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx +++ b/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx @@ -25,13 +25,16 @@ const meta = { ), ], excludeStories: /.*Data$/, + args: { + onDismissNotification: () => {}, + }, } satisfies Meta; export default meta; type Story = StoryObj; const onClear = action('onClear'); -const onDismissNotification = () => {}; +const onClick = action('onClick'); export const Simple: Story = { args: { @@ -42,27 +45,24 @@ export const Simple: Story = { headline: 'Storybook cool!', }, }, - onDismissNotification, }, }; export const Timeout: Story = { args: { notification: { - id: '1', + id: 'Timeout', onClear, content: { headline: 'Storybook cool!', }, duration: 5000, }, - onDismissNotification, }, }; export const LongHeadline: Story = { args: { - ...Simple.args, notification: { id: '2', onClear, @@ -74,9 +74,21 @@ export const LongHeadline: Story = { }, }; +export const Clickable: Story = { + args: { + notification: { + id: 'Clickable', + onClear, + onClick, + content: { + headline: 'Storybook cool!', + }, + }, + }, +}; + export const Link: Story = { args: { - ...Simple.args, notification: { id: '3', onClear, @@ -90,7 +102,6 @@ export const Link: Story = { export const LinkIconWithColor: Story = { args: { - ...Simple.args, notification: { id: '4', onClear, @@ -105,7 +116,6 @@ export const LinkIconWithColor: Story = { export const LinkIconWithColorSubHeadline: Story = { args: { - ...Simple.args, notification: { id: '5', onClear, @@ -121,7 +131,6 @@ export const LinkIconWithColorSubHeadline: Story = { export const BookIcon: Story = { args: { - ...Simple.args, notification: { id: '6', onClear, @@ -136,7 +145,6 @@ export const BookIcon: Story = { export const StrongSubHeadline: Story = { args: { - ...Simple.args, notification: { id: '7', onClear, @@ -152,7 +160,6 @@ export const StrongSubHeadline: Story = { export const StrongEmphasizedSubHeadline: Story = { args: { - ...Simple.args, notification: { id: '8', onClear, @@ -172,7 +179,6 @@ export const StrongEmphasizedSubHeadline: Story = { export const BookIconSubHeadline: Story = { args: { - ...Simple.args, notification: { id: '9', onClear, @@ -188,7 +194,6 @@ export const BookIconSubHeadline: Story = { export const BookIconLongSubHeadline: Story = { args: { - ...Simple.args, notification: { id: '10', onClear, @@ -205,7 +210,6 @@ export const BookIconLongSubHeadline: Story = { export const AccessibilityIcon: Story = { args: { - ...Simple.args, notification: { id: '11', onClear, @@ -221,7 +225,6 @@ export const AccessibilityIcon: Story = { export const AccessibilityGoldIcon: Story = { args: { - ...Simple.args, notification: { id: '12', onClear, @@ -237,7 +240,6 @@ export const AccessibilityGoldIcon: Story = { export const AccessibilityGoldIconLongHeadLineNoSubHeadline: Story = { args: { - ...Simple.args, notification: { id: '13', onClear, @@ -252,7 +254,6 @@ export const AccessibilityGoldIconLongHeadLineNoSubHeadline: Story = { export const WithOldIconFormat: Story = { args: { - ...Simple.args, notification: { id: '13', onClear, diff --git a/code/ui/manager/src/components/notifications/NotificationItem.tsx b/code/ui/manager/src/components/notifications/NotificationItem.tsx index ca07e513422a..35403fab8296 100644 --- a/code/ui/manager/src/components/notifications/NotificationItem.tsx +++ b/code/ui/manager/src/components/notifications/NotificationItem.tsx @@ -60,6 +60,7 @@ const Notification = styled.div<{ duration?: number }>( ); const NotificationWithInteractiveStates = styled(Notification)(() => ({ + cursor: 'pointer', transition: 'all 150ms ease-out', transform: 'translate3d(0, 0, 0)', '&:hover': { @@ -158,6 +159,7 @@ const DismissNotificationItem: FC<{ title="Dismiss notification" onClick={(e: SyntheticEvent) => { e.preventDefault(); + e.stopPropagation(); onDismiss(); }} > @@ -172,7 +174,10 @@ export const NotificationItemSpacer = styled.div({ const NotificationItem: FC<{ notification: State['notifications'][0]; onDismissNotification: (id: string) => void; -}> = ({ notification: { content, duration, link, onClear, id, icon }, onDismissNotification }) => { +}> = ({ + notification: { content, duration, link, onClear, onClick, id, icon }, + onDismissNotification, +}) => { const onTimeout = useCallback(() => { onDismissNotification(id); if (onClear) onClear({ dismissed: false, timeout: true }); @@ -191,12 +196,25 @@ const NotificationItem: FC<{ if (onClear) onClear({ dismissed: true, timeout: false }); }, [onDismissNotification, onClear]); - return link ? ( - - - - - ) : ( + if (link) { + return ( + + + + + ); + } + + if (onClick) { + return ( + onClick({ onDismiss })} duration={duration}> + + + + ); + } + + return ( From d61bfd875916a1d4fea13ddec225e995c6f9d932 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sat, 30 Mar 2024 21:42:03 +0100 Subject: [PATCH 08/11] Use button element for clickable notification --- .../notifications/NotificationItem.stories.tsx | 1 + .../src/components/notifications/NotificationItem.tsx | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx b/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx index 13b20e0f7d99..e9c56c7fa060 100644 --- a/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx +++ b/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx @@ -53,6 +53,7 @@ export const Timeout: Story = { notification: { id: 'Timeout', onClear, + onClick, content: { headline: 'Storybook cool!', }, diff --git a/code/ui/manager/src/components/notifications/NotificationItem.tsx b/code/ui/manager/src/components/notifications/NotificationItem.tsx index 35403fab8296..d47f539a3f1c 100644 --- a/code/ui/manager/src/components/notifications/NotificationItem.tsx +++ b/code/ui/manager/src/components/notifications/NotificationItem.tsx @@ -61,6 +61,9 @@ const Notification = styled.div<{ duration?: number }>( const NotificationWithInteractiveStates = styled(Notification)(() => ({ cursor: 'pointer', + border: 'none', + outline: 'none', + textAlign: 'left', transition: 'all 150ms ease-out', transform: 'translate3d(0, 0, 0)', '&:hover': { @@ -75,9 +78,10 @@ const NotificationWithInteractiveStates = styled(Notification)(() => ({ }, '&:focus': { boxShadow: - '0 1px 3px 0 rgba(30,167,253,0.5), 0 2px 5px 0 rgba(0,0,0,0.05), 0 5px 15px 0 rgba(0,0,0,0.1)', + 'rgba(2,156,253,1) 0 0 0 1px inset, 0 1px 3px 0 rgba(30,167,253,0.5), 0 2px 5px 0 rgba(0,0,0,0.05), 0 5px 15px 0 rgba(0,0,0,0.1)', }, })); +const NotificationButton = NotificationWithInteractiveStates.withComponent('button'); const NotificationLink = NotificationWithInteractiveStates.withComponent(Link); const NotificationIconWrapper = styled.div(() => ({ @@ -207,10 +211,10 @@ const NotificationItem: FC<{ if (onClick) { return ( - onClick({ onDismiss })} duration={duration}> + onClick({ onDismiss })}> - + ); } From b280b720ad8ac7d92864e3781a9266539b97357e Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sun, 31 Mar 2024 15:57:00 +0200 Subject: [PATCH 09/11] Verify callback functionality --- .../NotificationItem.stories.tsx | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx b/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx index e9c56c7fa060..af4c7fcc8e57 100644 --- a/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx +++ b/code/ui/manager/src/components/notifications/NotificationItem.stories.tsx @@ -8,6 +8,7 @@ import { BookIcon as BookIconIcon, FaceHappyIcon, } from '@storybook/icons'; +import { expect, fn, userEvent, waitFor, within } from '@storybook/test'; const meta = { component: NotificationItem, @@ -33,8 +34,8 @@ const meta = { export default meta; type Story = StoryObj; -const onClear = action('onClear'); -const onClick = action('onClick'); +const onClear = fn(action('onClear')); +const onClick = fn(action('onClick')); export const Simple: Story = { args: { @@ -57,9 +58,19 @@ export const Timeout: Story = { content: { headline: 'Storybook cool!', }, - duration: 5000, + duration: 3000, }, }, + play: async ({ args }) => { + await waitFor( + () => { + expect(args.notification.onClear).toHaveBeenCalledWith({ dismissed: false, timeout: true }); + }, + { + timeout: 4000, + } + ); + }, }; export const LongHeadline: Story = { @@ -86,6 +97,12 @@ export const Clickable: Story = { }, }, }, + play: async ({ args, canvasElement }) => { + const canvas = within(canvasElement); + const [button] = await canvas.findAllByRole('button'); + await userEvent.click(button); + await expect(args.notification.onClick).toHaveBeenCalledWith({ onDismiss: expect.anything() }); + }, }; export const Link: Story = { From 385f9b97f5e5446c88539b75e4fa9ae8d3c61da6 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 2 Apr 2024 16:53:47 +0200 Subject: [PATCH 10/11] Update tests --- .../src/tests/notifications.test.js | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/code/lib/manager-api/src/tests/notifications.test.js b/code/lib/manager-api/src/tests/notifications.test.js index 51a53be2b1e3..105b973f3b3b 100644 --- a/code/lib/manager-api/src/tests/notifications.test.js +++ b/code/lib/manager-api/src/tests/notifications.test.js @@ -2,34 +2,33 @@ import { describe, it, expect, vi } from 'vitest'; import { init as initNotifications } from '../modules/notifications'; describe('notifications API', () => { - it('allows adding notifications', () => { - const store = { - getState: () => ({ - notifications: [], - }), - setState: vi.fn(), - }; + const store = { + state: { notifications: [] }, + getState: () => store.state, + setState: (update) => { + if (typeof update === 'function') { + store.state = update(store.state); + } else { + store.state = update; + } + }, + }; + it.only('allows adding notifications', () => { const { api } = initNotifications({ store }); api.addNotification({ id: '1' }); - expect(store.setState).toHaveBeenCalledWith({ + expect(store.getState()).toEqual({ notifications: [{ id: '1' }], }); }); it('allows removing notifications', () => { - const store = { - getState: () => ({ - notifications: [{ id: '1' }, { id: '2' }, { id: '3' }], - }), - setState: vi.fn(), - }; - + store.setState({ notifications: [{ id: '1' }, { id: '2' }, { id: '3' }] }); const { api } = initNotifications({ store }); api.clearNotification('2'); - expect(store.setState).toHaveBeenCalledWith({ + expect(store.getState()).toEqual({ notifications: [{ id: '1' }, { id: '3' }], }); }); From f3c4e001f50e1cdca08201bbc923a7d770172f21 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 2 Apr 2024 20:20:00 +0200 Subject: [PATCH 11/11] Remove .only modifier --- code/lib/manager-api/src/tests/notifications.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/lib/manager-api/src/tests/notifications.test.js b/code/lib/manager-api/src/tests/notifications.test.js index 105b973f3b3b..8d2005db6837 100644 --- a/code/lib/manager-api/src/tests/notifications.test.js +++ b/code/lib/manager-api/src/tests/notifications.test.js @@ -14,7 +14,7 @@ describe('notifications API', () => { }, }; - it.only('allows adding notifications', () => { + it('allows adding notifications', () => { const { api } = initNotifications({ store }); api.addNotification({ id: '1' });