Skip to content

Commit

Permalink
TreeView: Fix scroll behavior when focus changes (#2464)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
colebemis authored Oct 24, 2022
1 parent 175f3a7 commit d68f5ff
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 9 deletions.
5 changes: 5 additions & 0 deletions .changeset/olive-sloths-count.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

TreeView: Fix scroll behavior when focus changes
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 3 additions & 0 deletions src/TreeView/TreeView.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ function renderWithTheme(
return render(<ThemeProvider>{ui}</ThemeProvider>, options)
}

// Mock `scrollIntoView` because it's not implemented in JSDOM
Element.prototype.scrollIntoView = jest.fn()

describe('Markup', () => {
it('uses tree role', () => {
const {queryByRole} = renderWithTheme(
Expand Down
6 changes: 5 additions & 1 deletion src/TreeView/TreeView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,9 @@ const Item = React.forwardRef<HTMLElement, TreeViewItemProps>(
trailingVisualId
}}
>
{/* @ts-ignore Box doesn't have type support for `ref` used in combination with `as` */}
<Box
as="li"
// @ts-ignore Box doesn't have type support for `ref` used in combination with `as`
ref={ref}
tabIndex={0}
id={itemId}
Expand All @@ -215,6 +215,10 @@ const Item = React.forwardRef<HTMLElement, TreeViewItemProps>(
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': {
Expand Down
1 change: 1 addition & 0 deletions src/TreeView/useRovingTabIndex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions src/__tests__/hooks/useAnchoredPosition.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit d68f5ff

Please sign in to comment.