Skip to content

Commit

Permalink
Updated tab item focus to wait for the next frame (#12335)
Browse files Browse the repository at this point in the history
### WHY are these changes introduced?

Fixes #11829

### WHAT is this pull request doing?

Using the same pattern as [popover
overlay](https://github.com/Shopify/polaris/blob/main/polaris-react/src/components/Popover/components/PopoverOverlay/PopoverOverlay.tsx#L181)
to delay focusing to the next frame (for measuring/positioning)

### Giphy

#### Before


https://github.com/Shopify/polaris/assets/24610840/60bef8f1-d9a5-4043-b4f9-cbe70bfc3067

#### After


https://github.com/Shopify/polaris/assets/24610840/b2704fe6-5d88-465c-8b7c-95f71c502db6

### How to 🎩

Playground code ⬇️ 

```tsx
import React, {useState} from 'react';

import {Page, Box, Tabs} from '../src';

const tabs = [
  {content: 'Hello1', id: '1'},
  {content: 'Hello2', id: '2'},
  {content: 'Hello3', id: '3'},
  {content: 'Hello4', id: '4'},
  {content: 'Hello5', id: '5'},
  {content: 'Hello6', id: '6'},
  {content: 'Hello7', id: '7'},
  {content: 'Hello8', id: '8'},
  {content: 'Hello9', id: '9'},
  {content: 'Hello10', id: '10'},
  {content: 'Hello11', id: '11'},
  {content: 'Hello12', id: '12'},
  {content: 'Hello13', id: '13'},
  {content: 'Hello14', id: '14'},
  {content: 'Hello15', id: '15'},
];

export const Playground = {
  tags: ['skip-tests'],
  render() {
    const [selected, setSelected] = useState(0);

    return (
      <Page narrowWidth>
        <Box background="avatar-five-bg-fill" minHeight="4000px" />
        <Tabs tabs={tabs} selected={selected} onSelect={setSelected} />
      </Page>
    );
  },
};

```

### Notes

While tophatting I noticed some odd behavior with how the focus styles
are applied. We're using the [focus
visible](https://github.com/Shopify/polaris/blob/6ca51eb02b976cf894d7bb7165c7831b16c3bb6b/polaris-react/src/components/Tabs/Tabs.module.css#L216)
selector, but manually sending focus. This is causing the browser to
sometimes not display the focus ring - I noticed it the most on firefox.

### 🎩 checklist

- [ ] Tested a
[snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases)
- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
  • Loading branch information
AndrewMusgrave authored Jul 9, 2024
1 parent 255fbf5 commit d71b3a2
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/good-terms-laugh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Fixed `Tabs` automatically closing when opened in a scrolled position
12 changes: 6 additions & 6 deletions polaris-react/src/components/Tabs/components/Item/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ export const Item = memo(function Item({
const focusedNode = useRef<HTMLButtonElement | ReactElement | null>(null);

useEffect(() => {
if (
focusedNode.current &&
focusedNode.current instanceof HTMLElement &&
focused
) {
focusedNode.current.focus();
const focusTarget = focusedNode.current;

if (focusTarget && focusTarget instanceof HTMLElement && focused) {
requestAnimationFrame(() => {
focusTarget.focus();
});
}
}, [focusedNode, focused]);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import {timer} from '@shopify/jest-dom-mocks';
import {mountWithApp} from 'tests/utilities';

import {UnstyledLink} from '../../../../UnstyledLink';
Expand All @@ -10,6 +11,14 @@ describe('<Item />', () => {
focused: false,
};

beforeEach(() => {
timer.mock();
});

afterEach(() => {
timer.restore();
});

it('renders UnstyledLink when item has url', () => {
const url = 'http://shopify.com';

Expand All @@ -25,4 +34,12 @@ describe('<Item />', () => {
expect(item).not.toContainReactComponent(UnstyledLink);
expect(item).toContainReactComponent('button');
});

it('focuses itself when focused is true', () => {
const item = mountWithApp(<Item {...mockProps} focused />);

timer.runAllTimers();

expect(document.activeElement).toStrictEqual(item.find('button')!.domNode);
});
});

0 comments on commit d71b3a2

Please sign in to comment.