fix: react-combobox Option focus outline only shows with keyboard nav (…

…#24700)
microsoft · Sep 12, 2022 · 58026c9 · 58026c9
1 parent cd8f5a1
commit 58026c9
Show file tree

Hide file tree

Showing 19 changed files with 104 additions and 16 deletions.
diff --git a/change/@fluentui-react-combobox-4f9e8f26-0811-4f02-b45e-61f97b2dfbbe.json b/change/@fluentui-react-combobox-4f9e8f26-0811-4f02-b45e-61f97b2dfbbe.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "fix: Combobox and Dropdown only show option focus outline when navigating by keyboard",
+  "packageName": "@fluentui/react-combobox",
+  "email": "sarah.higley@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/react-components/react-combobox/etc/react-combobox.api.md b/packages/react-components/react-combobox/etc/react-combobox.api.md
@@ -99,6 +99,7 @@ export type ListboxSlots = {
 // @public
 export type ListboxState = ComponentState<ListboxSlots> & OptionCollectionState & SelectionState & {
     activeOption?: OptionValue;
+    focusVisible: boolean;
     selectOption(event: SelectionEvents, option: OptionValue): void;
     setActiveOption(option?: OptionValue): void;
 };
@@ -143,6 +144,7 @@ export type OptionSlots = {
 // @public
 export type OptionState = ComponentState<OptionSlots> & Pick<OptionProps, 'disabled'> & {
     active: boolean;
+    focusVisible: boolean;
     multiselect?: boolean;
     selected: boolean;
 };

diff --git a/packages/react-components/react-combobox/src/components/Combobox/useCombobox.tsx b/packages/react-components/react-combobox/src/components/Combobox/useCombobox.tsx
@@ -37,6 +37,7 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLIn
     selectOption,
     selectedOptions,
     setActiveOption,
+    setFocusVisible,
     setOpen,
     setValue,
     value,
@@ -118,6 +119,8 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLIn
     const matchingOption = getOptionFromInput(inputValue);
     setActiveOption(matchingOption);
 
+    setFocusVisible(true);
+
     // clear selection for single-select if the input value no longer matches the selection
     if (
       !multiselect &&

diff --git a/packages/react-components/react-combobox/src/components/Combobox/useComboboxStyles.ts b/packages/react-components/react-combobox/src/components/Combobox/useComboboxStyles.ts
@@ -132,6 +132,7 @@ const useInputStyles = makeStyles({
   input: {
     backgroundColor: tokens.colorTransparentBackground,
     ...shorthands.border('0'),
+    color: tokens.colorNeutralForeground1,
     fontFamily: tokens.fontFamilyBase,
 
     '&:focus': {

diff --git a/packages/react-components/react-combobox/src/components/Dropdown/useDropdown.tsx b/packages/react-components/react-combobox/src/components/Dropdown/useDropdown.tsx
@@ -21,7 +21,15 @@ import type { DropdownProps, DropdownState } from './Dropdown.types';
  */
 export const useDropdown_unstable = (props: DropdownProps, ref: React.Ref<HTMLButtonElement>): DropdownState => {
   const baseState = useComboboxBaseState(props);
-  const { activeOption, getIndexOfId, getOptionsMatchingValue, open, setActiveOption, setOpen } = baseState;
+  const {
+    activeOption,
+    getIndexOfId,
+    getOptionsMatchingValue,
+    open,
+    setActiveOption,
+    setFocusVisible,
+    setOpen,
+  } = baseState;
 
   const { primary: triggerNativeProps, root: rootNativeProps } = getPartitionedNativeProps({
     props,
@@ -86,6 +94,7 @@ export const useDropdown_unstable = (props: DropdownProps, ref: React.Ref<HTMLBu
 
       const nextOption = getNextMatchingOption();
       setActiveOption(nextOption);
+      setFocusVisible(true);
     }
   };
 

diff --git a/packages/react-components/react-combobox/src/components/Dropdown/useDropdownStyles.ts b/packages/react-components/react-combobox/src/components/Dropdown/useDropdownStyles.ts
@@ -80,6 +80,7 @@ const useStyles = makeStyles({
     backgroundColor: tokens.colorTransparentBackground,
     ...shorthands.border('0'),
     boxSizing: 'border-box',
+    color: tokens.colorNeutralForeground1,
     columnGap: tokens.spacingHorizontalXXS,
     cursor: 'pointer',
     display: 'grid',

diff --git a/packages/react-components/react-combobox/src/components/Listbox/Listbox.types.ts b/packages/react-components/react-combobox/src/components/Listbox/Listbox.types.ts
@@ -22,6 +22,9 @@ export type ListboxState = ComponentState<ListboxSlots> &
     /* Option data for the currently highlighted option (not the selected option) */
     activeOption?: OptionValue;
 
+    // Whether the keyboard focus outline style should be visible
+    focusVisible: boolean;
+
     selectOption(event: SelectionEvents, option: OptionValue): void;
 
     setActiveOption(option?: OptionValue): void;

diff --git a/packages/react-components/react-combobox/src/components/Listbox/useListbox.ts b/packages/react-components/react-combobox/src/components/Listbox/useListbox.ts
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { getNativeElementProps } from '@fluentui/react-utilities';
+import { getNativeElementProps, mergeCallbacks, useEventCallback } from '@fluentui/react-utilities';
 import { useContextSelector, useHasParentContext } from '@fluentui/react-context-selector';
 import { useSelection } from '../../utils/useSelection';
 import { getDropdownActionFromKey, getIndexFromAction } from '../../utils/dropdownKeyActions';
@@ -26,6 +26,10 @@ export const useListbox_unstable = (props: ListboxProps, ref: React.Ref<HTMLElem
 
   const [activeOption, setActiveOption] = React.useState<OptionValue | undefined>();
 
+  // track whether keyboard focus outline should be shown
+  // tabster/keyborg doesn't work here, since the actual keyboard focus target doesn't move
+  const [focusVisible, setFocusVisible] = React.useState(false);
+
   const onKeyDown = (event: React.KeyboardEvent<HTMLElement>) => {
     const action = getDropdownActionFromKey(event, { open: true });
     const maxIndex = getCount() - 1;
@@ -45,12 +49,18 @@ export const useListbox_unstable = (props: ListboxProps, ref: React.Ref<HTMLElem
       // prevent default page scroll/keyboard action if the index changed
       event.preventDefault();
       setActiveOption(getOptionAtIndex(newIndex));
+      setFocusVisible(true);
     }
   };
 
+  const onMouseOver = (event: React.MouseEvent<HTMLElement>) => {
+    setFocusVisible(false);
+  };
+
   // get state from parent combobox, if it exists
   const hasComboboxContext = useHasParentContext(ComboboxContext);
   const comboboxActiveOption = useContextSelector(ComboboxContext, ctx => ctx.activeOption);
+  const comboboxFocusVisible = useContextSelector(ComboboxContext, ctx => ctx.focusVisible);
   const comboboxSelectedOptions = useContextSelector(ComboboxContext, ctx => ctx.selectedOptions);
   const comboboxSelectOption = useContextSelector(ComboboxContext, ctx => ctx.selectOption);
   const comboboxSetActiveOption = useContextSelector(ComboboxContext, ctx => ctx.setActiveOption);
@@ -59,18 +69,20 @@ export const useListbox_unstable = (props: ListboxProps, ref: React.Ref<HTMLElem
   const optionContextValues = hasComboboxContext
     ? {
         activeOption: comboboxActiveOption,
+        focusVisible: comboboxFocusVisible,
         selectedOptions: comboboxSelectedOptions,
         selectOption: comboboxSelectOption,
         setActiveOption: comboboxSetActiveOption,
       }
     : {
         activeOption,
+        focusVisible,
         selectedOptions,
         selectOption,
         setActiveOption,
       };
 
-  return {
+  const state: ListboxState = {
     components: {
       root: 'div',
     },
@@ -80,11 +92,15 @@ export const useListbox_unstable = (props: ListboxProps, ref: React.Ref<HTMLElem
       'aria-activedescendant': hasComboboxContext ? undefined : activeOption?.id,
       'aria-multiselectable': multiselect,
       tabIndex: 0,
-      onKeyDown,
       ...props,
     }),
     multiselect,
     ...optionCollection,
     ...optionContextValues,
   };
+
+  state.root.onKeyDown = useEventCallback(mergeCallbacks(state.root.onKeyDown, onKeyDown));
+  state.root.onMouseOver = useEventCallback(mergeCallbacks(state.root.onMouseOver, onMouseOver));
+
+  return state;
 };
diff --git a/packages/react-components/react-combobox/src/components/Option/Option.test.tsx b/packages/react-components/react-combobox/src/components/Option/Option.test.tsx
@@ -17,6 +17,7 @@ describe('Option', () => {
 
   const defaultContextValues = {
     activeOption: undefined,
+    focusVisible: false,
     multiselect: false,
     registerOption() {
       return () => undefined;

diff --git a/packages/react-components/react-combobox/src/components/Option/Option.types.ts b/packages/react-components/react-combobox/src/components/Option/Option.types.ts
@@ -33,6 +33,9 @@ export type OptionState = ComponentState<OptionSlots> &
     /* If true, this is the currently highlighted option */
     active: boolean;
 
+    // Whether the keyboard focus outline style should be visible
+    focusVisible: boolean;
+
     /* If true, the option is part of a multiselect combobox or listbox */
     multiselect?: boolean;
 

diff --git a/packages/react-components/react-combobox/src/components/Option/useOption.tsx b/packages/react-components/react-combobox/src/components/Option/useOption.tsx
@@ -46,6 +46,7 @@ export const useOption_unstable = (props: OptionProps, ref: React.Ref<HTMLElemen
   ]);
 
   // context values
+  const focusVisible = useContextSelector(ListboxContext, ctx => ctx.focusVisible);
   const multiselect = useContextSelector(ListboxContext, ctx => ctx.multiselect);
   const registerOption = useContextSelector(ListboxContext, ctx => ctx.registerOption);
   const selected = useContextSelector(ListboxContext, ctx => {
@@ -118,6 +119,7 @@ export const useOption_unstable = (props: OptionProps, ref: React.Ref<HTMLElemen
     }),
     active,
     disabled,
+    focusVisible,
     multiselect,
     selected,
   };

diff --git a/packages/react-components/react-combobox/src/components/Option/useOptionStyles.ts b/packages/react-components/react-combobox/src/components/Option/useOptionStyles.ts
@@ -113,12 +113,12 @@ const useStyles = makeStyles({
  * Apply styling to the Option slots based on the state
  */
 export const useOptionStyles_unstable = (state: OptionState): OptionState => {
-  const { active, disabled, multiselect, selected } = state;
+  const { active, disabled, focusVisible, multiselect, selected } = state;
   const styles = useStyles();
   state.root.className = mergeClasses(
     optionClassNames.root,
     styles.root,
-    active && styles.active,
+    active && focusVisible && styles.active,
     disabled && styles.disabled,
     selected && styles.selected,
     state.root.className,

diff --git a/packages/react-components/react-combobox/src/contexts/ComboboxContext.ts b/packages/react-components/react-combobox/src/contexts/ComboboxContext.ts
@@ -8,6 +8,7 @@ export type ComboboxContextValue = Pick<
   ComboboxState,
   | 'activeOption'
   | 'appearance'
+  | 'focusVisible'
   | 'open'
   | 'registerOption'
   | 'selectedOptions'
@@ -21,6 +22,7 @@ export type ComboboxContextValue = Pick<
 export const ComboboxContext = createContext<ComboboxContextValue>({
   activeOption: undefined,
   appearance: 'outline',
+  focusVisible: false,
   open: false,
   registerOption() {
     return () => undefined;

diff --git a/packages/react-components/react-combobox/src/contexts/ListboxContext.ts b/packages/react-components/react-combobox/src/contexts/ListboxContext.ts
@@ -6,12 +6,19 @@ import { ListboxState } from '../components/Listbox/Listbox.types';
  */
 export type ListboxContextValue = Pick<
   ListboxState,
-  'activeOption' | 'multiselect' | 'registerOption' | 'selectedOptions' | 'selectOption' | 'setActiveOption'
+  | 'activeOption'
+  | 'focusVisible'
+  | 'multiselect'
+  | 'registerOption'
+  | 'selectedOptions'
+  | 'selectOption'
+  | 'setActiveOption'
 >;
 
 // eslint-disable-next-line @fluentui/no-context-default-value
 export const ListboxContext = createContext<ListboxContextValue>({
   activeOption: undefined,
+  focusVisible: false,
   multiselect: false,
   registerOption() {
     return () => undefined;

diff --git a/packages/react-components/react-combobox/src/contexts/useComboboxContextValues.ts b/packages/react-components/react-combobox/src/contexts/useComboboxContextValues.ts
@@ -4,6 +4,7 @@ export function useComboboxContextValues(state: ComboboxBaseState): ComboboxBase
   const {
     activeOption,
     appearance,
+    focusVisible,
     open,
     registerOption,
     selectedOptions,
@@ -16,6 +17,7 @@ export function useComboboxContextValues(state: ComboboxBaseState): ComboboxBase
   const combobox = {
     activeOption,
     appearance,
+    focusVisible,
     open,
     registerOption,
     selectedOptions,

diff --git a/packages/react-components/react-combobox/src/contexts/useListboxContextValues.ts b/packages/react-components/react-combobox/src/contexts/useListboxContextValues.ts
@@ -4,7 +4,15 @@ import { ComboboxContext } from './ComboboxContext';
 
 export function useListboxContextValues(state: ListboxState): ListboxContextValues {
   const hasComboboxContext = useHasParentContext(ComboboxContext);
-  const { activeOption, multiselect, registerOption, selectedOptions, selectOption, setActiveOption } = state;
+  const {
+    activeOption,
+    focusVisible,
+    multiselect,
+    registerOption,
+    selectedOptions,
+    selectOption,
+    setActiveOption,
+  } = state;
 
   // get register/unregister functions from parent combobox context
   const comboboxRegisterOption = useContextSelector(ComboboxContext, ctx => ctx.registerOption);
@@ -13,6 +21,7 @@ export function useListboxContextValues(state: ListboxState): ListboxContextValu
 
   const listbox = {
     activeOption,
+    focusVisible,
     multiselect,
     registerOption: registerOptionValue,
     selectedOptions,

diff --git a/packages/react-components/react-combobox/src/utils/ComboboxBase.types.ts b/packages/react-components/react-combobox/src/utils/ComboboxBase.types.ts
@@ -77,6 +77,9 @@ export type ComboboxBaseState = Required<Pick<ComboboxBaseProps, 'appearance' |
     /* Option data for the currently highlighted option (not the selected option) */
     activeOption?: OptionValue;
 
+    // Whether the keyboard focus outline style should be visible
+    focusVisible: boolean;
+
     // whether the combobox/dropdown currently has focus
     hasFocus: boolean;
 
@@ -87,6 +90,8 @@ export type ComboboxBaseState = Required<Pick<ComboboxBaseProps, 'appearance' |
 
     setActiveOption(option?: OptionValue): void;
 
+    setFocusVisible(focusVisible: boolean): void;
+
     setHasFocus(hasFocus: boolean): void;
 
     setOpen(event: ComboboxBaseOpenEvents, newState: boolean): void;

diff --git a/packages/react-components/react-combobox/src/utils/useComboboxBaseState.ts b/packages/react-components/react-combobox/src/utils/useComboboxBaseState.ts
@@ -16,6 +16,10 @@ export const useComboboxBaseState = (props: ComboboxBaseProps) => {
 
   const [activeOption, setActiveOption] = React.useState<OptionValue | undefined>();
 
+  // track whether keyboard focus outline should be shown
+  // tabster/keyborg doesn't work here, since the actual keyboard focus target doesn't move
+  const [focusVisible, setFocusVisible] = React.useState(false);
+
   // track focused state to conditionally render collapsed listbox
   const [hasFocus, setHasFocus] = React.useState(false);
 
@@ -88,11 +92,13 @@ export const useComboboxBaseState = (props: ComboboxBaseProps) => {
     ...selectionState,
     activeOption,
     appearance,
+    focusVisible,
     hasFocus,
     ignoreNextBlur,
     inlinePopup,
     open,
     setActiveOption,
+    setFocusVisible,
     setHasFocus,
     setOpen,
     setValue,

diff --git a/packages/react-components/react-combobox/src/utils/useTriggerListboxSlots.ts b/packages/react-components/react-combobox/src/utils/useTriggerListboxSlots.ts
@@ -45,6 +45,7 @@ export function useTriggerListboxSlots(
     open,
     selectOption,
     setActiveOption,
+    setFocusVisible,
     setHasFocus,
     setOpen,
   } = state;
@@ -78,18 +79,17 @@ export function useTriggerListboxSlots(
      * 1. Move focus back to the button/input when the listbox is clicked (otherwise it goes to body)
      * 2. Do not close the listbox on button/input blur when clicking into the listbox
      */
-    const { onClick: onListboxClick, onMouseDown: onListboxMouseDown } = listbox;
-    listbox.onClick = (event: React.MouseEvent<HTMLDivElement>) => {
+    listbox.onClick = mergeCallbacks((event: React.MouseEvent<HTMLDivElement>) => {
       triggerRef.current?.focus();
+    }, listbox.onClick);
 
-      onListboxClick?.(event);
-    };
+    listbox.onMouseOver = mergeCallbacks((event: React.MouseEvent<HTMLDivElement>) => {
+      setFocusVisible(false);
+    }, listbox.onMouseOver);
 
-    listbox.onMouseDown = (event: React.MouseEvent<HTMLDivElement>) => {
+    listbox.onMouseDown = mergeCallbacks((event: React.MouseEvent<HTMLDivElement>) => {
       ignoreNextBlur.current = true;
-
-      onListboxMouseDown?.(event);
-    };
+    }, listbox.onMouseDown);
   }
 
   // the trigger should open/close the popup on click or blur
@@ -128,6 +128,7 @@ export function useTriggerListboxSlots(
       switch (action) {
         case 'Open':
           event.preventDefault();
+          setFocusVisible(true);
           setOpen(event, true);
           break;
         case 'Close':
@@ -153,10 +154,18 @@ export function useTriggerListboxSlots(
         // prevent default page scroll/keyboard action if the index changed
         event.preventDefault();
         setActiveOption(getOptionAtIndex(newIndex));
+        setFocusVisible(true);
       }
     },
     trigger.onKeyDown,
   );
 
+  trigger.onMouseOver = mergeCallbacks(
+    (event: React.MouseEvent<HTMLButtonElement> & React.MouseEvent<HTMLInputElement>) => {
+      setFocusVisible(false);
+    },
+    trigger.onMouseOver,
+  );
+
   return [trigger, listbox];
 }