feat(useFocusZone): update active-descendant on mousemove (#1308)

Author: dgreif

.changeset/funny-birds-allow.md

@@ -0,0 +1,5 @@
+---
+"@primer/components": patch
+---
+
+Focus zones will now update active-descendant on `mousemove` over focusable elements.  ActionList has been updated to handle direct (key press) vs indirect (`mousemove`, DOM change, etc.) changes to active-descendant, and will use a distinct background color for the directly activated items.

src/ActionList/Item.tsx

@@ -8,7 +8,11 @@ import styled from 'styled-components'
 import {StyledHeader} from './Header'
 import {StyledDivider} from './Divider'
 import {useColorSchemeVar, useTheme} from '../ThemeProvider'
-import {uniqueId} from '../utils/uniqueId'
+import {
+  activeDescendantActivatedDirectly,
+  activeDescendantActivatedIndirectly,
+  isActiveDescendantAttribute
+} from '../behaviors/focusZone'
 
 /**
  * These colors are not yet in our default theme.  Need to remove this once they are added.
@@ -121,8 +125,6 @@ export interface ItemProps extends Omit<React.ComponentPropsWithoutRef<'div'>, '
   id?: number | string
 }
 
-export const itemActiveDescendantClass = `${uniqueId()}active-descendant`
-
 const getItemVariant = (variant = 'default', disabled?: boolean) => {
   if (disabled) {
     return {
@@ -176,10 +178,13 @@ const StyledItem = styled.div<
   display: flex;
   border-radius: ${get('radii.2')};
   color: ${({variant, item}) => getItemVariant(variant, item?.disabled).color};
+  // 2 frames on a 60hz monitor
+  transition: background 33.333ms linear;
 
   @media (hover: hover) and (pointer: fine) {
     :hover {
-      background: ${({hoverBackground}) => hoverBackground};
+      // allow override in case another item in the list is active/focused
+      background: var(--item-hover-bg-override, ${({hoverBackground}) => hoverBackground});
       cursor: ${({variant, item}) => getItemVariant(variant, item?.disabled).hoverCursor};
     }
   }
@@ -205,27 +210,41 @@ const StyledItem = styled.div<
   &:hover ${StyledItemContent}::before,
   // - below Hovered
   // '*' instead of '&' because '&' maps to separate class names depending on 'variant'
-  :hover + * ${StyledItemContent}::before,
+  :hover + * ${StyledItemContent}::before {
+    // allow override in case another item in the list is active/focused
+    border-color: var(--item-hover-divider-border-color-override, transparent) !important;
+  }
+
   // - above Focused
   &:focus ${StyledItemContent}::before,
   // - below Focused
   // '*' instead of '&' because '&' maps to separate class names depending on 'variant'
   :focus + * ${StyledItemContent}::before,
   // - above Active Descendent
-  &.${itemActiveDescendantClass} ${StyledItemContent}::before,
+  &[${isActiveDescendantAttribute}] ${StyledItemContent}::before,
   // - below Active Descendent
-  .${itemActiveDescendantClass} + & ${StyledItemContent}::before {
+  [${isActiveDescendantAttribute}] + & ${StyledItemContent}::before {
     // '!important' because all the ':not's above give higher specificity
     border-color: transparent !important;
   }
 
-  // Focused OR Active Descendant
-  &:focus,
-  &.${itemActiveDescendantClass} {
+  // Active Descendant
+  &[${isActiveDescendantAttribute}='${activeDescendantActivatedDirectly}'] {
+    background: ${({focusBackground}) => focusBackground};
+  }
+  &[${isActiveDescendantAttribute}='${activeDescendantActivatedIndirectly}'] {
+    background: ${({hoverBackground}) => hoverBackground};
+  }
+
+  &:focus {
     background: ${({focusBackground}) => focusBackground};
     outline: none;
   }
 
+  &:active {
+    background: ${({focusBackground}) => focusBackground};
+  }
+
   ${sx}
 `
 

src/ActionList/List.tsx

@@ -6,6 +6,7 @@ import {Divider} from './Divider'
 import styled from 'styled-components'
 import {get} from '../constants'
 import {SystemCssProperties} from '@styled-system/css'
+import {hasActiveDescendantAttribute} from '../behaviors/focusZone'
 
 export type ItemInput = ItemProps | (Partial<ItemProps> & {renderItem: typeof Item})
 
@@ -102,6 +103,11 @@ const StyledList = styled.div`
    * hardcoded '20px'
    */
   line-height: 20px;
+
+  &[${hasActiveDescendantAttribute}], &:focus-within {
+    --item-hover-bg-override: none;
+    --item-hover-divider-border-color-override: ${get('colors.selectMenu.borderSecondary')};
+  }
 `
 
 /**
@@ -132,7 +138,7 @@ function useListVariant(variant: ListProps['variant'] = 'inset'): {
 /**
  * Lists `Item`s, either grouped or ungrouped, with a `Divider` between each `Group`.
  */
-export function List(props: ListProps): JSX.Element {
+export const List = React.forwardRef<HTMLDivElement, ListProps>((props, forwardedRef): JSX.Element => {
   // Get `sx` prop values for `List` children matching the given `List` style variation.
   const {firstGroupStyle, lastGroupStyle, headerStyle, itemStyle} = useListVariant(props.variant)
 
@@ -216,7 +222,7 @@ export function List(props: ListProps): JSX.Element {
   }
 
   return (
-    <StyledList {...props}>
+    <StyledList {...props} ref={forwardedRef}>
       {groups.map(({header, ...groupProps}, index) => {
         const hasFilledHeader = header?.variant === 'filled'
         const shouldShowDivider = index > 0 && !hasFilledHeader
@@ -242,4 +248,6 @@ export function List(props: ListProps): JSX.Element {
       })}
     </StyledList>
   )
-}
+})
+
+List.displayName = 'ActionList'

src/FilteredActionList/FilteredActionList.tsx

@@ -7,7 +7,6 @@ import {ActionList} from '../ActionList'
 import Spinner from '../Spinner'
 import {useFocusZone} from '../hooks/useFocusZone'
 import {uniqueId} from '../utils/uniqueId'
-import {itemActiveDescendantClass} from '../ActionList/Item'
 import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
 import styled from 'styled-components'
 import {get} from '../constants'
@@ -71,6 +70,7 @@ export function FilteredActionList({
     [onFilterChange, setInternalFilterValue]
   )
 
+  const scrollContainerRef = useRef<HTMLDivElement>(null)
   const listContainerRef = useRef<HTMLDivElement>(null)
   const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
   const activeDescendantRef = useRef<HTMLElement>()
@@ -93,38 +93,26 @@ export function FilteredActionList({
     containerRef: listContainerRef,
     focusOutBehavior: 'wrap',
     focusableElementFilter: element => {
-      if (element instanceof HTMLInputElement) {
-        // No active-descendant focus on checkboxes in list items
-        return false
-      }
-      return true
+      return !(element instanceof HTMLInputElement)
     },
     activeDescendantFocus: inputRef,
-    onActiveDescendantChanged: (current, previous) => {
+    onActiveDescendantChanged: (current, previous, directlyActivated) => {
       activeDescendantRef.current = current
 
-      if (previous) {
-        previous.classList.remove(itemActiveDescendantClass)
-      }
-
-      if (current) {
-        current.classList.add(itemActiveDescendantClass)
-
-        if (listContainerRef.current) {
-          scrollIntoViewingArea(current, listContainerRef.current)
-        }
+      if (current && scrollContainerRef.current && directlyActivated) {
+        scrollIntoViewingArea(current, scrollContainerRef.current)
       }
     }
   })
 
   useEffect(() => {
     // if items changed, we want to instantly move active descendant into view
-    if (activeDescendantRef.current && listContainerRef.current) {
-      scrollIntoViewingArea(activeDescendantRef.current, listContainerRef.current, undefined, 'auto')
+    if (activeDescendantRef.current && scrollContainerRef.current) {
+      scrollIntoViewingArea(activeDescendantRef.current, scrollContainerRef.current, undefined, 'auto')
     }
   }, [items])
 
-  useScrollFlash(listContainerRef)
+  useScrollFlash(scrollContainerRef)
 
   return (
     <Flex flexDirection="column" overflow="hidden">
@@ -143,13 +131,13 @@ export function FilteredActionList({
           {...textInputProps}
         />
       </StyledHeader>
-      <Box ref={listContainerRef} overflow="auto">
+      <Box ref={scrollContainerRef} overflow="auto">
         {loading ? (
           <Box width="100%" display="flex" flexDirection="row" justifyContent="center" pt={6} pb={7}>
             <Spinner />
           </Box>
         ) : (
-          <ActionList items={items} {...listProps} role="listbox" id={listId} />
+          <ActionList ref={listContainerRef} items={items} {...listProps} role="listbox" id={listId} />
         )}
       </Box>
     </Flex>

src/__tests__/__snapshots__/ActionList.tsx.snap

@@ -11,6 +11,12 @@ exports[`ActionList renders consistently 1`] = `
   line-height: 20px;
 }
 
