Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AC-2480::Flyout panels do not trap focus while open #3805

Merged
merged 16 commits into from
Jul 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { useStyle } from '../../classify';
import defaultClasses from './categoryBranch.module.css';

const Branch = props => {
const { category, setCategoryId } = props;
const { category, setCategoryId, tabindex } = props;
const { name } = category;
const classes = useStyle(defaultClasses, props.classes);

Expand All @@ -20,6 +20,7 @@ const Branch = props => {
return (
<li className={classes.root}>
<button
tabindex={tabindex}
className={classes.target}
data-cy="CategoryTree-Branch-target"
type="button"
Expand All @@ -44,5 +45,6 @@ Branch.propTypes = {
target: string,
text: string
}),
setCategoryId: func.isRequired
setCategoryId: func.isRequired,
tabindex: func.isRequired
};
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { useStyle } from '../../classify';
import defaultClasses from './categoryLeaf.module.css';

const Leaf = props => {
const { category, onNavigate, categoryUrlSuffix } = props;
const { category, onNavigate, categoryUrlSuffix, tabindex } = props;
const { name, url_path, children } = category;
const classes = useStyle(defaultClasses, props.classes);
const { handleClick } = useCategoryLeaf({ onNavigate });
Expand All @@ -35,6 +35,7 @@ const Leaf = props => {
data-cy="CategoryTree-Leaf-target"
to={destination}
onClick={handleClick}
tabIndex={tabindex}
>
<span className={classes.text}>{leafLabel}</span>
</Link>
Expand All @@ -55,5 +56,6 @@ Leaf.propTypes = {
text: string
}),
onNavigate: func.isRequired,
tabindex: func.isRequired,
categoryUrlSuffix: string
};
13 changes: 11 additions & 2 deletions packages/venia-ui/lib/components/CategoryTree/categoryTree.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ import Leaf from './categoryLeaf';
import defaultClasses from './categoryTree.module.css';

const Tree = props => {
const { categoryId, onNavigate, setCategoryId, updateCategories } = props;
const {
categoryId,
onNavigate,
setCategoryId,
updateCategories,
tabindex
} = props;

const talonProps = useCategoryTree({
categoryId,
Expand All @@ -29,12 +35,14 @@ const Tree = props => {
category={category}
onNavigate={onNavigate}
categoryUrlSuffix={categoryUrlSuffix}
tabindex={tabindex}
/>
) : (
<Branch
key={id}
category={category}
setCategoryId={setCategoryId}
tabindex={tabindex}
/>
);
})
Expand All @@ -57,5 +65,6 @@ Tree.propTypes = {
}),
onNavigate: func.isRequired,
setCategoryId: func.isRequired,
updateCategories: func.isRequired
updateCategories: func.isRequired,
tabindex: func.isRequired
};
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ jest.mock('../../Header/currencySwitcher', () => () => 'CurrencySwitcher');

jest.mock('@magento/peregrine/lib/talons/Navigation/useNavigation');

jest.mock('react-aria', () => ({
FocusScope: jest.fn(({ children }) => {
return children;
})
}));

jest.mock('../../Portal', () => ({
Portal: jest.fn(({ children }) => {
return children;
})
}));
/*
* Tests.
*/
Expand Down
69 changes: 39 additions & 30 deletions packages/venia-ui/lib/components/Navigation/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import StoreSwitcher from '../Header/storeSwitcher';
import LoadingIndicator from '../LoadingIndicator';
import NavHeader from './navHeader';
import defaultClasses from './navigation.module.css';

import { FocusScope } from 'react-aria';
import { Portal } from '../Portal';
const AuthModal = React.lazy(() => import('../AuthModal'));

const Navigation = props => {
Expand All @@ -35,6 +36,7 @@ const Navigation = props => {
const rootClassName = isOpen ? classes.root_open : classes.root;
const modalClassName = hasModal ? classes.modal_open : classes.modal;
const bodyClassName = hasModal ? classes.body_masked : classes.body;
const tabindex = isOpen ? '0' : '-1';

// Lazy load the auth modal because it may not be needed.
const authModal = hasModal ? (
Expand All @@ -52,35 +54,42 @@ const Navigation = props => {
) : null;

return (
<aside className={rootClassName}>
<header className={classes.header}>
<NavHeader
isTopLevel={isTopLevel}
onBack={handleBack}
view={view}
/>
</header>
<div className={bodyClassName}>
<CategoryTree
categoryId={categoryId}
onNavigate={handleClose}
setCategoryId={setCategoryId}
updateCategories={catalogActions.updateCategories}
/>
</div>
<div className={classes.footer}>
<div className={classes.switchers}>
<StoreSwitcher />
<CurrencySwitcher />
</div>
<AuthBar
disabled={hasModal}
showMyAccount={showMyAccount}
showSignIn={showSignIn}
/>
</div>
<div className={modalClassName}>{authModal}</div>
</aside>
<Portal>
{/* eslint-disable-next-line jsx-a11y/no-autofocus */}
<FocusScope contain={isOpen} restoreFocus autoFocus>
{/* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions */}
<aside className={rootClassName}>
<header className={classes.header}>
<NavHeader
isTopLevel={isTopLevel}
onBack={handleBack}
view={view}
/>
</header>
<div className={bodyClassName}>
<CategoryTree
categoryId={categoryId}
onNavigate={handleClose}
setCategoryId={setCategoryId}
updateCategories={catalogActions.updateCategories}
tabindex={tabindex}
/>
</div>
<div className={classes.footer}>
<div className={classes.switchers}>
<StoreSwitcher />
<CurrencySwitcher />
</div>
<AuthBar
disabled={hasModal}
showMyAccount={showMyAccount}
showSignIn={showSignIn}
/>
</div>
<div className={modalClassName}>{authModal}</div>
</aside>
</FocusScope>
</Portal>
);
};

Expand Down