From d68f5ff671805b24f0917379901c93bebafded5d Mon Sep 17 00:00:00 2001 From: Cole Bemis Date: Mon, 24 Oct 2022 16:24:52 -0700 Subject: [PATCH] TreeView: Fix scroll behavior when focus changes (#2464) * Avoid unecessary style recalculation * Add stress test story * Only render subtree if it's expanded * Don't expand current item by default * Update controlled story * Create popular-taxis-yawn.md * Fix scroll behavior on focus * Create olive-sloths-count.md * Update src/TreeView/TreeView.tsx * Fix tests * Update snapshot --- .changeset/olive-sloths-count.md | 5 +++++ package-lock.json | 14 +++++++------- package.json | 2 +- src/TreeView/TreeView.test.tsx | 3 +++ src/TreeView/TreeView.tsx | 6 +++++- src/TreeView/useRovingTabIndex.ts | 1 + src/__tests__/hooks/useAnchoredPosition.test.tsx | 1 + 7 files changed, 23 insertions(+), 9 deletions(-) create mode 100644 .changeset/olive-sloths-count.md diff --git a/.changeset/olive-sloths-count.md b/.changeset/olive-sloths-count.md new file mode 100644 index 00000000000..c39b1dda537 --- /dev/null +++ b/.changeset/olive-sloths-count.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +TreeView: Fix scroll behavior when focus changes diff --git a/package-lock.json b/package-lock.json index 6ca83739e72..6f0c40b2120 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,7 @@ "@github/combobox-nav": "^2.1.5", "@github/markdown-toolbar-element": "^2.1.0", "@github/paste-markdown": "^1.4.0", - "@primer/behaviors": "^1.1.1", + "@primer/behaviors": "1.3.0", "@primer/octicons-react": "^17.7.0", "@primer/primitives": "7.10.0", "@react-aria/ssr": "^3.1.0", @@ -6103,9 +6103,9 @@ } }, "node_modules/@primer/behaviors": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/@primer/behaviors/-/behaviors-1.1.1.tgz", - "integrity": "sha512-wvF1PYjyxKNTr6+5w4uR5Gkz53t1fsRDgKjWxDKk7wmlh0cwiILBo4dDFjjVhWRF1mBSjaIxxJGB4WGaP7ct2Q==" + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@primer/behaviors/-/behaviors-1.3.0.tgz", + "integrity": "sha512-odndV9KERhRBRGA0Ir+kdr2CNYf6JzBvJ3Ne2Zj7pMMQox2713ME11Uw3JCYyt1STPxKtZqX2RrRcnZRbJRoKw==" }, "node_modules/@primer/octicons-react": { "version": "17.7.0", @@ -47573,9 +47573,9 @@ } }, "@primer/behaviors": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/@primer/behaviors/-/behaviors-1.1.1.tgz", - "integrity": "sha512-wvF1PYjyxKNTr6+5w4uR5Gkz53t1fsRDgKjWxDKk7wmlh0cwiILBo4dDFjjVhWRF1mBSjaIxxJGB4WGaP7ct2Q==" + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@primer/behaviors/-/behaviors-1.3.0.tgz", + "integrity": "sha512-odndV9KERhRBRGA0Ir+kdr2CNYf6JzBvJ3Ne2Zj7pMMQox2713ME11Uw3JCYyt1STPxKtZqX2RrRcnZRbJRoKw==" }, "@primer/octicons-react": { "version": "17.7.0", diff --git a/package.json b/package.json index 53cc6274a9d..9a727ebbac0 100644 --- a/package.json +++ b/package.json @@ -82,7 +82,7 @@ "@github/combobox-nav": "^2.1.5", "@github/markdown-toolbar-element": "^2.1.0", "@github/paste-markdown": "^1.4.0", - "@primer/behaviors": "^1.1.1", + "@primer/behaviors": "1.3.0", "@primer/octicons-react": "^17.7.0", "@primer/primitives": "7.10.0", "@react-aria/ssr": "^3.1.0", diff --git a/src/TreeView/TreeView.test.tsx b/src/TreeView/TreeView.test.tsx index 4072d8955a2..114169259d0 100644 --- a/src/TreeView/TreeView.test.tsx +++ b/src/TreeView/TreeView.test.tsx @@ -15,6 +15,9 @@ function renderWithTheme( return render({ui}, options) } +// Mock `scrollIntoView` because it's not implemented in JSDOM +Element.prototype.scrollIntoView = jest.fn() + describe('Markup', () => { it('uses tree role', () => { const {queryByRole} = renderWithTheme( diff --git a/src/TreeView/TreeView.tsx b/src/TreeView/TreeView.tsx index 25c4ff34aa1..87b398acd17 100644 --- a/src/TreeView/TreeView.tsx +++ b/src/TreeView/TreeView.tsx @@ -202,9 +202,9 @@ const Item = React.forwardRef( trailingVisualId }} > + {/* @ts-ignore Box doesn't have type support for `ref` used in combination with `as` */} ( aria-expanded={isSubTreeEmpty ? undefined : isExpanded} aria-current={isCurrentItem ? 'true' : undefined} onKeyDown={handleKeyDown} + onFocus={event => { + // Scroll the first child into view when the item receives focus + event.currentTarget.firstElementChild?.scrollIntoView({block: 'nearest', inline: 'nearest'}) + }} sx={{ outline: 'none', '&:focus-visible > div': { diff --git a/src/TreeView/useRovingTabIndex.ts b/src/TreeView/useRovingTabIndex.ts index bc3bc3cc048..621c6fa3b0e 100644 --- a/src/TreeView/useRovingTabIndex.ts +++ b/src/TreeView/useRovingTabIndex.ts @@ -6,6 +6,7 @@ export function useRovingTabIndex({containerRef}: {containerRef: React.RefObject useFocusZone({ containerRef, bindKeys: FocusKeys.ArrowVertical | FocusKeys.ArrowHorizontal | FocusKeys.HomeAndEnd, + preventScroll: true, getNextFocusable: (direction, from, event) => { if (!(from instanceof HTMLElement)) return diff --git a/src/__tests__/hooks/useAnchoredPosition.test.tsx b/src/__tests__/hooks/useAnchoredPosition.test.tsx index eb64b9d953a..2df3ae72cc2 100644 --- a/src/__tests__/hooks/useAnchoredPosition.test.tsx +++ b/src/__tests__/hooks/useAnchoredPosition.test.tsx @@ -23,6 +23,7 @@ it('should should return a position', () => { expect(cb).toHaveBeenCalledTimes(2) expect(cb.mock.calls[1][0]['position']).toMatchInlineSnapshot(` { + "anchorAlign": "start", "anchorSide": "outside-bottom", "left": 0, "top": 4,