From 61be2ba917df7c9dc64eff232f4b5a15543b63bf Mon Sep 17 00:00:00 2001 From: Kasper Birch Date: Fri, 13 Oct 2023 14:48:24 +0200 Subject: [PATCH 01/17] Add `promobar` to `PatronPage` + Introduce `useNotificationMessage` hook The `useNotificationMessage` hook manages to handle auto-scrolling to the top and setting a removal timeout for notifications. It integrates seamlessly with `promobar` and offers utility for additional use-cases. --- src/apps/patron-page/PatronPage.tsx | 11 ++++++++ src/core/utils/useNotificationMessage.ts | 32 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 src/core/utils/useNotificationMessage.ts diff --git a/src/apps/patron-page/PatronPage.tsx b/src/apps/patron-page/PatronPage.tsx index c1989e473f..487d5d09cb 100644 --- a/src/apps/patron-page/PatronPage.tsx +++ b/src/apps/patron-page/PatronPage.tsx @@ -17,6 +17,8 @@ import PauseReservation from "../reservation-list/modal/pause-reservation/pause- import { getModalIds } from "../../core/utils/helpers/general"; import { useUrls } from "../../core/utils/url"; import { usePatronData } from "../../components/material/helper"; +import PromoBar from "../../components/promo-bar/PromoBar"; +import { useNotificationMessage } from "../../core/utils/useNotificationMessage"; const PatronPage: FC = () => { const queryClient = useQueryClient(); @@ -33,6 +35,11 @@ const PatronPage: FC = () => { const [successPinMessage, setSuccessPinMessage] = useState( null ); + const [notificationMessage, handleNotificationMessage] = + useNotificationMessage({ + scrollToTop: true, + timeout: 5000 + }); useEffect(() => { if (patronData && patronData.patron) { @@ -86,6 +93,7 @@ const PatronPage: FC = () => { setSuccessPinMessage(t("patronPinSavedSuccessText")); } setDisableSubmitButton(false); + handleNotificationMessage(t("handleResponseInformationText")); }, // todo error handling, missing in figma onError: () => { @@ -105,6 +113,9 @@ const PatronPage: FC = () => { <>
handleSubmit(e)}>

{t("patronPageHeaderText")}

