Skip to content

Commit

Permalink
fix: Prevent scrollbar from overlapping side navigation content (#3002)
Browse files Browse the repository at this point in the history
  • Loading branch information
jperals authored Nov 14, 2024
1 parent f8fb30c commit 5343b83
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 30 deletions.
55 changes: 55 additions & 0 deletions pages/app-layout/navigation-with-scrollbar.page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React from 'react';

import AppLayout from '~components/app-layout';
import Select from '~components/select';
import SideNavigation, { SideNavigationProps } from '~components/side-navigation';

import labels from './utils/labels';

const items: SideNavigationProps.Item[] = new Array(50).fill(null).map((_, index) => ({
type: 'link',
text: `Link to page ${index + 1} with long enough text to wrap`,
href: '#',
}));

const itemsControl = (
<Select
options={[
{ value: 'option1', label: 'Option 1' },
{ value: 'option2', label: 'Option 2' },
]}
selectedOption={{ value: 'option1', label: 'Option 1' }}
onChange={() => null}
/>
);

export default function SideNavigationPage() {
const [open, setOpen] = React.useState(true);

return (
<AppLayout
navigationOpen={open}
onNavigationChange={({ detail }) => {
setOpen(detail.open);
}}
ariaLabels={labels}
navigation={
<SideNavigation
header={{
href: '#/',
text: 'Header title',
}}
items={items}
itemsControl={itemsControl}
/>
}
content={
<>
<h1>App Layout with scrollable Side navigation</h1>
</>
}
/>
);
}
66 changes: 66 additions & 0 deletions src/__integ__/use-browser-with-scrollbars.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import useBrowser from '@cloudscape-design/browser-test-tools/use-browser';

// Workaround until scrollbars are generally shown in tests (AWSUI-59983)

interface TestFunction {
(browser: WebdriverIO.Browser): Promise<void> | void;
}

const options = {
capabilities: {
'goog:chromeOptions': {
args: [
// Same array as in
// https://github.com/cloudscape-design/browser-test-tools/blob/4aaed9e410b13e05a7d5dbace17231776d250b97/src/browsers/capabilities.ts
// but without --hide-scrollbar.
'--disable-background-timer-throttling',
'--disable-breakpad',
'--disable-client-side-phishing-detection',
'--disable-cloud-import',
'--disable-default-apps',
'--disable-dev-shm-usage',
'--disable-extensions',
'--disable-gesture-typing',
'--disable-hang-monitor',
'--disable-infobars',
'--disable-notifications',
'--disable-offer-store-unmasked-wallet-cards',
'--disable-offer-upload-credit-cards',
'--disable-popup-blocking',
'--disable-print-preview',
'--disable-prompt-on-repost',
'--disable-setuid-sandbox',
'--disable-speech-api',
'--disable-sync',
'--disable-tab-for-desktop-share',
'--disable-translate',
'--disable-voice-input',
'--disable-wake-on-wifi',
'--disk-cache-size=33554432',
'--enable-async-dns',
'--enable-simple-cache-backend',
'--enable-tcp-fast-open',
'--enable-webgl',
'--ignore-gpu-blacklist',
'--media-cache-size=33554432',
'--metrics-recording-only',
'--mute-audio',
'--no-default-browser-check',
'--no-first-run',
'--no-pings',
'--no-zygote',
'--password-store=basic',
'--prerender-from-omnibox=disabled',
'--no-sandbox',
'--disable-gpu',
'--headless=new',
],
},
},
};

export default function (testFn: TestFunction): () => Promise<void> {
return useBrowser(options, testFn);
}
34 changes: 34 additions & 0 deletions src/app-layout/__integ__/navigation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import { BasePageObject } from '@cloudscape-design/browser-test-tools/page-objects';

import createWrapper from '../../../lib/components/test-utils/selectors';
import useBrowserWithScrollbars from '../../__integ__/use-browser-with-scrollbars';
import { getUrlParams, Theme } from './utils';

const wrapper = createWrapper().findAppLayout().findNavigation();

function setupTest(testFn: (page: BasePageObject) => Promise<void>, theme: Theme) {
return useBrowserWithScrollbars(async browser => {
const page = new BasePageObject(browser);
const params = getUrlParams(theme);
await browser.url(`#/light/app-layout/navigation-with-scrollbar?${params}`);
await page.waitForVisible(createWrapper().findSideNavigation().toSelector());
await testFn(page);
})();
}

describe('Navigation slot', () => {
describe('has expected inline size when a scrollbar is present', () => {
test.each(['classic', 'refresh-toolbar'] as const)('%s', theme =>
setupTest(async page => {
const { width } = await page.getBoundingBox(wrapper.toSelector());
expect(width).toBe(280);
const navigation = wrapper.findSideNavigation();
const navigationWidth = (await page.getBoundingBox(navigation.toSelector())).width;
expect(navigationWidth).toBeLessThan(280);
}, theme)
);
});
});
34 changes: 18 additions & 16 deletions src/app-layout/visual-refresh-toolbar/navigation/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,27 @@ export function AppLayoutNavigationImplementation({ appLayoutInternals }: AppLay
};

return (
<nav
aria-label={ariaLabels?.navigation ?? undefined}
className={clsx(
styles.navigation,
{
[styles['is-navigation-open']]: navigationOpen,
[testutilStyles['drawer-closed']]: !navigationOpen,
},
testutilStyles.navigation,
sharedStyles['with-motion-horizontal']
)}
aria-hidden={!navigationOpen}
onClick={onNavigationClick}
<div
className={clsx(styles['navigation-container'], sharedStyles['with-motion-horizontal'], {
[styles['is-navigation-open']]: navigationOpen,
})}
style={{
blockSize: drawerHeight,
insetBlockStart: drawerTopOffset,
}}
>
<div className={clsx(styles['content-container'], styles['animated-content'])}>
<nav
aria-label={ariaLabels?.navigation ?? undefined}
className={clsx(
styles.navigation,
{
[testutilStyles['drawer-closed']]: !navigationOpen,
},
testutilStyles.navigation
)}
aria-hidden={!navigationOpen}
onClick={onNavigationClick}
>
<div className={clsx(styles['hide-navigation'])}>
<InternalButton
ariaLabel={ariaLabels?.navigationClose ?? undefined}
Expand All @@ -76,8 +78,8 @@ export function AppLayoutNavigationImplementation({ appLayoutInternals }: AppLay
/>
</div>
{navigation}
</div>
</nav>
</nav>
</div>
);
}

Expand Down
24 changes: 10 additions & 14 deletions src/app-layout/visual-refresh-toolbar/navigation/styles.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,13 @@
@use '../../../internal/generated/custom-css-properties/index.scss' as custom-props;
@use '../../constants.scss' as constants;

.navigation {
// This wrapper clips the actual navigation content during the enter animation.
.navigation-container {
position: sticky;
z-index: constants.$drawer-z-index;
background-color: awsui.$color-background-container-content;
inset-block-end: 0;
block-size: 100%;
overflow-x: hidden;
overflow-y: auto;
/* stylelint-disable-next-line plugin/no-unsupported-browser-features */
overscroll-behavior-y: contain;
word-wrap: break-word;
pointer-events: auto;
display: flex;
Expand All @@ -30,17 +27,16 @@
display: none;
}

& > .content-container {
& > .navigation {
flex-grow: 1;
}

/*
A non-semantic node is added with a fixed width equal to the final Navigation
width. This will create the visual appearance of horizontal movement and
prevent unwanted text wrapping.
*/
> .animated-content {
block-size: 100%;
overflow-y: auto;
/* stylelint-disable-next-line plugin/no-unsupported-browser-features */
overscroll-behavior-y: contain;
// Needs to have a fixed width during the enter animation, in which it will be cropped by its parent.
inline-size: var(#{custom-props.$navigationWidth});
// Necessary for the hide-navigation button to be positioned correctly when a scrollbar is present.
position: relative;
}

// The Navigation drawer will take up the entire viewport on mobile
Expand Down

0 comments on commit 5343b83

Please sign in to comment.