From 0a0ced65da520d15e1c42bc62f052a8d8428c9b2 Mon Sep 17 00:00:00 2001 From: Gabriel Dugny Date: Mon, 3 Mar 2025 16:23:41 +0100 Subject: [PATCH 1/4] feat(core-components): Avoid Layout Shift for DismissableBanner when using a storageApi with latency (e.g. user-settings-backend) Properly handle the `unknown` state of the storageApi. This may lead to some Layout Shift if the banner has not been dismissed, but once it has been dismissed, you won't have any. Signed-off-by: Gabriel Dugny --- .changeset/serious-chefs-provide.md | 7 ++++++ .../DismissableBanner/DismissableBanner.tsx | 23 +++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) create mode 100644 .changeset/serious-chefs-provide.md diff --git a/.changeset/serious-chefs-provide.md b/.changeset/serious-chefs-provide.md new file mode 100644 index 0000000000..fd84577330 --- /dev/null +++ b/.changeset/serious-chefs-provide.md @@ -0,0 +1,7 @@ +--- +'@backstage/core-components': patch +--- + +Avoid Layout Shift for DismissableBanner when using a storageApi with latency (e.g. user-settings-backend) + +Properly handle the `unknown` state of the storageApi. There's a trade-off: this may lead to some Layout Shift if the banner has not been dismissed, but once it has been dismissed, you won't have any. diff --git a/packages/core-components/src/components/DismissableBanner/DismissableBanner.tsx b/packages/core-components/src/components/DismissableBanner/DismissableBanner.tsx index 8c18a1feee..e3e423d166 100644 --- a/packages/core-components/src/components/DismissableBanner/DismissableBanner.tsx +++ b/packages/core-components/src/components/DismissableBanner/DismissableBanner.tsx @@ -102,11 +102,14 @@ export const DismissableBanner = (props: Props) => { const classes = useStyles(); const storageApi = useApi(storageApiRef); const notificationsStore = storageApi.forBucket('notifications'); - const rawDismissedBanners = - notificationsStore.snapshot('dismissedBanners').value ?? []; + const dismissedBannersSnapshot = + notificationsStore.snapshot('dismissedBanners'); const [dismissedBanners, setDismissedBanners] = useState( - new Set(rawDismissedBanners), + new Set(dismissedBannersSnapshot.value ?? []), + ); + const [loadingSettings, setLoadingSettings] = useState( + dismissedBannersSnapshot.presence === 'unknown', ); const observedItems = useObservable( @@ -114,11 +117,21 @@ export const DismissableBanner = (props: Props) => { ); useEffect(() => { + if (observedItems?.presence === 'unknown' || observedItems === undefined) { + setLoadingSettings(true); + } + if (observedItems?.value) { const currentValue = observedItems?.value ?? []; setDismissedBanners(new Set(currentValue)); } - }, [observedItems?.value]); + if ( + observedItems?.presence === 'absent' || + observedItems?.presence === 'present' + ) { + setLoadingSettings(false); + } + }, [observedItems]); const handleClick = () => { notificationsStore.set('dismissedBanners', [...dismissedBanners, id]); @@ -131,7 +144,7 @@ export const DismissableBanner = (props: Props) => { ? { vertical: 'bottom', horizontal: 'center' } : { vertical: 'top', horizontal: 'center' } } - open={!dismissedBanners.has(id)} + open={!loadingSettings && !dismissedBanners.has(id)} classes={{ root: classNames(classes.root, !fixed && classes.topPosition), }} From c66efe48bd8c15da8b07c6643875b82eba1485ee Mon Sep 17 00:00:00 2001 From: Gabriel Dugny Date: Tue, 11 Mar 2025 15:07:23 +0100 Subject: [PATCH 2/4] docs: Proper markdown in changeset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Signed-off-by: Gabriel Dugny --- .changeset/serious-chefs-provide.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/serious-chefs-provide.md b/.changeset/serious-chefs-provide.md index fd84577330..30d9adf15c 100644 --- a/.changeset/serious-chefs-provide.md +++ b/.changeset/serious-chefs-provide.md @@ -2,6 +2,6 @@ '@backstage/core-components': patch --- -Avoid Layout Shift for DismissableBanner when using a storageApi with latency (e.g. user-settings-backend) +Avoid Layout Shift for `DismissableBanner` when using a `storageApi` with latency (e.g. `user-settings-backend`) -Properly handle the `unknown` state of the storageApi. There's a trade-off: this may lead to some Layout Shift if the banner has not been dismissed, but once it has been dismissed, you won't have any. +Properly handle the `unknown` state of the `storageApi`. There's a trade-off: this may lead to some Layout Shift if the banner has not been dismissed, but once it has been dismissed, you won't have any. From db83ee04b92e7b3cc0645a8befb95bad54019e1f Mon Sep 17 00:00:00 2001 From: Gabriel Dugny Date: Wed, 12 Mar 2025 13:34:56 +0100 Subject: [PATCH 3/4] fix: useObservable for initialValue Signed-off-by: Gabriel Dugny --- .../DismissableBanner/DismissableBanner.tsx | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/core-components/src/components/DismissableBanner/DismissableBanner.tsx b/packages/core-components/src/components/DismissableBanner/DismissableBanner.tsx index e3e423d166..a1a8b9d91f 100644 --- a/packages/core-components/src/components/DismissableBanner/DismissableBanner.tsx +++ b/packages/core-components/src/components/DismissableBanner/DismissableBanner.tsx @@ -102,18 +102,16 @@ export const DismissableBanner = (props: Props) => { const classes = useStyles(); const storageApi = useApi(storageApiRef); const notificationsStore = storageApi.forBucket('notifications'); - const dismissedBannersSnapshot = - notificationsStore.snapshot('dismissedBanners'); - - const [dismissedBanners, setDismissedBanners] = useState( - new Set(dismissedBannersSnapshot.value ?? []), - ); - const [loadingSettings, setLoadingSettings] = useState( - dismissedBannersSnapshot.presence === 'unknown', - ); - const observedItems = useObservable( notificationsStore.observe$('dismissedBanners'), + notificationsStore.snapshot('dismissedBanners'), + ); + + const [dismissedBanners, setDismissedBanners] = useState( + new Set(observedItems.value ?? []), + ); + const [loadingSettings, setLoadingSettings] = useState( + observedItems.presence === 'unknown', ); useEffect(() => { From d2257b7f883d7b46a9f85551014db66052102880 Mon Sep 17 00:00:00 2001 From: Gabriel Dugny Date: Thu, 13 Mar 2025 09:49:10 +0100 Subject: [PATCH 4/4] fix: simplify loading state Signed-off-by: Gabriel Dugny --- .../DismissableBanner/DismissableBanner.tsx | 27 ++++--------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/packages/core-components/src/components/DismissableBanner/DismissableBanner.tsx b/packages/core-components/src/components/DismissableBanner/DismissableBanner.tsx index a1a8b9d91f..f65db08393 100644 --- a/packages/core-components/src/components/DismissableBanner/DismissableBanner.tsx +++ b/packages/core-components/src/components/DismissableBanner/DismissableBanner.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { ReactNode, useState, useEffect } from 'react'; +import React, { ReactNode, useMemo } from 'react'; import { useApi, storageApiRef } from '@backstage/core-plugin-api'; import useObservable from 'react-use/esm/useObservable'; import classNames from 'classnames'; @@ -107,29 +107,12 @@ export const DismissableBanner = (props: Props) => { notificationsStore.snapshot('dismissedBanners'), ); - const [dismissedBanners, setDismissedBanners] = useState( - new Set(observedItems.value ?? []), - ); - const [loadingSettings, setLoadingSettings] = useState( - observedItems.presence === 'unknown', + const dismissedBanners = useMemo( + () => new Set(observedItems.value ?? []), + [observedItems.value], ); - useEffect(() => { - if (observedItems?.presence === 'unknown' || observedItems === undefined) { - setLoadingSettings(true); - } - - if (observedItems?.value) { - const currentValue = observedItems?.value ?? []; - setDismissedBanners(new Set(currentValue)); - } - if ( - observedItems?.presence === 'absent' || - observedItems?.presence === 'present' - ) { - setLoadingSettings(false); - } - }, [observedItems]); + const loadingSettings = observedItems.presence === 'unknown'; const handleClick = () => { notificationsStore.set('dismissedBanners', [...dismissedBanners, id]);