From 1ae86ab5fb5f4061e3192dc042f7d6d08a02bb8c Mon Sep 17 00:00:00 2001 From: Andre Wanlin <67169551+awanlin@users.noreply.github.com> Date: Fri, 2 Sep 2022 12:18:35 -0500 Subject: [PATCH 01/13] Added transient AlertMessage Signed-off-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com> --- .changeset/strong-rice-warn.md | 20 +++++++++++ packages/app/src/App.tsx | 2 +- packages/core-components/api-report.md | 1 + .../components/AlertDisplay/AlertDisplay.tsx | 36 +++++++++++++------ packages/core-plugin-api/api-report.md | 1 + .../src/apis/definitions/AlertApi.ts | 1 + 6 files changed, 49 insertions(+), 12 deletions(-) create mode 100644 .changeset/strong-rice-warn.md diff --git a/.changeset/strong-rice-warn.md b/.changeset/strong-rice-warn.md new file mode 100644 index 0000000000..a18a41633d --- /dev/null +++ b/.changeset/strong-rice-warn.md @@ -0,0 +1,20 @@ +--- +'@backstage/core-components': patch +'@backstage/core-plugin-api': patch +--- + +Added option to allow the `AlertMessage` to be self-closing. This is done with a new `transient` boolean that is set on the `AlertMessage`. The length of time that these transient message stay open for can be set using the `transientTimeoutMs` prop on the `AlertDisplay` in the `App.tsx`. Here is an example: + +```diff + const App = () => ( + ++ + + + {routes} + + + ); +``` + +The above example will set the transient timeout to 2500ms from the default of 5000ms diff --git a/packages/app/src/App.tsx b/packages/app/src/App.tsx index fd73e99bcb..05ef61c11a 100644 --- a/packages/app/src/App.tsx +++ b/packages/app/src/App.tsx @@ -280,7 +280,7 @@ const routes = ( const App = () => ( - + {routes} diff --git a/packages/core-components/api-report.md b/packages/core-components/api-report.md index 9ed964db44..6b6a4c1b6d 100644 --- a/packages/core-components/api-report.md +++ b/packages/core-components/api-report.md @@ -61,6 +61,7 @@ export type AlertDisplayProps = { vertical: 'top' | 'bottom'; horizontal: 'left' | 'center' | 'right'; }; + transientTimeoutMs?: number; }; // @public diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx index 83c11647bb..3b9065b5dd 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx @@ -22,16 +22,6 @@ import { Alert } from '@material-ui/lab'; import pluralize from 'pluralize'; import React, { useEffect, useState } from 'react'; -// TODO: improve on this and promote to a shared component for use by all apps. - -/** @public */ -export type AlertDisplayProps = { - anchorOrigin?: { - vertical: 'top' | 'bottom'; - horizontal: 'left' | 'center' | 'right'; - }; -}; - /** * Displays alerts from {@link @backstage/core-plugin-api#AlertApi} * @@ -40,11 +30,27 @@ export type AlertDisplayProps = { * * Shown as SnackBar at the center top of the page by default. Configurable with props. */ + +// TODO: improve on this and promote to a shared component for use by all apps. + +export type AlertDisplayProps = { + anchorOrigin?: { + vertical: 'top' | 'bottom'; + horizontal: 'left' | 'center' | 'right'; + }; + transientTimeoutMs?: number; +}; + +/** @public */ export function AlertDisplay(props: AlertDisplayProps) { const [messages, setMessages] = useState>([]); const alertApi = useApi(alertApiRef); - const { anchorOrigin = { vertical: 'top', horizontal: 'center' } } = props; + const { + anchorOrigin = { vertical: 'top', horizontal: 'center' }, + transientTimeoutMs, + } = props; + const timeoutMs = transientTimeoutMs ?? 5000; useEffect(() => { const subscription = alertApi @@ -56,6 +62,14 @@ export function AlertDisplay(props: AlertDisplayProps) { }; }, [alertApi]); + useEffect(() => { + const [current] = messages; + if (current && current.transient) + setTimeout(() => { + setMessages(msgs => msgs.filter(msg => msg !== current)); + }, timeoutMs); + }, [messages, timeoutMs]); + if (messages.length === 0) { return null; } diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index cebf09e908..3145f18e79 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -29,6 +29,7 @@ export const alertApiRef: ApiRef; export type AlertMessage = { message: string; severity?: 'success' | 'info' | 'warning' | 'error'; + transient?: boolean; }; // @public diff --git a/packages/core-plugin-api/src/apis/definitions/AlertApi.ts b/packages/core-plugin-api/src/apis/definitions/AlertApi.ts index 0c84f7c975..c9c731530f 100644 --- a/packages/core-plugin-api/src/apis/definitions/AlertApi.ts +++ b/packages/core-plugin-api/src/apis/definitions/AlertApi.ts @@ -26,6 +26,7 @@ export type AlertMessage = { message: string; // Severity will default to success since that is what material ui defaults the value to. severity?: 'success' | 'info' | 'warning' | 'error'; + transient?: boolean; }; /** From 66f74acaee7e55ac923aee95d5e139950150fc16 Mon Sep 17 00:00:00 2001 From: Andre Wanlin <67169551+awanlin@users.noreply.github.com> Date: Tue, 6 Sep 2022 09:36:02 -0500 Subject: [PATCH 02/13] Added example of using transient Signed-off-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com> --- .changeset/strong-rice-warn.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.changeset/strong-rice-warn.md b/.changeset/strong-rice-warn.md index a18a41633d..c4fc28180b 100644 --- a/.changeset/strong-rice-warn.md +++ b/.changeset/strong-rice-warn.md @@ -18,3 +18,18 @@ Added option to allow the `AlertMessage` to be self-closing. This is done with a ``` The above example will set the transient timeout to 2500ms from the default of 5000ms + +Here's a rough example of how to trigger an alert using the new `transient` boolean: + +```ts +import { alertApiRef, useApi } from '@backstage/core-plugin-api'; + +const exampleTransient = () => { + const alertApi = useApi(alertApiRef); + alertApi.post({ + message: 'Example of Transient Alert', + severity: 'success', + transient: true, + }); +}; +``` From 9777aa50987a5803afdde20eb6aa856141123472 Mon Sep 17 00:00:00 2001 From: Andre Wanlin <67169551+awanlin@users.noreply.github.com> Date: Mon, 24 Oct 2022 11:05:02 -0500 Subject: [PATCH 03/13] Changes to use display over transient property Signed-off-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com> --- .changeset/strong-rice-warn.md | 22 +++++++++---------- .../components/AlertDisplay/AlertDisplay.tsx | 2 +- packages/core-plugin-api/api-report.md | 2 +- .../src/apis/definitions/AlertApi.ts | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.changeset/strong-rice-warn.md b/.changeset/strong-rice-warn.md index c4fc28180b..30207e9d63 100644 --- a/.changeset/strong-rice-warn.md +++ b/.changeset/strong-rice-warn.md @@ -3,18 +3,18 @@ '@backstage/core-plugin-api': patch --- -Added option to allow the `AlertMessage` to be self-closing. This is done with a new `transient` boolean that is set on the `AlertMessage`. The length of time that these transient message stay open for can be set using the `transientTimeoutMs` prop on the `AlertDisplay` in the `App.tsx`. Here is an example: +Added an option to allow the `AlertMessage` to be self-closing. This is done with a new `display` property that is set to `transient` on the `AlertMessage`. The length of time that these transient message stay open for can be set using the `transientTimeoutMs` prop on the `AlertDisplay` in the `App.tsx`. Here is an example: ```diff - const App = () => ( - -+ - - - {routes} - - - ); + const App = () => ( + ++ + + + {routes} + + + ); ``` The above example will set the transient timeout to 2500ms from the default of 5000ms @@ -29,7 +29,7 @@ const exampleTransient = () => { alertApi.post({ message: 'Example of Transient Alert', severity: 'success', - transient: true, + display: 'transient', }); }; ``` diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx index 3b9065b5dd..75cb2c78de 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx @@ -64,7 +64,7 @@ export function AlertDisplay(props: AlertDisplayProps) { useEffect(() => { const [current] = messages; - if (current && current.transient) + if (current && current.display === 'transient') setTimeout(() => { setMessages(msgs => msgs.filter(msg => msg !== current)); }, timeoutMs); diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index 3145f18e79..dff4f7e5a6 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -29,7 +29,7 @@ export const alertApiRef: ApiRef; export type AlertMessage = { message: string; severity?: 'success' | 'info' | 'warning' | 'error'; - transient?: boolean; + display?: 'permanent' | 'transient'; }; // @public diff --git a/packages/core-plugin-api/src/apis/definitions/AlertApi.ts b/packages/core-plugin-api/src/apis/definitions/AlertApi.ts index c9c731530f..afdcb6a617 100644 --- a/packages/core-plugin-api/src/apis/definitions/AlertApi.ts +++ b/packages/core-plugin-api/src/apis/definitions/AlertApi.ts @@ -26,7 +26,7 @@ export type AlertMessage = { message: string; // Severity will default to success since that is what material ui defaults the value to. severity?: 'success' | 'info' | 'warning' | 'error'; - transient?: boolean; + display?: 'permanent' | 'transient'; }; /** From 3b55150fa8aaf54e6768b9d36922125e1c1975dd Mon Sep 17 00:00:00 2001 From: Andre Wanlin <67169551+awanlin@users.noreply.github.com> Date: Fri, 11 Nov 2022 14:34:38 -0600 Subject: [PATCH 04/13] Added TSDoc comments Signed-off-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com> --- .../src/components/AlertDisplay/AlertDisplay.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx index 75cb2c78de..e5b939fe27 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx @@ -29,6 +29,21 @@ import React, { useEffect, useState } from 'react'; * @remarks * * Shown as SnackBar at the center top of the page by default. Configurable with props. + * + * @param anchorOrigin.vertical - Vertical orientation of where the AlertDisplay will be located + * @param anchorOrigin.horizontal - Horizontal orientation of where the AlertDisplay will be located + * @param transientTimeoutMs - Number of milliseconds a transient alert will stay open for. Default value is 5000 + * + * @example + * + * // This example shows the default usage, the SnackBar will show up at the top in the center and any transient messages will stay open for 5000ms + * + * + * // With this example the SnackBar will show up in the bottom right hand corner and any transient messages will stay open for 2500ms + * + * + * // If you want to just set the time a transientTimeoutMs, you can do that like this: + * */ // TODO: improve on this and promote to a shared component for use by all apps. From 9a1864976a025d941f2a9de9bd384e5920c2e759 Mon Sep 17 00:00:00 2001 From: Andre Wanlin <67169551+awanlin@users.noreply.github.com> Date: Fri, 11 Nov 2022 14:35:33 -0600 Subject: [PATCH 05/13] Split up changeset Signed-off-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com> --- .changeset/chilled-ladybugs-cough.md | 20 ++++++++++++++++++++ .changeset/strong-rice-warn.md | 18 +----------------- 2 files changed, 21 insertions(+), 17 deletions(-) create mode 100644 .changeset/chilled-ladybugs-cough.md diff --git a/.changeset/chilled-ladybugs-cough.md b/.changeset/chilled-ladybugs-cough.md new file mode 100644 index 0000000000..fd5e3f02b0 --- /dev/null +++ b/.changeset/chilled-ladybugs-cough.md @@ -0,0 +1,20 @@ +--- +'@backstage/core-plugin-api': patch +--- + +Added a new `display` property to the `AlertMessage` which can accept the values `permanent` or `transient`. + +Here's a rough example of how to trigger an alert using the new `display` property: + +```ts +import { alertApiRef, useApi } from '@backstage/core-plugin-api'; + +const ExampleTransient = () => { + const alertApi = useApi(alertApiRef); + alertApi.post({ + message: 'Example of Transient Alert', + severity: 'success', + display: 'transient', + }); +}; +``` diff --git a/.changeset/strong-rice-warn.md b/.changeset/strong-rice-warn.md index 30207e9d63..da157ddaaf 100644 --- a/.changeset/strong-rice-warn.md +++ b/.changeset/strong-rice-warn.md @@ -1,9 +1,8 @@ --- '@backstage/core-components': patch -'@backstage/core-plugin-api': patch --- -Added an option to allow the `AlertMessage` to be self-closing. This is done with a new `display` property that is set to `transient` on the `AlertMessage`. The length of time that these transient message stay open for can be set using the `transientTimeoutMs` prop on the `AlertDisplay` in the `App.tsx`. Here is an example: +Added an option to allow the `AlertMessage` to be self-closing. This is done with a new `display` property that is set to `transient` on the `AlertMessage` when triggering a message to the `AlertApi`. The length of time that these transient messages stay open for can be set using the `transientTimeoutMs` prop on the `AlertDisplay` in the `App.tsx`. Here is an example: ```diff const App = () => ( @@ -18,18 +17,3 @@ Added an option to allow the `AlertMessage` to be self-closing. This is done wit ``` The above example will set the transient timeout to 2500ms from the default of 5000ms - -Here's a rough example of how to trigger an alert using the new `transient` boolean: - -```ts -import { alertApiRef, useApi } from '@backstage/core-plugin-api'; - -const exampleTransient = () => { - const alertApi = useApi(alertApiRef); - alertApi.post({ - message: 'Example of Transient Alert', - severity: 'success', - display: 'transient', - }); -}; -``` From 7fdf55dcddec529426ea0725e481772b557ff988 Mon Sep 17 00:00:00 2001 From: Andre Wanlin <67169551+awanlin@users.noreply.github.com> Date: Sat, 12 Nov 2022 15:12:13 -0600 Subject: [PATCH 06/13] Trying to fix up api-report Signed-off-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com> --- packages/core-components/api-report.md | 5 ++++- .../src/components/AlertDisplay/AlertDisplay.tsx | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/core-components/api-report.md b/packages/core-components/api-report.md index 6b6a4c1b6d..63c10f3387 100644 --- a/packages/core-components/api-report.md +++ b/packages/core-components/api-report.md @@ -55,7 +55,10 @@ import { WithStyles } from '@material-ui/core/styles'; // @public export function AlertDisplay(props: AlertDisplayProps): JSX.Element | null; -// @public (undocumented) +// Warning: (tsdoc-param-tag-with-invalid-name) The @param block should be followed by a valid parameter name: The identifier cannot non-word characters +// Warning: (tsdoc-param-tag-with-invalid-name) The @param block should be followed by a valid parameter name: The identifier cannot non-word characters +// +// @public export type AlertDisplayProps = { anchorOrigin?: { vertical: 'top' | 'bottom'; diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx index e5b939fe27..db90952104 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx @@ -35,7 +35,8 @@ import React, { useEffect, useState } from 'react'; * @param transientTimeoutMs - Number of milliseconds a transient alert will stay open for. Default value is 5000 * * @example - * + * Here's some examples: + * ``` * // This example shows the default usage, the SnackBar will show up at the top in the center and any transient messages will stay open for 5000ms * * @@ -44,6 +45,7 @@ import React, { useEffect, useState } from 'react'; * * // If you want to just set the time a transientTimeoutMs, you can do that like this: * + * ``` */ // TODO: improve on this and promote to a shared component for use by all apps. From d0dd3d5eb9cdfe421d0aa69a7c8a01d448ca3718 Mon Sep 17 00:00:00 2001 From: Andre Wanlin <67169551+awanlin@users.noreply.github.com> Date: Fri, 18 Nov 2022 14:03:34 -0600 Subject: [PATCH 07/13] Fixed API Report warnings Signed-off-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com> --- packages/core-components/api-report.md | 3 --- .../src/components/AlertDisplay/AlertDisplay.tsx | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/core-components/api-report.md b/packages/core-components/api-report.md index 63c10f3387..d8ea9d2bbe 100644 --- a/packages/core-components/api-report.md +++ b/packages/core-components/api-report.md @@ -55,9 +55,6 @@ import { WithStyles } from '@material-ui/core/styles'; // @public export function AlertDisplay(props: AlertDisplayProps): JSX.Element | null; -// Warning: (tsdoc-param-tag-with-invalid-name) The @param block should be followed by a valid parameter name: The identifier cannot non-word characters -// Warning: (tsdoc-param-tag-with-invalid-name) The @param block should be followed by a valid parameter name: The identifier cannot non-word characters -// // @public export type AlertDisplayProps = { anchorOrigin?: { diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx index db90952104..1f54abfe19 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx @@ -30,8 +30,8 @@ import React, { useEffect, useState } from 'react'; * * Shown as SnackBar at the center top of the page by default. Configurable with props. * - * @param anchorOrigin.vertical - Vertical orientation of where the AlertDisplay will be located - * @param anchorOrigin.horizontal - Horizontal orientation of where the AlertDisplay will be located + * @param anchorOrigin - The `vertical` property will set the vertical orientation of where the AlertDisplay will be located + * and the `horizontal` property will set the horizontal orientation of where the AlertDisplay will be located * @param transientTimeoutMs - Number of milliseconds a transient alert will stay open for. Default value is 5000 * * @example From e8ba2bc8f3428e4a9047f89bb2dd0351f9161628 Mon Sep 17 00:00:00 2001 From: Andre Wanlin <67169551+awanlin@users.noreply.github.com> Date: Fri, 18 Nov 2022 15:11:33 -0600 Subject: [PATCH 08/13] Added tests Signed-off-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com> --- .../AlertDisplay/AlertDisplay.test.tsx | 97 ++++++++++++++++++- .../components/AlertDisplay/AlertDisplay.tsx | 7 +- 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx index c366082b46..afccbd8812 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx @@ -17,10 +17,11 @@ import React from 'react'; import { screen } from '@testing-library/react'; import { AlertDisplay } from './AlertDisplay'; -import { alertApiRef } from '@backstage/core-plugin-api'; +import { alertApiRef, AlertMessage } from '@backstage/core-plugin-api'; import { AlertApiForwarder } from '@backstage/core-app-api'; import Observable from 'zen-observable'; import { renderInTestApp, TestApiProvider } from '@backstage/test-utils'; +import { act } from '@testing-library/react'; const TEST_MESSAGE = 'TEST_MESSAGE'; @@ -93,4 +94,98 @@ describe('', () => { expect(screen.getByText('(2 older messages)')).toBeInTheDocument(); }); }); + + describe('with transient message', () => { + const apis = [ + [ + alertApiRef, + { + post() {}, + alert$() { + return Observable.of({ + message: 'transient message one', + display: 'transient', + }); + }, + }, + ] as const, + ] as const; + + it('renders message and then removes it', async () => { + jest.useFakeTimers(); + const { queryByText } = await renderInTestApp( + + + , + ); + + expect(queryByText('transient message one')).toBeInTheDocument(); + act(() => { + jest.advanceTimersByTime(5005); + }); + expect(queryByText('transient message one')).not.toBeInTheDocument(); + jest.useRealTimers(); + }); + + it('respects transientTimeoutMs prop', async () => { + jest.useFakeTimers(); + const { queryByText } = await renderInTestApp( + + + , + ); + + expect(queryByText('transient message one')).toBeInTheDocument(); + act(() => { + jest.advanceTimersByTime(2505); + }); + expect(queryByText('transient message one')).not.toBeInTheDocument(); + jest.useRealTimers(); + }); + }); + + // What I'd like to do here is be able to trigger a few messages with a bit of time between each + // Then be able to check that they close one after the other, just not sure how to set that up? + describe('with multiple transient messages', () => { + const apis = [ + [ + alertApiRef, + { + post() {}, + alert$() { + return Observable.of( + { + message: 'transient message one', + display: 'transient', + }, + { + message: 'transient message two', + display: 'transient', + }, + { + message: 'transient message three', + display: 'transient', + }, + ); + }, + }, + ] as const, + ] as const; + + it('renders message and then removes it', async () => { + jest.useFakeTimers(); + const { queryByText } = await renderInTestApp( + + + , + ); + + expect(queryByText('transient message one')).toBeInTheDocument(); + act(() => { + jest.advanceTimersByTime(5005); + }); + expect(queryByText('transient message one')).not.toBeInTheDocument(); + jest.useRealTimers(); + }); + }); }); diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx index 1f54abfe19..e85887c174 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx @@ -81,10 +81,13 @@ export function AlertDisplay(props: AlertDisplayProps) { useEffect(() => { const [current] = messages; - if (current && current.display === 'transient') - setTimeout(() => { + if (current && current.display === 'transient') { + const timer = setTimeout(() => { setMessages(msgs => msgs.filter(msg => msg !== current)); }, timeoutMs); + return () => clearTimeout(timer); + } + return undefined; }, [messages, timeoutMs]); if (messages.length === 0) { From b0739c7e689b4cd861e7c7cab553f0b3869fd858 Mon Sep 17 00:00:00 2001 From: Andre Wanlin <67169551+awanlin@users.noreply.github.com> Date: Fri, 25 Nov 2022 10:57:26 -0600 Subject: [PATCH 09/13] Expanded tests Signed-off-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com> --- .../AlertDisplay/AlertDisplay.test.tsx | 148 +++++++++++++++--- 1 file changed, 122 insertions(+), 26 deletions(-) diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx index afccbd8812..bff018cc49 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx @@ -14,13 +14,14 @@ * limitations under the License. */ -import React from 'react'; -import { screen } from '@testing-library/react'; -import { AlertDisplay } from './AlertDisplay'; -import { alertApiRef, AlertMessage } from '@backstage/core-plugin-api'; +import { AlertMessage, alertApiRef } from '@backstage/core-plugin-api'; +import { TestApiProvider, renderInTestApp } from '@backstage/test-utils'; +import { fireEvent, screen } from '@testing-library/react'; + import { AlertApiForwarder } from '@backstage/core-app-api'; +import { AlertDisplay } from './AlertDisplay'; import Observable from 'zen-observable'; -import { renderInTestApp, TestApiProvider } from '@backstage/test-utils'; +import React from 'react'; import { act } from '@testing-library/react'; const TEST_MESSAGE = 'TEST_MESSAGE'; @@ -144,30 +145,33 @@ describe('', () => { }); }); - // What I'd like to do here is be able to trigger a few messages with a bit of time between each - // Then be able to check that they close one after the other, just not sure how to set that up? describe('with multiple transient messages', () => { + const alert$ = () => + new Observable(subscriber => { + subscriber.next({ + message: 'transient message one', + display: 'transient', + }); + setTimeout(() => { + subscriber.next({ + message: 'transient message two', + display: 'transient', + }); + }, 1000); + setTimeout(() => { + subscriber.next({ + message: 'transient message three', + display: 'transient', + }); + }, 2000); + }); + const apis = [ [ alertApiRef, { post() {}, - alert$() { - return Observable.of( - { - message: 'transient message one', - display: 'transient', - }, - { - message: 'transient message two', - display: 'transient', - }, - { - message: 'transient message three', - display: 'transient', - }, - ); - }, + alert$, }, ] as const, ] as const; @@ -179,12 +183,104 @@ describe('', () => { , ); - + // Validate adding messages expect(queryByText('transient message one')).toBeInTheDocument(); act(() => { - jest.advanceTimersByTime(5005); + jest.advanceTimersByTime(1000); }); - expect(queryByText('transient message one')).not.toBeInTheDocument(); + expect(queryByText('transient message one')).toBeInTheDocument(); + expect(queryByText('(1 older message)')).toBeInTheDocument(); + act(() => { + jest.advanceTimersByTime(1000); + }); + expect(queryByText('transient message one')).toBeInTheDocument(); + expect(queryByText('(2 older messages)')).toBeInTheDocument(); + + // Validate removing messages + act(() => { + jest.advanceTimersByTime(5000); + }); + expect(queryByText('transient message two')).toBeInTheDocument(); + expect(queryByText('(1 older message)')).toBeInTheDocument(); + + act(() => { + jest.advanceTimersByTime(5000); + }); + expect(queryByText('transient message three')).toBeInTheDocument(); + + act(() => { + jest.advanceTimersByTime(5000); + }); + expect(queryByText('transient message')).not.toBeInTheDocument(); + jest.useRealTimers(); + }); + }); + + describe('with multiple messages of mixed display', () => { + const alert$ = () => + new Observable(subscriber => { + subscriber.next({ + message: 'transient message one', + display: 'transient', + }); + setTimeout(() => { + subscriber.next({ + message: 'permanent message', + display: 'permanent', + }); + }, 1000); + setTimeout(() => { + subscriber.next({ + message: 'transient message three', + display: 'transient', + }); + }, 2000); + }); + + const apis = [ + [ + alertApiRef, + { + post() {}, + alert$, + }, + ] as const, + ] as const; + + it('renders message and then removes it', async () => { + jest.useFakeTimers(); + const { queryByText } = await renderInTestApp( + + + , + ); + // Validate adding messages + expect(queryByText('transient message one')).toBeInTheDocument(); + act(() => { + jest.advanceTimersByTime(1000); + }); + expect(queryByText('transient message one')).toBeInTheDocument(); + expect(queryByText('(1 older message)')).toBeInTheDocument(); + act(() => { + jest.advanceTimersByTime(1000); + }); + expect(queryByText('transient message one')).toBeInTheDocument(); + expect(queryByText('(2 older messages)')).toBeInTheDocument(); + + // Validate removing messages + act(() => { + jest.advanceTimersByTime(5000); + }); + expect(queryByText('permanent message')).toBeInTheDocument(); + expect(queryByText('(1 older message)')).toBeInTheDocument(); + + fireEvent.click(screen.getByTestId('error-button-close')); + expect(queryByText('transient message three')).toBeInTheDocument(); + + act(() => { + jest.advanceTimersByTime(5000); + }); + expect(queryByText('transient message')).not.toBeInTheDocument(); jest.useRealTimers(); }); }); From fe0d0a2ec7be76c4d9d1bc2954ac59de619f169b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 5 Dec 2022 14:22:44 +0100 Subject: [PATCH 10/13] core-components: handle overlapping display of transient alert messages Signed-off-by: Patrik Oldsberg --- .../AlertDisplay/AlertDisplay.test.tsx | 33 +++++++++++++++++++ .../components/AlertDisplay/AlertDisplay.tsx | 11 ++++--- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx index bff018cc49..d21adf1479 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx @@ -214,6 +214,39 @@ describe('', () => { expect(queryByText('transient message')).not.toBeInTheDocument(); jest.useRealTimers(); }); + + it('renders 3 different messages with overlapping timeout', async () => { + jest.useFakeTimers(); + const { queryByText } = await renderInTestApp( + + + , + ); + + // 3 messages stacked with 1.5s each, display times: 0-1.5, 1.5-3, 3-4.5 + + // 0s in, only message 1 + expect(queryByText('transient message one')).toBeInTheDocument(); + + // 1s in, message 1 still shown, message 2 added in background + act(() => jest.advanceTimersByTime(1000)); + expect(queryByText('transient message one')).toBeInTheDocument(); + expect(queryByText('(1 older message)')).toBeInTheDocument(); + + // 2s in, message 2 now shown, message 3 added + act(() => jest.advanceTimersByTime(1000)); + expect(queryByText('transient message two')).toBeInTheDocument(); + expect(queryByText('(1 older message)')).toBeInTheDocument(); + + // 3.5s in, message 3 now shown + act(() => jest.advanceTimersByTime(1500)); + expect(queryByText('transient message three')).toBeInTheDocument(); + + // 5s in, all messages gone + act(() => jest.advanceTimersByTime(1500)); + expect(queryByText('transient message three')).not.toBeInTheDocument(); + jest.useRealTimers(); + }); }); describe('with multiple messages of mixed display', () => { diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx index e85887c174..fe20b1d66d 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx @@ -19,6 +19,7 @@ import Snackbar from '@material-ui/core/Snackbar'; import Typography from '@material-ui/core/Typography'; import CloseIcon from '@material-ui/icons/Close'; import { Alert } from '@material-ui/lab'; +import { useIsMounted } from '@react-hookz/web'; import pluralize from 'pluralize'; import React, { useEffect, useState } from 'react'; @@ -62,6 +63,7 @@ export type AlertDisplayProps = { export function AlertDisplay(props: AlertDisplayProps) { const [messages, setMessages] = useState>([]); const alertApi = useApi(alertApiRef); + const isMounted = useIsMounted(); const { anchorOrigin = { vertical: 'top', horizontal: 'center' }, @@ -82,13 +84,14 @@ export function AlertDisplay(props: AlertDisplayProps) { useEffect(() => { const [current] = messages; if (current && current.display === 'transient') { - const timer = setTimeout(() => { - setMessages(msgs => msgs.filter(msg => msg !== current)); + setTimeout(() => { + if (isMounted()) { + setMessages(msgs => msgs.filter(msg => msg !== current)); + } }, timeoutMs); - return () => clearTimeout(timer); } return undefined; - }, [messages, timeoutMs]); + }, [messages, timeoutMs, isMounted]); if (messages.length === 0) { return null; From 64e29da6645a706230421482ce1a125a879b957c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 5 Dec 2022 14:39:53 +0100 Subject: [PATCH 11/13] core-components: handle overlapping display of transient alert messages with manual removal Signed-off-by: Patrik Oldsberg --- .../AlertDisplay/AlertDisplay.test.tsx | 37 +++++++++++++++++++ .../components/AlertDisplay/AlertDisplay.tsx | 21 +++++------ 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx index d21adf1479..b75f773f90 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.test.tsx @@ -247,6 +247,43 @@ describe('', () => { expect(queryByText('transient message three')).not.toBeInTheDocument(); jest.useRealTimers(); }); + + it('renders 3 different messages with overlapping timeout and manual removal', async () => { + jest.useFakeTimers(); + const { queryByText } = await renderInTestApp( + + + , + ); + + // 3 messages stacked with 1.5s each, display times: 0-1.5, 1.5-3, 3-4.5 + + // 0s in, only message 1 + expect(queryByText('transient message one')).toBeInTheDocument(); + + // 1s in, message 1 still shown, message 2 added in background + act(() => jest.advanceTimersByTime(1000)); + expect(queryByText('transient message one')).toBeInTheDocument(); + expect(queryByText('(1 older message)')).toBeInTheDocument(); + + // manually remove message 1 + fireEvent.click(screen.getByTestId('error-button-close')); + expect(screen.getByText('transient message two')).toBeInTheDocument(); + + // 2s in, message 2 now shown, message 3 added + act(() => jest.advanceTimersByTime(1000)); + expect(queryByText('transient message two')).toBeInTheDocument(); + expect(queryByText('(1 older message)')).toBeInTheDocument(); + + // 3s in, message 3 now shown + act(() => jest.advanceTimersByTime(1500)); + expect(queryByText('transient message three')).toBeInTheDocument(); + + // 4s in, all messages gone + act(() => jest.advanceTimersByTime(1500)); + expect(queryByText('transient message three')).not.toBeInTheDocument(); + jest.useRealTimers(); + }); }); describe('with multiple messages of mixed display', () => { diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx index fe20b1d66d..f1021b1f8b 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx @@ -19,7 +19,6 @@ import Snackbar from '@material-ui/core/Snackbar'; import Typography from '@material-ui/core/Typography'; import CloseIcon from '@material-ui/icons/Close'; import { Alert } from '@material-ui/lab'; -import { useIsMounted } from '@react-hookz/web'; import pluralize from 'pluralize'; import React, { useEffect, useState } from 'react'; @@ -63,7 +62,6 @@ export type AlertDisplayProps = { export function AlertDisplay(props: AlertDisplayProps) { const [messages, setMessages] = useState>([]); const alertApi = useApi(alertApiRef); - const isMounted = useIsMounted(); const { anchorOrigin = { vertical: 'top', horizontal: 'center' }, @@ -81,24 +79,25 @@ export function AlertDisplay(props: AlertDisplayProps) { }; }, [alertApi]); + const [firstMessage] = messages; + useEffect(() => { - const [current] = messages; - if (current && current.display === 'transient') { - setTimeout(() => { - if (isMounted()) { - setMessages(msgs => msgs.filter(msg => msg !== current)); - } + if (firstMessage && firstMessage.display === 'transient') { + const timeout = setTimeout(() => { + setMessages(msgs => { + const newMsgs = msgs.filter(msg => msg !== firstMessage); + return newMsgs.length === msgs.length ? msgs : newMsgs; + }); }, timeoutMs); + return () => clearTimeout(timeout); } return undefined; - }, [messages, timeoutMs, isMounted]); + }, [firstMessage, timeoutMs]); if (messages.length === 0) { return null; } - const [firstMessage] = messages; - const handleClose = () => { setMessages(msgs => msgs.filter(msg => msg !== firstMessage)); }; From abb8ee5951c0f8995f95eae5eb229b6442dfb905 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 5 Dec 2022 23:49:20 +0100 Subject: [PATCH 12/13] core-components: rearrage AlertDisplay docs Signed-off-by: Patrik Oldsberg --- .../components/AlertDisplay/AlertDisplay.tsx | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx index f1021b1f8b..b1c8273749 100644 --- a/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx +++ b/packages/core-components/src/components/AlertDisplay/AlertDisplay.tsx @@ -22,6 +22,19 @@ import { Alert } from '@material-ui/lab'; import pluralize from 'pluralize'; import React, { useEffect, useState } from 'react'; +/** + * Properties for {@link AlertDisplay} + * + * @public + */ +export type AlertDisplayProps = { + anchorOrigin?: { + vertical: 'top' | 'bottom'; + horizontal: 'left' | 'center' | 'right'; + }; + transientTimeoutMs?: number; +}; + /** * Displays alerts from {@link @backstage/core-plugin-api#AlertApi} * @@ -47,18 +60,6 @@ import React, { useEffect, useState } from 'react'; * * ``` */ - -// TODO: improve on this and promote to a shared component for use by all apps. - -export type AlertDisplayProps = { - anchorOrigin?: { - vertical: 'top' | 'bottom'; - horizontal: 'left' | 'center' | 'right'; - }; - transientTimeoutMs?: number; -}; - -/** @public */ export function AlertDisplay(props: AlertDisplayProps) { const [messages, setMessages] = useState>([]); const alertApi = useApi(alertApiRef); From 7d11560c44549ebeee1980282eb1f6ac7e0b4191 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 6 Dec 2022 13:38:35 +0100 Subject: [PATCH 13/13] Update .changeset/chilled-ladybugs-cough.md Signed-off-by: Patrik Oldsberg --- .changeset/chilled-ladybugs-cough.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/chilled-ladybugs-cough.md b/.changeset/chilled-ladybugs-cough.md index fd5e3f02b0..95a86e97c6 100644 --- a/.changeset/chilled-ladybugs-cough.md +++ b/.changeset/chilled-ladybugs-cough.md @@ -1,5 +1,5 @@ --- -'@backstage/core-plugin-api': patch +'@backstage/core-plugin-api': minor --- Added a new `display` property to the `AlertMessage` which can accept the values `permanent` or `transient`.