+.c0[data-has-active-descendant],
+.c0:focus-within {
+  --item-hover-bg-override: none;
+  --item-hover-divider-border-color-override: hsla(214,13%,93%,1);
+}
+
 <div
   className="c0"
 >

src/__tests__/__snapshots__/ActionMenu.tsx.snap

@@ -70,9 +70,9 @@ exports[`ActionMenu renders consistently 1`] = `
 <button
   aria-haspopup="listbox"
   aria-label="menu"
-  aria-labelledby="__primer_id_10001"
+  aria-labelledby="__primer_id_10000"
   className="c0"
-  id="__primer_id_10001"
+  id="__primer_id_10000"
   onClick={[Function]}
   onKeyDown={[Function]}
   tabIndex={0}

src/__tests__/__snapshots__/AnchoredOverlay.tsx.snap

@@ -69,9 +69,9 @@ exports[`AnchoredOverlay renders consistently 1`] = `
 
 <button
   aria-haspopup="listbox"
-  aria-labelledby="__primer_id_10001"
+  aria-labelledby="__primer_id_10000"
   className="c0"
-  id="__primer_id_10001"
+  id="__primer_id_10000"
   onClick={[Function]}
   onKeyDown={[Function]}
   tabIndex={0}

src/__tests__/__snapshots__/DropdownMenu.tsx.snap

