Jdrush89/tree content visibility (#2640)

* Using content-visibility: auto on tree items * Improving typeahead perf * Add prop docs * Updating focus style * Adding changeset * Fixing linter errors * removing filter for typeahead Co-authored-by: Josh Black <joshblack@github.com>
primer · Dec 8, 2022 · a8f2289 · a8f2289
1 parent ac38cb2
commit a8f2289
Show file tree

Hide file tree

Showing 6 changed files with 120 additions and 44 deletions.
diff --git a/.changeset/swift-kiwis-sparkle.md b/.changeset/swift-kiwis-sparkle.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+TreeView: Add containIntrinsicSize prop and typeahead performance improvement
diff --git a/docs/content/TreeView.mdx b/docs/content/TreeView.mdx
@@ -300,6 +300,11 @@ See [Storybook](https://primer.style/react/storybook?path=/story/components-tree
     type="boolean"
     description="The expanded state of the item when it is initially rendered. Use when you do not need to control the state."
   />
+  <PropsTableRow
+    name="containIntrinsicSize"
+    type="string"
+    description="The size of this item's contents. Passing this will set 'content-visiblity: auto' on the content container, delaying rendering until the item is in the viewport."
+  />
   <PropsTableRow
     name="expanded"
     type="boolean"

diff --git a/src/TreeView/TreeView.features.stories.tsx b/src/TreeView/TreeView.features.stories.tsx
@@ -652,7 +652,7 @@ export const StressTest: Story = () => {
           Directory {i}
           <TreeView.SubTree>
             {Array.from({length: 100}).map((_, j) => (
-              <TreeView.Item key={i} id={`directory-${i}/file-${j}`}>
+              <TreeView.Item key={j} id={`directory-${i}/file-${j}`}>
                 <TreeView.LeadingVisual>
                   <FileIcon />
                 </TreeView.LeadingVisual>
@@ -670,4 +670,33 @@ StressTest.parameters = {
   chromatic: {disableSnapshot: true},
 }
 
+export const ContainIntrinsicSize: Story = () => {
+  return (
+    <TreeView aria-label="Files">
+      {Array.from({length: 10}).map((_, i) => (
+        <TreeView.Item key={i} id={`directory-${i}`} defaultExpanded containIntrinsicSize="2rem">
+          <TreeView.LeadingVisual>
+            <TreeView.DirectoryIcon />
+          </TreeView.LeadingVisual>
+          Directory {i}
+          <TreeView.SubTree>
+            {Array.from({length: 1000}).map((_, j) => (
+              <TreeView.Item key={j} id={`directory-${i}/file-${j}`} containIntrinsicSize="2rem">
+                <TreeView.LeadingVisual>
+                  <FileIcon />
+                </TreeView.LeadingVisual>
+                File {j}
+              </TreeView.Item>
+            ))}
+          </TreeView.SubTree>
+        </TreeView.Item>
+      ))}
+    </TreeView>
+  )
+}
+
+ContainIntrinsicSize.parameters = {
+  chromatic: {disableSnapshot: true},
+}
+
 export default meta
diff --git a/src/TreeView/TreeView.test.tsx b/src/TreeView/TreeView.test.tsx
@@ -198,6 +198,28 @@ describe('Markup', () => {
     await user.click(getByText(/Item 2/))
     expect(treeitem).not.toHaveAttribute('aria-expanded')
   })
+
+  it('should render with containIntrinsicSize', () => {
+    const {getByLabelText} = renderWithTheme(
+      <TreeView aria-label="Test tree">
+        <TreeView.Item id="parent" containIntrinsicSize="2rem" defaultExpanded>
+          Parent
+          <TreeView.SubTree>
+            <TreeView.Item containIntrinsicSize="2rem" id="child">
+              Child
+            </TreeView.Item>
+          </TreeView.SubTree>
+        </TreeView.Item>
+      </TreeView>,
+    )
+
+    // The test runner removes the contain-intrinsic-size and content-visibility
+    // properties, so we can only test that the elements are still rendering.
+    const childItem = getByLabelText(/Child/)
+    expect(childItem).toBeInTheDocument()
+    const parentItem = getByLabelText(/Parent/)
+    expect(parentItem).toBeInTheDocument()
+  })
 })
 
 describe('Keyboard interactions', () => {

diff --git a/src/TreeView/TreeView.tsx b/src/TreeView/TreeView.tsx
@@ -86,7 +86,8 @@ const UlBox = styled.ul<SxProp>`
   .PRIVATE_TreeView-item {
     outline: none;
 
-    &:focus-visible > div {
+    &:focus-visible > div,
+    &.focus-visible > div {
       box-shadow: inset 0 0 0 2px ${get(`colors.accent.fg`)};
       @media (forced-colors: active) {
         outline: 2px solid HighlightText;
@@ -293,6 +294,7 @@ Root.displayName = 'TreeView'
 export type TreeViewItemProps = {
   id: string
   children: React.ReactNode
+  containIntrinsicSize?: string
   current?: boolean
   defaultExpanded?: boolean
   expanded?: boolean
@@ -304,7 +306,16 @@ const {Slots, Slot} = createSlots(['LeadingVisual', 'TrailingVisual'])
 
 const Item = React.forwardRef<HTMLElement, TreeViewItemProps>(
   (
-    {id: itemId, current: isCurrentItem = false, defaultExpanded, expanded, onExpandedChange, onSelect, children},
+    {
+      id: itemId,
+      containIntrinsicSize,
+      current: isCurrentItem = false,
+      defaultExpanded,
+      expanded,
+      onExpandedChange,
+      onSelect,
+      children,
+    },
     ref,
   ) => {
     const {expandedStateCache} = React.useContext(RootContext)
@@ -408,6 +419,8 @@ const Item = React.forwardRef<HTMLElement, TreeViewItemProps>(
             style={{
               // @ts-ignore CSS custom property
               '--level': level,
+              contentVisibility: containIntrinsicSize ? 'auto' : undefined,
+              containIntrinsicSize,
             }}
             onClick={event => {
               if (onSelect) {

diff --git a/src/TreeView/useTypeahead.ts b/src/TreeView/useTypeahead.ts
@@ -8,7 +8,7 @@ type TypeaheadOptions = {
 }
 
 export function useTypeahead({containerRef, onFocusChange}: TypeaheadOptions) {
-  const [searchValue, setSearchValue] = React.useState('')
+  const searchValue = React.useRef('')
   const timeoutRef = React.useRef(0)
   const onFocusChangeRef = React.useRef(onFocusChange)
   const {safeSetTimeout, safeClearTimeout} = useSafeTimeout()
@@ -18,6 +18,44 @@ export function useTypeahead({containerRef, onFocusChange}: TypeaheadOptions) {
     onFocusChangeRef.current = onFocusChange
   }, [onFocusChange])
 
+  // Focus the closest element that matches the search value
+  const focusSearchValue = React.useCallback(
+    (searchValue: string) => {
+      // Don't change focus if the search value is empty
+      if (!searchValue) return
+
+      if (!containerRef.current) return
+      const container = containerRef.current
+
+      // Get focusable elements
+      const elements = Array.from(container.querySelectorAll('[role="treeitem"]'))
+
+      // Get the index of active element
+      const activeIndex = elements.findIndex(element => element === document.activeElement)
+
+      // Wrap the array elements such that the active descendant is at the beginning
+      let sortedElements = wrapArray(elements, activeIndex)
+
+      // Remove the active descendant from the beginning of the array
+      // when the user initiates a new search
+      if (searchValue.length === 1) {
+        sortedElements = sortedElements.slice(1)
+      }
+
+      // Find the first element that matches the search value
+      const nextElement = sortedElements.find(element => {
+        const name = getAccessibleName(element).toLowerCase()
+        return name.startsWith(searchValue.toLowerCase())
+      })
+
+      // If a match is found, focus it
+      if (nextElement) {
+        onFocusChangeRef.current(nextElement)
+      }
+    },
+    [containerRef],
+  )
+
   // Update the search value when the user types
   React.useEffect(() => {
     if (!containerRef.current) return
@@ -31,11 +69,12 @@ export function useTypeahead({containerRef, onFocusChange}: TypeaheadOptions) {
       if (event.ctrlKey || event.altKey || event.metaKey) return
 
       // Update the existing search value with the new key press
-      setSearchValue(value => value + event.key)
+      searchValue.current += event.key
+      focusSearchValue(searchValue.current)
 
       // Reset the timeout
       safeClearTimeout(timeoutRef.current)
-      timeoutRef.current = safeSetTimeout(() => setSearchValue(''), 300)
+      timeoutRef.current = safeSetTimeout(() => (searchValue.current = ''), 300)
 
       // Prevent default behavior
       event.preventDefault()
@@ -44,44 +83,7 @@ export function useTypeahead({containerRef, onFocusChange}: TypeaheadOptions) {
 
     container.addEventListener('keydown', onKeyDown)
     return () => container.removeEventListener('keydown', onKeyDown)
-  }, [containerRef, safeClearTimeout, safeSetTimeout])
-
-  // Update focus when the search value changes
-  React.useEffect(() => {
-    // Don't change focus if the search value is empty
-    if (!searchValue) return
-
-    if (!containerRef.current) return
-    const container = containerRef.current
-
-    // Get focusable elements
-    const elements = Array.from(container.querySelectorAll('[role="treeitem"]'))
-      // Filter out collapsed items
-      .filter(element => !element.parentElement?.closest('[role=treeitem][aria-expanded=false]'))
-
-    // Get the index of active element
-    const activeIndex = elements.findIndex(element => element === document.activeElement)
-
-    // Wrap the array elements such that the active descendant is at the beginning
-    let sortedElements = wrapArray(elements, activeIndex)
-
-    // Remove the active descendant from the beginning of the array
-    // when the user initiates a new search
-    if (searchValue.length === 1) {
-      sortedElements = sortedElements.slice(1)
-    }
-
-    // Find the first element that matches the search value
-    const nextElement = sortedElements.find(element => {
-      const name = getAccessibleName(element).toLowerCase()
-      return name.startsWith(searchValue.toLowerCase())
-    })
-
-    // If a match is found, focus it
-    if (nextElement) {
-      onFocusChangeRef.current(nextElement)
-    }
-  }, [searchValue, containerRef])
+  }, [containerRef, focusSearchValue, safeClearTimeout, safeSetTimeout])
 }
 
 /**