From e1dbf3c3615b73d93b108e3ebd491a8937ca1d10 Mon Sep 17 00:00:00 2001 From: Sarah Higley Date: Tue, 6 Sep 2022 14:54:27 -0700 Subject: [PATCH 1/7] add color style to input/button --- .../react-combobox/src/components/Combobox/useComboboxStyles.ts | 1 + .../react-combobox/src/components/Dropdown/useDropdownStyles.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/react-components/react-combobox/src/components/Combobox/useComboboxStyles.ts b/packages/react-components/react-combobox/src/components/Combobox/useComboboxStyles.ts index f268a78671ee6..60ed625a202cc 100644 --- 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/useDropdownStyles.ts b/packages/react-components/react-combobox/src/components/Dropdown/useDropdownStyles.ts index 33fa418500109..8da3124af80c8 100644 --- 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', From ccad200c52c168dcf96170755cb3690398a1cb4f Mon Sep 17 00:00:00 2001 From: Sarah Higley Date: Wed, 7 Sep 2022 12:56:00 -0700 Subject: [PATCH 2/7] update Combobox and Dropdown to only show option focus when navigating by keyboard --- .../src/components/Combobox/useCombobox.tsx | 3 ++ .../src/components/Listbox/Listbox.types.ts | 3 ++ .../src/components/Listbox/useListbox.ts | 22 ++++++++++++-- .../src/components/Option/Option.types.ts | 3 ++ .../src/components/Option/useOption.tsx | 2 ++ .../src/components/Option/useOptionStyles.ts | 4 +-- .../src/contexts/ComboboxContext.ts | 2 ++ .../src/contexts/ListboxContext.ts | 9 +++++- .../src/contexts/useComboboxContextValues.ts | 2 ++ .../src/contexts/useListboxContextValues.ts | 11 ++++++- .../src/utils/ComboboxBase.types.ts | 5 ++++ .../src/utils/useComboboxBaseState.ts | 6 ++++ .../src/utils/useTriggerListboxSlots.ts | 30 ++++++++++++++----- 13 files changed, 87 insertions(+), 15 deletions(-) diff --git a/packages/react-components/react-combobox/src/components/Combobox/useCombobox.tsx b/packages/react-components/react-combobox/src/components/Combobox/useCombobox.tsx index d5ff1d8c21455..5b10d720a8465 100644 --- 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 & /* 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 index b09e3697ba021..143186d6e2310 100644 --- 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(); + // 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) => { const action = getDropdownActionFromKey(event, { open: true }); const maxIndex = getCount() - 1; @@ -45,12 +49,18 @@ export const useListbox_unstable = (props: ListboxProps, ref: React.Ref) => { + 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 & /* 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 index ca99493a0f7c6..8bf86d69b00d3 100644 --- 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 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 { - 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 index 3c17fafcacc67..bc7559599cfb9 100644 --- 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({ 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 index ad44b28d9efaa..31b1ef4c995b2 100644 --- 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({ 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 index 278005bcc18b4..ffd4ed3d46ccc 100644 --- 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 index 2c8a0006defec..a9339ad2c577d 100644 --- 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 index 7c9a8c445a4a6..8dc75f57f4f95 100644 --- 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 { const [activeOption, setActiveOption] = React.useState(); + // 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 index c90f3f1e02fa2..79cdc6d66c1c0 100644 --- a/packages/react-components/react-combobox/src/utils/useTriggerListboxSlots.ts +++ b/packages/react-components/react-combobox/src/utils/useTriggerListboxSlots.ts @@ -1,4 +1,5 @@ import * as React from 'react'; +import { useKeyboardNavAttribute } from '@fluentui/react-tabster'; import { mergeCallbacks, useMergedRefs } from '@fluentui/react-utilities'; import type { ExtractSlotProps, Slot } from '@fluentui/react-utilities'; import { getDropdownActionFromKey, getIndexFromAction } from '../utils/dropdownKeyActions'; @@ -45,6 +46,7 @@ export function useTriggerListboxSlots( open, selectOption, setActiveOption, + setFocusVisible, setHasFocus, setOpen, } = state; @@ -52,11 +54,15 @@ export function useTriggerListboxSlots( // handle trigger focus/blur const triggerRef: typeof ref = React.useRef(null); + // apply focus visible attribute on listbox for active option styles + const listboxRef = useMergedRefs(listboxSlot?.ref, useKeyboardNavAttribute()); + // resolve listbox shorthand props const listbox: typeof listboxSlot = listboxSlot && { multiselect, tabIndex: undefined, ...listboxSlot, + ref: listboxRef, }; // resolve trigger shorthand props @@ -78,18 +84,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) => { + listbox.onClick = mergeCallbacks((event: React.MouseEvent) => { triggerRef.current?.focus(); + }, listbox.onClick); - onListboxClick?.(event); - }; + listbox.onMouseOver = mergeCallbacks((event: React.MouseEvent) => { + setFocusVisible(false); + }, listbox.onMouseOver); - listbox.onMouseDown = (event: React.MouseEvent) => { + listbox.onMouseDown = mergeCallbacks((event: React.MouseEvent) => { ignoreNextBlur.current = true; - - onListboxMouseDown?.(event); - }; + }, listbox.onMouseDown); } // the trigger should open/close the popup on click or blur @@ -128,6 +133,7 @@ export function useTriggerListboxSlots( switch (action) { case 'Open': event.preventDefault(); + setFocusVisible(true); setOpen(event, true); break; case 'Close': @@ -153,10 +159,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 & React.MouseEvent) => { + setFocusVisible(false); + }, + trigger.onMouseOver, + ); + return [trigger, listbox]; } From 3c2d540391f53144f2a70dce77bdf2a0902e43cc Mon Sep 17 00:00:00 2001 From: Sarah Higley Date: Wed, 7 Sep 2022 12:57:29 -0700 Subject: [PATCH 3/7] change file --- ...eact-combobox-4f9e8f26-0811-4f02-b45e-61f97b2dfbbe.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-combobox-4f9e8f26-0811-4f02-b45e-61f97b2dfbbe.json diff --git a/change/@fluentui-react-combobox-4f9e8f26-0811-4f02-b45e-61f97b2dfbbe.json b/change/@fluentui-react-combobox-4f9e8f26-0811-4f02-b45e-61f97b2dfbbe.json new file mode 100644 index 0000000000000..d512ef1b9d8f4 --- /dev/null +++ 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" +} From d09c9412e7fbdc906834cad4ed42788f02ffe700 Mon Sep 17 00:00:00 2001 From: Sarah Higley Date: Wed, 7 Sep 2022 13:04:05 -0700 Subject: [PATCH 4/7] update api file --- .../react-components/react-combobox/etc/react-combobox.api.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react-components/react-combobox/etc/react-combobox.api.md b/packages/react-components/react-combobox/etc/react-combobox.api.md index 7c7e09dfb6caf..4e71789dffe54 100644 --- 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 & 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 & Pick & { active: boolean; + focusVisible: boolean; multiselect?: boolean; selected: boolean; }; From b37664a04144d6a51133cca94ee748d071527e00 Mon Sep 17 00:00:00 2001 From: Sarah Higley Date: Wed, 7 Sep 2022 13:10:37 -0700 Subject: [PATCH 5/7] show focus for dropdown typing --- .../src/components/Dropdown/useDropdown.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/react-components/react-combobox/src/components/Dropdown/useDropdown.tsx b/packages/react-components/react-combobox/src/components/Dropdown/useDropdown.tsx index 3b8eab7952fff..cb9a4515aceea 100644 --- 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): 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 Date: Wed, 7 Sep 2022 14:12:47 -0700 Subject: [PATCH 6/7] remove accidental extra dep --- .../react-combobox/src/utils/useTriggerListboxSlots.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/react-components/react-combobox/src/utils/useTriggerListboxSlots.ts b/packages/react-components/react-combobox/src/utils/useTriggerListboxSlots.ts index 79cdc6d66c1c0..06b01b54c0714 100644 --- a/packages/react-components/react-combobox/src/utils/useTriggerListboxSlots.ts +++ b/packages/react-components/react-combobox/src/utils/useTriggerListboxSlots.ts @@ -1,5 +1,4 @@ import * as React from 'react'; -import { useKeyboardNavAttribute } from '@fluentui/react-tabster'; import { mergeCallbacks, useMergedRefs } from '@fluentui/react-utilities'; import type { ExtractSlotProps, Slot } from '@fluentui/react-utilities'; import { getDropdownActionFromKey, getIndexFromAction } from '../utils/dropdownKeyActions'; @@ -54,15 +53,11 @@ export function useTriggerListboxSlots( // handle trigger focus/blur const triggerRef: typeof ref = React.useRef(null); - // apply focus visible attribute on listbox for active option styles - const listboxRef = useMergedRefs(listboxSlot?.ref, useKeyboardNavAttribute()); - // resolve listbox shorthand props const listbox: typeof listboxSlot = listboxSlot && { multiselect, tabIndex: undefined, ...listboxSlot, - ref: listboxRef, }; // resolve trigger shorthand props From 79dc9cbb7b99d739f4f591da1f3d16a111e152ea Mon Sep 17 00:00:00 2001 From: Sarah Higley Date: Wed, 7 Sep 2022 19:28:08 -0700 Subject: [PATCH 7/7] fix option test --- .../react-combobox/src/components/Option/Option.test.tsx | 1 + 1 file changed, 1 insertion(+) 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 index 16445525e3aee..7a05bf547b281 100644 --- 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;