From 793049388b7ed2aa8d3a05819f660b61edf73709 Mon Sep 17 00:00:00 2001 From: Michel Heusschen <59014050+michelheusschen@users.noreply.github.com> Date: Thu, 21 Mar 2024 14:44:54 +0100 Subject: [PATCH] refactor(web): cleanup notification components (#8150) * refactor(web): cleanup notification components * use counter for ID --- .../__tests__/notification-card.spec.ts | 4 +- .../__tests__/notification-list.spec.ts | 19 ++--- .../notification/notification-card.svelte | 84 ++++++------------- .../notification/notification-list.svelte | 4 +- .../notification/notification.ts | 62 ++++++-------- 5 files changed, 64 insertions(+), 109 deletions(-) diff --git a/web/src/lib/components/shared-components/notification/__tests__/notification-card.spec.ts b/web/src/lib/components/shared-components/notification/__tests__/notification-card.spec.ts index 179f1c2f62..76b9c39564 100644 --- a/web/src/lib/components/shared-components/notification/__tests__/notification-card.spec.ts +++ b/web/src/lib/components/shared-components/notification/__tests__/notification-card.spec.ts @@ -10,7 +10,7 @@ describe('NotificationCard component', () => { vi.spyOn(window, 'clearTimeout'); sut = render(NotificationCard, { - notificationInfo: { + notification: { id: 1234, message: 'Notification message', timeout: 1000, @@ -25,7 +25,7 @@ describe('NotificationCard component', () => { it('shows message and title', () => { sut = render(NotificationCard, { - notificationInfo: { + notification: { id: 1234, message: 'Notification message', timeout: 1000, diff --git a/web/src/lib/components/shared-components/notification/__tests__/notification-list.spec.ts b/web/src/lib/components/shared-components/notification/__tests__/notification-list.spec.ts index f8472a3ced..44634d6b20 100644 --- a/web/src/lib/components/shared-components/notification/__tests__/notification-list.spec.ts +++ b/web/src/lib/components/shared-components/notification/__tests__/notification-list.spec.ts @@ -12,11 +12,14 @@ describe('NotificationList component', () => { const sut: RenderResult = render(NotificationList); beforeAll(() => { - vi.useFakeTimers(); + // https://testing-library.com/docs/svelte-testing-library/faq#why-arent-transition-events-running + vi.stubGlobal('requestAnimationFrame', (fn: FrameRequestCallback) => { + setTimeout(() => fn(Date.now()), 16); + }); }); afterAll(() => { - vi.useRealTimers(); + vi.unstubAllGlobals(); }); it('shows a notification when added and closes it automatically after the delay timeout', async () => { @@ -25,18 +28,14 @@ describe('NotificationList component', () => { notificationController.show({ message: 'Notification', type: NotificationType.Info, - timeout: 3000, + timeout: 1, }); await waitFor(() => expect(_getNotificationListElement(sut)).toBeInTheDocument()); + await waitFor(() => expect(_getNotificationListElement(sut)?.children).toHaveLength(1)); + expect(get(notificationController.notificationList)).toHaveLength(1); - expect(_getNotificationListElement(sut)?.children).toHaveLength(1); - - vi.advanceTimersByTime(4000); - // due to some weirdness in svelte (or testing-library) need to check if it has been removed from the store to make sure it works. + await waitFor(() => expect(_getNotificationListElement(sut)).not.toBeInTheDocument()); expect(get(notificationController.notificationList)).toHaveLength(0); - - // TODO: investigate why this element is not removed from the DOM even notification list is in fact 0. - // await waitFor(() => expect(_getNotificationListElement(sut)).not.toBeInTheDocument()); }); }); diff --git a/web/src/lib/components/shared-components/notification/notification-card.svelte b/web/src/lib/components/shared-components/notification/notification-card.svelte index ef855c90a2..a81146692e 100644 --- a/web/src/lib/components/shared-components/notification/notification-card.svelte +++ b/web/src/lib/components/shared-components/notification/notification-card.svelte @@ -2,77 +2,47 @@ import { fade } from 'svelte/transition'; import Icon from '$lib/components/elements/icon.svelte'; import { - ImmichNotification, + type Notification, notificationController, NotificationType, } from '$lib/components/shared-components/notification/notification'; import { onMount } from 'svelte'; import { mdiCloseCircleOutline, mdiInformationOutline, mdiWindowClose } from '@mdi/js'; - export let notificationInfo: ImmichNotification; + export let notification: Notification; - let infoPrimaryColor = '#4250AF'; - let errorPrimaryColor = '#E64132'; - let warningPrimaryColor = '#D08613'; + $: icon = notification.type === NotificationType.Error ? mdiCloseCircleOutline : mdiInformationOutline; - $: icon = notificationInfo.type === NotificationType.Error ? mdiCloseCircleOutline : mdiInformationOutline; - - $: backgroundColor = () => { - if (notificationInfo.type === NotificationType.Info) { - return '#E0E2F0'; - } - - if (notificationInfo.type === NotificationType.Error) { - return '#FBE8E6'; - } - - if (notificationInfo.type === NotificationType.Warning) { - return '#FFF6DC'; - } + const backgroundColor: Record = { + [NotificationType.Info]: '#E0E2F0', + [NotificationType.Error]: '#FBE8E6', + [NotificationType.Warning]: '#FFF6DC', }; - $: borderStyle = () => { - if (notificationInfo.type === NotificationType.Info) { - return '1px solid #D8DDFF'; - } - - if (notificationInfo.type === NotificationType.Error) { - return '1px solid #F0E8E7'; - } - - if (notificationInfo.type === NotificationType.Warning) { - return '1px solid #FFE6A5'; - } + const borderColor: Record = { + [NotificationType.Info]: '#D8DDFF', + [NotificationType.Error]: '#F0E8E7', + [NotificationType.Warning]: '#FFE6A5', }; - $: primaryColor = () => { - if (notificationInfo.type === NotificationType.Info) { - return infoPrimaryColor; - } - - if (notificationInfo.type === NotificationType.Error) { - return errorPrimaryColor; - } - - if (notificationInfo.type === NotificationType.Warning) { - return warningPrimaryColor; - } + const primaryColor: Record = { + [NotificationType.Info]: '#4250AF', + [NotificationType.Error]: '#E64132', + [NotificationType.Warning]: '#D08613', }; - let removeNotificationTimeout: ReturnType | undefined; - onMount(() => { - removeNotificationTimeout = setTimeout(discard, notificationInfo.timeout); - return () => clearTimeout(removeNotificationTimeout); + const timeoutId = setTimeout(discard, notification.timeout); + return () => clearTimeout(timeoutId); }); const discard = () => { - notificationController.removeNotificationById(notificationInfo.id); + notificationController.removeNotificationById(notification.id); }; const handleClick = () => { - const action = notificationInfo.action; - if (action.type == 'discard') { + const action = notification.action; + if (action.type === 'discard') { discard(); } else if (action.type == 'link') { window.open(action.target); @@ -83,17 +53,17 @@
- -

- {notificationInfo.type.toString()} + +

+ {notification.type.toString()}

- {notificationInfo.message} + {notification.message}

diff --git a/web/src/lib/components/shared-components/notification/notification-list.svelte b/web/src/lib/components/shared-components/notification/notification-list.svelte index bf8d93d5f2..d94ff5c14d 100644 --- a/web/src/lib/components/shared-components/notification/notification-list.svelte +++ b/web/src/lib/components/shared-components/notification/notification-list.svelte @@ -11,9 +11,9 @@ {#if $notificationList.length > 0}
- {#each $notificationList as notificationInfo (notificationInfo.id)} + {#each $notificationList as notification (notification.id)}
- +
{/each}
diff --git a/web/src/lib/components/shared-components/notification/notification.ts b/web/src/lib/components/shared-components/notification/notification.ts index f1a2140461..52e75bf41f 100644 --- a/web/src/lib/components/shared-components/notification/notification.ts +++ b/web/src/lib/components/shared-components/notification/notification.ts @@ -6,57 +6,43 @@ export enum NotificationType { Warning = 'Warning', } -export class ImmichNotification { - id = Date.now() + Math.random(); - type!: NotificationType; - message!: string; - action!: NotificationAction; - timeout = 3000; -} +export type Notification = { + id: number; + type: NotificationType; + message: string; + /** The action to take when the notification is clicked */ + action: NotificationAction; + /** Timeout in miliseconds */ + timeout: number; +}; type DiscardAction = { type: 'discard' }; type NoopAction = { type: 'noop' }; type LinkAction = { type: 'link'; target: string }; export type NotificationAction = DiscardAction | NoopAction | LinkAction; -export class ImmichNotificationDto { - /** - * Notification type - * @type {NotificationType} [Info, Error] - */ - type: NotificationType = NotificationType.Info; - - /** - * Notification message - */ - message = ''; - - /** - * Timeout in miliseconds - */ - timeout?: number; - - /** - * The action to take when the notification is clicked - */ - action?: NotificationAction; -} +export type NotificationOptions = Partial> & { message: string }; function createNotificationList() { - const notificationList = writable([]); + const notificationList = writable([]); + let count = 1; - const show = (notificationInfo: ImmichNotificationDto) => { - const newNotification = new ImmichNotification(); - newNotification.message = notificationInfo.message; - newNotification.type = notificationInfo.type; - newNotification.timeout = notificationInfo.timeout || 3000; - newNotification.action = notificationInfo.action || { type: 'discard' }; + const show = (options: NotificationOptions) => { + notificationList.update((currentList) => { + currentList.push({ + id: count++, + type: NotificationType.Info, + action: { type: 'discard' }, + timeout: 3000, + ...options, + }); - notificationList.update((currentList) => [...currentList, newNotification]); + return currentList; + }); }; const removeNotificationById = (id: number) => { - notificationList.update((currentList) => currentList.filter((n) => n.id != id)); + notificationList.update((currentList) => currentList.filter((n) => n.id !== id)); }; return {