+ {notificationMessage && ( + + )} {patron && }
{patron && ( diff --git a/src/core/utils/useNotificationMessage.ts b/src/core/utils/useNotificationMessage.ts new file mode 100644 index 0000000000..43993c7cb1 --- /dev/null +++ b/src/core/utils/useNotificationMessage.ts @@ -0,0 +1,32 @@ +import { useState } from "react"; + +type UseNotificationOptionsType = { + timeout?: number; + scrollToTop?: boolean; +}; +type UseNotificationReturnType = [string | null, (text: string) => void]; + +export const useNotificationMessage = ({ + timeout, + scrollToTop +}: UseNotificationOptionsType): UseNotificationReturnType => { + const [notificationMessage, setNotificationMessage] = useState( + null + ); + + const handleNotificationMessage = (text: string) => { + setNotificationMessage(text); + if (scrollToTop) { + window.scrollTo(0, 0); + } + if (timeout) { + setTimeout(() => { + setNotificationMessage(null); + }, timeout); + } + }; + + return [notificationMessage, handleNotificationMessage]; +}; + +export default {}; From 0b3374e5b08cfa92f532ecb63e61dc06444ca338 Mon Sep 17 00:00:00 2001 From: Kasper Birch Date: Fri, 13 Oct 2023 14:48:56 +0200 Subject: [PATCH 02/17] Add margin to the icon in `PromoBarIcon` --- src/components/promo-bar/PromoBarIcon.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/promo-bar/PromoBarIcon.tsx b/src/components/promo-bar/PromoBarIcon.tsx index 560de65933..2e46e5e688 100644 --- a/src/components/promo-bar/PromoBarIcon.tsx +++ b/src/components/promo-bar/PromoBarIcon.tsx @@ -11,7 +11,7 @@ export const PromoBarIcon: React.FunctionComponent = ({ type }) => { if (type === "info") { - return ; + return ; } return null; }; From 5df01b9118407731fe824b901e4989bf5753706c Mon Sep 17 00:00:00 2001 From: Kasper Birch Date: Fri, 13 Oct 2023 15:05:52 +0200 Subject: [PATCH 03/17] Add new translations for `promobar` in `PatronPage` --- src/apps/patron-page/PatronPage.dev.tsx | 9 +++++++++ src/apps/patron-page/PatronPage.entry.tsx | 2 ++ 2 files changed, 11 insertions(+) diff --git a/src/apps/patron-page/PatronPage.dev.tsx b/src/apps/patron-page/PatronPage.dev.tsx index f9c457d335..e88adbb034 100644 --- a/src/apps/patron-page/PatronPage.dev.tsx +++ b/src/apps/patron-page/PatronPage.dev.tsx @@ -245,6 +245,15 @@ export default { defaultValue: "The phone number must be 6 to 15 characters in length and should be comprised solely of numbers or begin with a +", control: { type: "text" } + }, + handleResponseInformationText: { + defaultValue: "Dine ændringer er gemt.", + control: { type: "text" } + }, + loadingText: { + name: "Loading", + defaultValue: "Loading..", + control: { type: "text" } } } } as ComponentMeta; diff --git a/src/apps/patron-page/PatronPage.entry.tsx b/src/apps/patron-page/PatronPage.entry.tsx index 0ec2fbe099..78e43620e2 100644 --- a/src/apps/patron-page/PatronPage.entry.tsx +++ b/src/apps/patron-page/PatronPage.entry.tsx @@ -71,6 +71,8 @@ interface PatronPageTextProps { patronPageStatusSectionOutOfAriaLabelAudioBooksText: string; patronPageStatusSectionOutOfAriaLabelEbooksText: string; phoneInputMessageText: string; + handleResponseInformationText: string; + loadingText: string; } export interface PatronPageProps From 15f855ab98ceaf85e357464883ef0dd8daf2c93e Mon Sep 17 00:00:00 2001 From: Kasper Birch Date: Fri, 13 Oct 2023 14:51:29 +0200 Subject: [PATCH 04/17] Handle button loading state in `PatronPage` --- src/apps/patron-page/PatronPage.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/apps/patron-page/PatronPage.tsx b/src/apps/patron-page/PatronPage.tsx index 487d5d09cb..2ca03f7f39 100644 --- a/src/apps/patron-page/PatronPage.tsx +++ b/src/apps/patron-page/PatronPage.tsx @@ -146,7 +146,9 @@ const PatronPage: FC = () => { type="submit" disabled={disableSubmitButton} > - {t("patronPageSaveButtonText")} + {disableSubmitButton + ? t("loadingText") + : t("patronPageSaveButtonText")}
From 36d2d3ad82c2028b5a65a4848993610131e575bd Mon Sep 17 00:00:00 2001 From: Kasper Birch Date: Tue, 17 Oct 2023 15:01:50 +0200 Subject: [PATCH 05/17] Translate string to english --- src/apps/patron-page/PatronPage.dev.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apps/patron-page/PatronPage.dev.tsx b/src/apps/patron-page/PatronPage.dev.tsx index e88adbb034..ce0e337d6c 100644 --- a/src/apps/patron-page/PatronPage.dev.tsx +++ b/src/apps/patron-page/PatronPage.dev.tsx @@ -247,7 +247,7 @@ export default { control: { type: "text" } }, handleResponseInformationText: { - defaultValue: "Dine ændringer er gemt.", + defaultValue: "Your changes are saved.", control: { type: "text" } }, loadingText: { From 6bc7f33a9f468da8c4e831792a4e31770194f543 Mon Sep 17 00:00:00 2001 From: Kasper Birch Date: Thu, 19 Oct 2023 11:12:36 +0200 Subject: [PATCH 06/17] Refactor `useNotificationMessage` Modified the hook to return either the `Promobar` component or 'null' as the initial item in the array. Also provided default settings for `timeout` and `scrollToTop` which can be overridden during usage. --- src/apps/patron-page/PatronPage.tsx | 12 +++--------- ...onMessage.ts => useNotificationMessage.tsx} | 18 ++++++++++++------ 2 files changed, 15 insertions(+), 15 deletions(-) rename src/core/utils/{useNotificationMessage.ts => useNotificationMessage.tsx} (50%) diff --git a/src/apps/patron-page/PatronPage.tsx b/src/apps/patron-page/PatronPage.tsx index 2ca03f7f39..1d4079b34e 100644 --- a/src/apps/patron-page/PatronPage.tsx +++ b/src/apps/patron-page/PatronPage.tsx @@ -17,7 +17,6 @@ import PauseReservation from "../reservation-list/modal/pause-reservation/pause- import { getModalIds } from "../../core/utils/helpers/general"; import { useUrls } from "../../core/utils/url"; import { usePatronData } from "../../components/material/helper"; -import PromoBar from "../../components/promo-bar/PromoBar"; import { useNotificationMessage } from "../../core/utils/useNotificationMessage"; const PatronPage: FC = () => { @@ -35,11 +34,8 @@ const PatronPage: FC = () => { const [successPinMessage, setSuccessPinMessage] = useState( null ); - const [notificationMessage, handleNotificationMessage] = - useNotificationMessage({ - scrollToTop: true, - timeout: 5000 - }); + const [NotificationComponent, handleNotificationMessage] = + useNotificationMessage(); useEffect(() => { if (patronData && patronData.patron) { @@ -113,9 +109,7 @@ const PatronPage: FC = () => { <> handleSubmit(e)}>

{t("patronPageHeaderText")}

- {notificationMessage && ( - - )} + {patron && }
{patron && ( diff --git a/src/core/utils/useNotificationMessage.ts b/src/core/utils/useNotificationMessage.tsx similarity index 50% rename from src/core/utils/useNotificationMessage.ts rename to src/core/utils/useNotificationMessage.tsx index 43993c7cb1..3dad9d972b 100644 --- a/src/core/utils/useNotificationMessage.ts +++ b/src/core/utils/useNotificationMessage.tsx @@ -1,15 +1,16 @@ -import { useState } from "react"; +import React, { useState } from "react"; +import PromoBar from "../../components/promo-bar/PromoBar"; type UseNotificationOptionsType = { timeout?: number; scrollToTop?: boolean; }; -type UseNotificationReturnType = [string | null, (text: string) => void]; +type UseNotificationReturnType = [React.FC, (text: string) => void]; export const useNotificationMessage = ({ - timeout, - scrollToTop -}: UseNotificationOptionsType): UseNotificationReturnType => { + timeout = 5000, + scrollToTop = true +}: UseNotificationOptionsType = {}): UseNotificationReturnType => { const [notificationMessage, setNotificationMessage] = useState( null ); @@ -26,7 +27,12 @@ export const useNotificationMessage = ({ } }; - return [notificationMessage, handleNotificationMessage]; + const NotificationComponent: React.FC = () => { + if (!notificationMessage) return null; + return ; + }; + + return [NotificationComponent, handleNotificationMessage]; }; export default {}; From 72210b74a13d75b58daebf831af415253ea020ad Mon Sep 17 00:00:00 2001 From: Kasper Birch Date: Thu, 19 Oct 2023 11:25:52 +0200 Subject: [PATCH 07/17] Prefix Patron translations with 'patronPage' --- src/apps/patron-page/PatronPage.dev.tsx | 6 +++--- src/apps/patron-page/PatronPage.entry.tsx | 6 +++--- src/apps/patron-page/PatronPage.tsx | 6 ++++-- src/components/contact-info-section/ContactInfoPhone.tsx | 4 ++-- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/apps/patron-page/PatronPage.dev.tsx b/src/apps/patron-page/PatronPage.dev.tsx index ce0e337d6c..ee779ad039 100644 --- a/src/apps/patron-page/PatronPage.dev.tsx +++ b/src/apps/patron-page/PatronPage.dev.tsx @@ -240,17 +240,17 @@ export default { defaultValue: "You used @this ebooks out of you quota of @that ebooks", control: { type: "text" } }, - phoneInputMessageText: { + patronPagePhoneInputMessageText: { name: "Phone input validation message", defaultValue: "The phone number must be 6 to 15 characters in length and should be comprised solely of numbers or begin with a +", control: { type: "text" } }, - handleResponseInformationText: { + patronPageHandleResponseInformationText: { defaultValue: "Your changes are saved.", control: { type: "text" } }, - loadingText: { + patronPageLoadingText: { name: "Loading", defaultValue: "Loading..", control: { type: "text" } diff --git a/src/apps/patron-page/PatronPage.entry.tsx b/src/apps/patron-page/PatronPage.entry.tsx index 78e43620e2..a59586ab65 100644 --- a/src/apps/patron-page/PatronPage.entry.tsx +++ b/src/apps/patron-page/PatronPage.entry.tsx @@ -70,9 +70,9 @@ interface PatronPageTextProps { patronPageStatusSectionOutOfText: string; patronPageStatusSectionOutOfAriaLabelAudioBooksText: string; patronPageStatusSectionOutOfAriaLabelEbooksText: string; - phoneInputMessageText: string; - handleResponseInformationText: string; - loadingText: string; + patronPagePhoneInputMessageText: string; + patronPageHandleResponseInformationText: string; + patronPageLoadingText: string; } export interface PatronPageProps diff --git a/src/apps/patron-page/PatronPage.tsx b/src/apps/patron-page/PatronPage.tsx index 1d4079b34e..91076e1d49 100644 --- a/src/apps/patron-page/PatronPage.tsx +++ b/src/apps/patron-page/PatronPage.tsx @@ -89,7 +89,9 @@ const PatronPage: FC = () => { setSuccessPinMessage(t("patronPinSavedSuccessText")); } setDisableSubmitButton(false); - handleNotificationMessage(t("handleResponseInformationText")); + handleNotificationMessage( + t("patronPageHandleResponseInformationText") + ); }, // todo error handling, missing in figma onError: () => { @@ -141,7 +143,7 @@ const PatronPage: FC = () => { disabled={disableSubmitButton} > {disableSubmitButton - ? t("loadingText") + ? t("patronPageLoadingText") : t("patronPageSaveButtonText")} diff --git a/src/components/contact-info-section/ContactInfoPhone.tsx b/src/components/contact-info-section/ContactInfoPhone.tsx index e8e8aed13e..1c9d14484e 100644 --- a/src/components/contact-info-section/ContactInfoPhone.tsx +++ b/src/components/contact-info-section/ContactInfoPhone.tsx @@ -30,13 +30,13 @@ const ContactInfoPhone: FC = ({ required={isRequired} type="tel" pattern="\+?[0-9]{6,15}" - title={t("phoneInputMessageText")} + title={t("patronPagePhoneInputMessageText")} onChange={(newPhoneNumber) => changePatron(newPhoneNumber, "phoneNumber") } value={patron?.phoneNumber} label={t("patronContactPhoneLabelText")} - placeholder={t("phoneInputMessageText")} + placeholder={t("patronPagePhoneInputMessageText")} /> {showCheckboxes && ( Date: Thu, 19 Oct 2023 16:15:45 +0200 Subject: [PATCH 08/17] Separate `NotificationComponent` from `useNotificationMessage` hook In this commit, `NotificationComponent` has been extracted from the `useNotificationMessage` hook. By wrapping the retrun of `NotificationComponent` in a function, we enhance the reactivity of the component, ensuring that its body (the JSX) is re-evaluated with each render. This method is beneficial as it provides a mechanism to always capture the most recent state or props, eliminating concerns about stale closures. --- .../notification/NotificationComponent.tsx | 15 +++++++++++++++ src/core/utils/useNotificationMessage.tsx | 12 +++++------- 2 files changed, 20 insertions(+), 7 deletions(-) create mode 100644 src/components/notification/NotificationComponent.tsx diff --git a/src/components/notification/NotificationComponent.tsx b/src/components/notification/NotificationComponent.tsx new file mode 100644 index 0000000000..31f8cdf188 --- /dev/null +++ b/src/components/notification/NotificationComponent.tsx @@ -0,0 +1,15 @@ +import React from "react"; +import PromoBar from "../promo-bar/PromoBar"; + +interface NotificationComponentProps { + notificationMessage: string | null; +} + +const NotificationComponent: React.FC = ({ + notificationMessage +}) => { + if (!notificationMessage) return null; + return ; +}; + +export default NotificationComponent; diff --git a/src/core/utils/useNotificationMessage.tsx b/src/core/utils/useNotificationMessage.tsx index 3dad9d972b..3dbc266ad6 100644 --- a/src/core/utils/useNotificationMessage.tsx +++ b/src/core/utils/useNotificationMessage.tsx @@ -1,5 +1,5 @@ import React, { useState } from "react"; -import PromoBar from "../../components/promo-bar/PromoBar"; +import NotificationComponent from "../../components/notification/NotificationComponent"; type UseNotificationOptionsType = { timeout?: number; @@ -27,12 +27,10 @@ export const useNotificationMessage = ({ } }; - const NotificationComponent: React.FC = () => { - if (!notificationMessage) return null; - return ; - }; - - return [NotificationComponent, handleNotificationMessage]; + return [ + () => , + handleNotificationMessage + ]; }; export default {}; From 64c268f187dbe1398419cca5234249b492a1fad2 Mon Sep 17 00:00:00 2001 From: Kasper Birch Date: Thu, 19 Oct 2023 16:37:40 +0200 Subject: [PATCH 09/17] Refactor logic to decide returning `` or null. Centralize all `useNotificationMessage` related logic. --- src/components/notification/NotificationComponent.tsx | 3 +-- src/core/utils/useNotificationMessage.tsx | 5 ++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/notification/NotificationComponent.tsx b/src/components/notification/NotificationComponent.tsx index 31f8cdf188..0cc1303965 100644 --- a/src/components/notification/NotificationComponent.tsx +++ b/src/components/notification/NotificationComponent.tsx @@ -2,13 +2,12 @@ import React from "react"; import PromoBar from "../promo-bar/PromoBar"; interface NotificationComponentProps { - notificationMessage: string | null; + notificationMessage: string; } const NotificationComponent: React.FC = ({ notificationMessage }) => { - if (!notificationMessage) return null; return ; }; diff --git a/src/core/utils/useNotificationMessage.tsx b/src/core/utils/useNotificationMessage.tsx index 3dbc266ad6..558cc7eda2 100644 --- a/src/core/utils/useNotificationMessage.tsx +++ b/src/core/utils/useNotificationMessage.tsx @@ -28,7 +28,10 @@ export const useNotificationMessage = ({ }; return [ - () => , + () => + notificationMessage ? ( + + ) : null, handleNotificationMessage ]; }; From 17431686e8364cf3178e47af9ff08a0dec77f66d Mon Sep 17 00:00:00 2001 From: Mikkel Jakobsen Date: Fri, 27 Oct 2023 14:10:28 +0200 Subject: [PATCH 10/17] Add tests for the useNotificationMessage hook --- src/tests/unit/notification-message.test.tsx | 91 ++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 src/tests/unit/notification-message.test.tsx diff --git a/src/tests/unit/notification-message.test.tsx b/src/tests/unit/notification-message.test.tsx new file mode 100644 index 0000000000..e9148d1320 --- /dev/null +++ b/src/tests/unit/notification-message.test.tsx @@ -0,0 +1,91 @@ +import { describe, expect, it } from "vitest"; +import React from "react"; +import { fireEvent, render } from "@testing-library/react"; +import { useNotificationMessage } from "../../core/utils/useNotificationMessage"; + +const ComponentWithNotificationMessage = ({ + wrapperTitle, + buttonTitle +}: { + wrapperTitle: string; + buttonTitle: string; +}) => { + const [NotificationMessage, handler] = useNotificationMessage(); + + return ( +
+ + +
+ ); +}; + +describe("useNotification hook", () => { + it("Does not show a message if button is not clicked", async () => { + const { getByTitle } = render( + + ); + const wrapper = getByTitle("test-1"); + + expect(wrapper).toMatchInlineSnapshot(` +
+ +
+ `); + }); + + it("Does shows a message if button is clicked", async () => { + const { getByTitle } = render( + + ); + const wrapper = getByTitle("test-2"); + const button = getByTitle("test-2-button"); + + fireEvent.click(button); + expect(wrapper).toMatchInlineSnapshot(` +
+
+ +

+ Some message +

+
+ +
+ `); + }); +}); From 8d94fbaa7b8c40e01359bd353c58610f7b8f0228 Mon Sep 17 00:00:00 2001 From: Mikkel Jakobsen Date: Fri, 27 Oct 2023 14:42:49 +0200 Subject: [PATCH 11/17] Add assertion for scroll in notification message test When the handler has been called the user should be sent to the top of the page --- src/tests/unit/notification-message.test.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/tests/unit/notification-message.test.tsx b/src/tests/unit/notification-message.test.tsx index e9148d1320..b419294333 100644 --- a/src/tests/unit/notification-message.test.tsx +++ b/src/tests/unit/notification-message.test.tsx @@ -1,4 +1,4 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import React from "react"; import { fireEvent, render } from "@testing-library/react"; import { useNotificationMessage } from "../../core/utils/useNotificationMessage"; @@ -51,6 +51,8 @@ describe("useNotification hook", () => { }); it("Does shows a message if button is clicked", async () => { + vi.spyOn(window, "scrollTo"); + const { getByTitle } = render( { const button = getByTitle("test-2-button"); fireEvent.click(button); + + // We expect that the hook has scrolled to the top of the page. + expect(window.scrollTo).toHaveBeenCalledWith(0, 0); + // And shows us a message. expect(wrapper).toMatchInlineSnapshot(`
Date: Fri, 27 Oct 2023 14:58:15 +0200 Subject: [PATCH 12/17] Add assertion for notification message will disappear --- src/tests/unit/notification-message.test.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tests/unit/notification-message.test.tsx b/src/tests/unit/notification-message.test.tsx index b419294333..117f631571 100644 --- a/src/tests/unit/notification-message.test.tsx +++ b/src/tests/unit/notification-message.test.tsx @@ -52,6 +52,7 @@ describe("useNotification hook", () => { it("Does shows a message if button is clicked", async () => { vi.spyOn(window, "scrollTo"); + vi.spyOn(window, "setTimeout"); const { getByTitle } = render( { // We expect that the hook has scrolled to the top of the page. expect(window.scrollTo).toHaveBeenCalledWith(0, 0); + // And the message is gone after 5 seconds. + expect(window.setTimeout).toHaveBeenCalledTimes(1); // And shows us a message. expect(wrapper).toMatchInlineSnapshot(`
Date: Sun, 29 Oct 2023 11:48:02 +0100 Subject: [PATCH 13/17] Refactor test and component for `useNotificationMessage` hook - Replaced custom prop-based component identification with data-testid attributes for better test clarity and maintainability. - Introduced afterEach cleanup in tests to ensure isolated test environments. - Updated test descriptions for consistency and better readability. - Simplified ComponentWithNotificationMessage by removing unnecessary props. --- src/tests/unit/notification-message.test.tsx | 68 +++++++++----------- 1 file changed, 30 insertions(+), 38 deletions(-) diff --git a/src/tests/unit/notification-message.test.tsx b/src/tests/unit/notification-message.test.tsx index 117f631571..b5762c0dc1 100644 --- a/src/tests/unit/notification-message.test.tsx +++ b/src/tests/unit/notification-message.test.tsx @@ -1,22 +1,17 @@ -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import React from "react"; -import { fireEvent, render } from "@testing-library/react"; +import { cleanup, fireEvent, render } from "@testing-library/react"; import { useNotificationMessage } from "../../core/utils/useNotificationMessage"; -const ComponentWithNotificationMessage = ({ - wrapperTitle, - buttonTitle -}: { - wrapperTitle: string; - buttonTitle: string; -}) => { +// Define a test component that utilizes the useNotificationMessage hook +const ComponentWithNotificationMessage = () => { const [NotificationMessage, handler] = useNotificationMessage(); return ( -
+