Skip to content

[Skip to content] Fix tabIndex issue #1821

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 12, 2019
Merged

[Skip to content] Fix tabIndex issue #1821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions UNRELEASED.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f

- Fixed a bug preventing the display of `Tooltip` when cursor enters from a disabled element ([#1783](https://github.com/Shopify/polaris-react/pull/1783)).
- Fixed React imports in the `Filters` component to use `import * as React` for projects that don't use `esModuleInterop` ([#1820](https://github.com/Shopify/polaris-react/pull/1820))
- Fixed `tabIndex` on `main` element causing event delegation issues ([#1821](https://github.com/Shopify/polaris-react/pull/1821))

### Documentation

Expand Down
21 changes: 15 additions & 6 deletions src/components/Frame/Frame.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export interface State {

export const GLOBAL_RIBBON_CUSTOM_PROPERTY = '--global-ribbon-height';
export const APP_FRAME_MAIN = 'AppFrameMain';
export const APP_FRAME_MAIN_ANCHOR_TARGET = 'AppFrameMainContent';
const APP_FRAME_NAV = 'AppFrameNav';
const APP_FRAME_TOP_BAR = 'AppFrameTopBar';
const APP_FRAME_LOADING_BAR = 'AppFrameLoadingBar';
Expand All @@ -72,7 +73,7 @@ class Frame extends React.PureComponent<CombinedProps, State> {

private globalRibbonContainer: HTMLDivElement | null = null;

private mainContentNode = React.createRef<HTMLDivElement>();
private skipoToMainContentTargetNode = React.createRef<HTMLAnchorElement>();

getChildContext(): FrameContext {
return {
Expand Down Expand Up @@ -214,7 +215,7 @@ class Frame extends React.PureComponent<CombinedProps, State> {
const skipMarkup = (
<div className={skipClassName}>
<a
href={`#${APP_FRAME_MAIN}`}
href={`#${APP_FRAME_MAIN_ANCHOR_TARGET}`}
onFocus={this.handleFocus}
onBlur={this.handleBlur}
onClick={this.handleClick}
Expand Down Expand Up @@ -246,6 +247,15 @@ class Frame extends React.PureComponent<CombinedProps, State> {
/>
) : null;

const skipToMainContentTarget = (
// eslint-disable-next-line jsx-a11y/anchor-is-valid
<a
id={APP_FRAME_MAIN_ANCHOR_TARGET}
ref={this.skipoToMainContentTargetNode}
Copy link
Contributor

@jgodson jgodson Jul 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skipoToMainContentTargetNode extra o in here (skipo)

tabIndex={-1}
/>
);

return (
<div
className={frameClassName}
Expand All @@ -264,9 +274,8 @@ class Frame extends React.PureComponent<CombinedProps, State> {
className={styles.Main}
id={APP_FRAME_MAIN}
data-has-global-ribbon={Boolean(globalRibbon)}
ref={this.mainContentNode}
tabIndex={-1}
>
{skipToMainContentTarget}
<div className={styles.Content}>{children}</div>
</main>
<ToastManager toastMessages={toastMessages} />
Expand Down Expand Up @@ -365,10 +374,10 @@ class Frame extends React.PureComponent<CombinedProps, State> {
};

private handleClick = () => {
if (this.mainContentNode.current == null) {
if (this.skipoToMainContentTargetNode.current == null) {
return;
}
this.mainContentNode.current.focus();
this.skipoToMainContentTargetNode.current.focus();
};

private handleNavigationDismiss = () => {
Expand Down
9 changes: 5 additions & 4 deletions src/components/Frame/tests/Frame.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,16 +143,17 @@ describe('<Frame />', () => {
it('renders a skip to content link with the proper text', () => {
const skipToContentLinkText = mountWithAppProvider(<Frame />)
.find('a')
.at(0)
.text();

expect(skipToContentLinkText).toStrictEqual('Skip to content');
});

it('sets focus to the <main> element when the skip to content link is clicked', () => {
it('sets focus to the main content target anchor element when the skip to content link is clicked', () => {
const frame = mountWithAppProvider(<Frame />);
const mainEl = frame.find('main');
trigger(frame.find('a'), 'onClick');
expect(mainEl.getDOMNode()).toBe(document.activeElement);
const mainAnchor = frame.find('main').find('a');
trigger(frame.find('a').at(0), 'onClick');
expect(mainAnchor.getDOMNode()).toBe(document.activeElement);
});

it('renders with a has nav data attribute when nav is passed', () => {
Expand Down