Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down Expand Up @@ -143,6 +144,7 @@ export type OptionSlots = {
// @public
export type OptionState = ComponentState<OptionSlots> & Pick<OptionProps, 'disabled'> & {
active: boolean;
focusVisible: boolean;
multiselect?: boolean;
selected: boolean;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLIn
selectOption,
selectedOptions,
setActiveOption,
setFocusVisible,
setOpen,
setValue,
value,
Expand Down Expand Up @@ -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 &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ const useInputStyles = makeStyles({
input: {
backgroundColor: tokens.colorTransparentBackground,
...shorthands.border('0'),
color: tokens.colorNeutralForeground1,
fontFamily: tokens.fontFamilyBase,

'&:focus': {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -86,6 +94,7 @@ export const useDropdown_unstable = (props: DropdownProps, ref: React.Ref<HTMLBu

const nextOption = getNextMatchingOption();
setActiveOption(nextOption);
setFocusVisible(true);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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',
},
Expand All @@ -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;
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe('Option', () => {

const defaultContextValues = {
activeOption: undefined,
focusVisible: false,
multiselect: false,
registerOption() {
return () => undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -118,6 +119,7 @@ export const useOption_unstable = (props: OptionProps, ref: React.Ref<HTMLElemen
}),
active,
disabled,
focusVisible,
multiselect,
selected,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export type ComboboxContextValue = Pick<
ComboboxState,
| 'activeOption'
| 'appearance'
| 'focusVisible'
| 'open'
| 'registerOption'
| 'selectedOptions'
Expand All @@ -21,6 +22,7 @@ export type ComboboxContextValue = Pick<
export const ComboboxContext = createContext<ComboboxContextValue>({
activeOption: undefined,
appearance: 'outline',
focusVisible: false,
open: false,
registerOption() {
return () => undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export function useComboboxContextValues(state: ComboboxBaseState): ComboboxBase
const {
activeOption,
appearance,
focusVisible,
open,
registerOption,
selectedOptions,
Expand All @@ -16,6 +17,7 @@ export function useComboboxContextValues(state: ComboboxBaseState): ComboboxBase
const combobox = {
activeOption,
appearance,
focusVisible,
open,
registerOption,
selectedOptions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -13,6 +21,7 @@ export function useListboxContextValues(state: ListboxState): ListboxContextValu

const listbox = {
activeOption,
focusVisible,
multiselect,
registerOption: registerOptionValue,
selectedOptions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -88,11 +92,13 @@ export const useComboboxBaseState = (props: ComboboxBaseProps) => {
...selectionState,
activeOption,
appearance,
focusVisible,
hasFocus,
ignoreNextBlur,
inlinePopup,
open,
setActiveOption,
setFocusVisible,
setHasFocus,
setOpen,
setValue,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export function useTriggerListboxSlots(
open,
selectOption,
setActiveOption,
setFocusVisible,
setHasFocus,
setOpen,
} = state;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -128,6 +128,7 @@ export function useTriggerListboxSlots(
switch (action) {
case 'Open':
event.preventDefault();
setFocusVisible(true);
setOpen(event, true);
break;
case 'Close':
Expand All @@ -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];
}