Skip to content

Commit

Permalink
fix: Empty notifications height (#2856)
Browse files Browse the repository at this point in the history
Co-authored-by: Georgii Lobko <[email protected]>
  • Loading branch information
just-boris and georgylobko authored Oct 14, 2024
1 parent 09a1b44 commit 25f4435
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 37 deletions.
28 changes: 10 additions & 18 deletions pages/app-layout/utils/content-blocks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import range from 'lodash/range';
import BreadcrumbGroup from '~components/breadcrumb-group';
import Button from '~components/button';
import Container from '~components/container';
import Flashbar from '~components/flashbar';
import Flashbar, { FlashbarProps } from '~components/flashbar';
import Header from '~components/header';
import HelpPanel from '~components/help-panel';
import SideNavigation from '~components/side-navigation';
Expand Down Expand Up @@ -66,23 +66,15 @@ export function Navigation() {

export function Notifications() {
const [visible, setVisible] = useState(true);
if (!visible) {
return null;
}
return (
<Flashbar
items={[
{
type: 'success',
header: 'Success message',
statusIconAriaLabel: 'success',
dismissLabel: 'Dismiss notification',
dismissible: true,
onDismiss: () => setVisible(false),
},
]}
/>
);
const demoNotification: FlashbarProps.MessageDefinition = {
type: 'success',
header: 'Success message',
statusIconAriaLabel: 'success',
dismissLabel: 'Dismiss notification',
dismissible: true,
onDismiss: () => setVisible(false),
};
return <Flashbar items={visible ? [demoNotification] : []} />;
}

export function Footer({ legacyConsoleNav }: { legacyConsoleNav: boolean }) {
Expand Down
14 changes: 11 additions & 3 deletions pages/app-layout/with-notifications.page.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,33 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React from 'react';
import React, { useContext } from 'react';

import AppLayout from '~components/app-layout';

import AppContext, { AppContextType } from '../app/app-context';
import ScreenshotArea from '../utils/screenshot-area';
import { Notifications } from './utils/content-blocks';
import labels from './utils/labels';

import styles from './styles.scss';

type DemoPageContext = React.Context<
AppContextType<{
disableNotifications: boolean;
}>
>;

export default function () {
const { urlParams } = useContext(AppContext as DemoPageContext);
return (
<ScreenshotArea gutters={false}>
<AppLayout
ariaLabels={labels}
// Note: Rendering flashbar here directly does not work. It has to be a separate component to reproduce
// the case when `notifications` slot is not falsy, but eventually it renders no HTML
notifications={<Notifications />}
notifications={!urlParams.disableNotifications && <Notifications />}
content={
<div className={styles.highlightBorder}>
<div data-testid="content-root" className={styles.highlightBorder}>
<h1>Distribution details</h1>
<p>Some information</p>
</div>
Expand Down
30 changes: 21 additions & 9 deletions src/app-layout/__integ__/awsui-applayout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ const wrapper = createWrapper().findAppLayout();
const mobileSelector = `.${testutilStyles['mobile-bar']}`;

class AppLayoutPage extends BasePageObject {
async visit(url: string) {
await this.browser.url(url);
await this.waitForVisible(wrapper.findContentRegion().toSelector());
}

getNavPosition() {
return this.getBoundingBox(wrapper.findNavigation().findSideNavigation().findHeaderLink().toSelector());
}
Expand All @@ -24,14 +29,13 @@ class AppLayoutPage extends BasePageObject {

describe.each(['classic', 'refresh', 'refresh-toolbar'] as Theme[])('%s', theme => {
function setupTest(
{ viewport = viewports.desktop, pageName = 'default' },
{ viewport = viewports.desktop, pageName = 'default', extraParams = {} },
testFn: (page: AppLayoutPage) => Promise<void>
) {
return useBrowser(async browser => {
const page = new AppLayoutPage(browser);
await page.setWindowSize(viewport);
await browser.url(`#/light/app-layout/${pageName}?${getUrlParams(theme)}`);
await page.waitForVisible(wrapper.findContentRegion().toSelector());
await page.visit(`#/light/app-layout/${pageName}?${getUrlParams(theme, extraParams)}`);
await testFn(page);
});
}
Expand Down Expand Up @@ -165,13 +169,21 @@ describe.each(['classic', 'refresh', 'refresh-toolbar'] as Theme[])('%s', theme
);

test(
'does not render notifications slot when it is empty',
setupTest({ pageName: 'with-notifications' }, async page => {
const { height: originalHeight } = await page.getBoundingBox(wrapper.findNotifications().toSelector());
expect(originalHeight).toBeGreaterThan(0);
'maintains consistent content top offset between initially empty notifications and dynamically dismissed',
setupTest({ pageName: 'with-notifications', extraParams: { disableNotifications: 'true' } }, async page => {
// get reference offset on a page with notifications disabled
const { top: expectedOffset } = await page.getBoundingBox('[data-testid="content-root"]');
// visit the same page with notifications enabled to compare
await page.visit(
`#/light/app-layout/with-notifications?${getUrlParams(theme, { disableNotifications: 'false' })}`
);
await page.click(wrapper.findNotifications().findFlashbar().findItems().get(1).findDismissButton().toSelector());
const { height: newHeight } = await page.getBoundingBox(wrapper.findNotifications().toSelector());
expect(newHeight).toEqual(0);
await expect(page.isExisting(wrapper.findNotifications().findFlashbar().toSelector())).resolves.toBe(true);
await expect(
page.getElementsCount(wrapper.findNotifications().findFlashbar().findItems().toSelector())
).resolves.toBe(0);
const { top: contentTop } = await page.getBoundingBox('[data-testid="content-root"]');
expect(contentTop).toEqual(expectedOffset);
})
);
});
16 changes: 12 additions & 4 deletions src/app-layout/visual-refresh-toolbar/notifications/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import React, { useEffect, useRef } from 'react';
import clsx from 'clsx';

import { useContainerQuery } from '@cloudscape-design/component-toolkit';
import { useResizeObserver } from '@cloudscape-design/component-toolkit/internal';

import { highContrastHeaderClassName } from '../../../internal/utils/content-header-utils';
Expand All @@ -23,8 +24,9 @@ export function AppLayoutNotificationsImplementation({
children,
}: AppLayoutNotificationsImplementationProps) {
const { ariaLabels, stickyNotifications, setNotificationsHeight, verticalOffsets } = appLayoutInternals;
const ref = useRef<HTMLElement>(null);
useResizeObserver(ref, entry => setNotificationsHeight(entry.borderBoxHeight));
const [hasNotificationsContent, contentRef] = useContainerQuery(rect => rect.borderBoxHeight > 0);
const rootRef = useRef<HTMLDivElement>(null);
useResizeObserver(rootRef, entry => setNotificationsHeight(entry.borderBoxHeight));
useEffect(() => {
return () => {
setNotificationsHeight(0);
Expand All @@ -34,17 +36,23 @@ export function AppLayoutNotificationsImplementation({
}, []);
return (
<NotificationsSlot
ref={ref}
ref={rootRef}
className={clsx(
appLayoutInternals.headerVariant === 'high-contrast' && highContrastHeaderClassName,
stickyNotifications && styles['sticky-notifications'],
hasNotificationsContent && styles['has-notifications-content'],
appLayoutInternals.headerVariant !== 'high-contrast' && styles['sticky-notifications-with-background']
)}
style={{
insetBlockStart: stickyNotifications ? verticalOffsets.notifications : undefined,
}}
>
<div className={testutilStyles.notifications} role="region" aria-label={ariaLabels?.notifications}>
<div
ref={contentRef}
className={testutilStyles.notifications}
role="region"
aria-label={ariaLabels?.notifications}
>
{children}
</div>
</NotificationsSlot>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@
background-color: awsui.$color-background-layout-main;
}
}

.has-notifications-content {
padding-block-start: awsui.$space-scaled-xs;
}
3 changes: 0 additions & 3 deletions src/app-layout/visual-refresh-toolbar/skeleton/styles.scss
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,6 @@

.notifications-container {
grid-area: notifications;
&:not(:empty) {
padding-block-start: awsui.$space-scaled-xs;
}
}

.notifications-background {
Expand Down

0 comments on commit 25f4435

Please sign in to comment.