@@ -73,9 +73,9 @@ exports[`DropdownMenu renders consistently 1`] = `
 
 <button
   aria-haspopup="listbox"
-  aria-labelledby="__primer_id_10001"
+  aria-labelledby="__primer_id_10000"
   className="c0"
-  id="__primer_id_10001"
+  id="__primer_id_10000"
   onClick={[Function]}
   onKeyDown={[Function]}
   tabIndex={0}

src/__tests__/__snapshots__/SelectPanel.tsx.snap

@@ -85,9 +85,9 @@ exports[`SelectPanel renders consistently 1`] = `
 >
   <button
     aria-haspopup="listbox"
-    aria-labelledby="__primer_id_10001"
+    aria-labelledby="__primer_id_10000"
     className="c1"
-    id="__primer_id_10001"
+    id="__primer_id_10000"
     onClick={[Function]}
     onKeyDown={[Function]}
     tabIndex={0}

src/__tests__/behaviors/focusZone.tsx

@@ -1,7 +1,7 @@
 import React from 'react'
-import {render} from '@testing-library/react'
+import {fireEvent, render} from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
-import {FocusKeys, focusZone} from '../../behaviors/focusZone'
+import {FocusKeys, focusZone, FocusZoneSettings} from '../../behaviors/focusZone'
 
 async function nextTick() {
   return new Promise(resolve => setTimeout(resolve, 0))
@@ -419,24 +419,49 @@ it('Should call onActiveDescendantChanged properly', () => {
     activeDescendantControl: control,
     onActiveDescendantChanged: activeDescendantChangedCallback
   })
+  type ActiveDescendantChangedCallbackParameters = Parameters<
+    Exclude<FocusZoneSettings['onActiveDescendantChanged'], undefined>
+  >
 
   control.focus()
-  userEvent.type(control, '{arrowdown}')
-  expect(activeDescendantChangedCallback).toHaveBeenCalledWith<[HTMLElement | undefined, HTMLElement | undefined]>(
+  expect(activeDescendantChangedCallback).toHaveBeenLastCalledWith<ActiveDescendantChangedCallbackParameters>(
     firstButton,
-    undefined
+    undefined,
+    false
   )
   userEvent.type(control, '{arrowdown}')
-  expect(activeDescendantChangedCallback).toHaveBeenCalledWith<[HTMLElement | undefined, HTMLElement | undefined]>(
+  expect(activeDescendantChangedCallback).toHaveBeenLastCalledWith<ActiveDescendantChangedCallbackParameters>(
+    secondButton,
+    firstButton,
+    true
+  )
+  userEvent.type(control, '{arrowup}')
+  expect(activeDescendantChangedCallback).toHaveBeenLastCalledWith<ActiveDescendantChangedCallbackParameters>(
+    firstButton,
+    secondButton,
+    true
+  )
+  fireEvent.mouseMove(secondButton)
+  expect(activeDescendantChangedCallback).toHaveBeenLastCalledWith<ActiveDescendantChangedCallbackParameters>(
     secondButton,
-    firstButton
+    firstButton,
+    false
   )
   userEvent.type(control, '{arrowup}')
+  expect(activeDescendantChangedCallback).toHaveBeenLastCalledWith<ActiveDescendantChangedCallbackParameters>(
+    firstButton,
+    secondButton,
+    true
+  )
   userEvent.type(control, '{arrowUp}')
-  expect(activeDescendantChangedCallback).toHaveBeenCalledWith<[HTMLElement | undefined, HTMLElement | undefined]>(
-    undefined,
-    firstButton
+  expect(activeDescendantChangedCallback).toHaveBeenLastCalledWith<ActiveDescendantChangedCallbackParameters>(
+    firstButton,
+    firstButton,
+    true
   )
+  activeDescendantChangedCallback.mockReset()
+  fireEvent.mouseMove(firstButton)
+  expect(activeDescendantChangedCallback).not.toBeCalled()
 
   controller.abort()
 })