Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A11y/combobox nav refactor and expose list focus and keyboard navigation hook #636

Merged
merged 21 commits into from
Apr 28, 2022
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ exports[`BreadcrumbsItem renders correctly when disabled item 1`] = `
<span
aria-disabled="true"
className="breadcrumb-content disabled"
tabIndex="0"
tabIndex="-1"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not relevant. for fixing tests.

>
<span
className="breadcrumb-text"
Expand Down
99 changes: 53 additions & 46 deletions src/components/Combobox/Combobox.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ import useMergeRefs from "../../hooks/useMergeRefs";
import Search from "../Search/Search";
import { SIZES } from "../../constants/sizes";
import Button from "../Button/Button";
import useListKeyboardNavigation from "../../hooks/useListKeyboardNavigation";
import ComboboxOption from "./components/ComboboxOption/ComboboxOption";
import { defaultFilter } from "./ComboboxService";
import { ComboboxItems } from "components/Combobox/components/ComboboxItems/ComboboxItems";
import { StickyCategoryHeader } from "components/Combobox/components/StickyCategoryHeader/StickyCategoryHeader";
import useActiveDescendantListFocus from "hooks/useActiveDescendantListFocus";
import { getOptionId } from "./ComboboxHelpers/ComboboxHelpers";
import "./Combobox.scss";

const Combobox = forwardRef(
Expand Down Expand Up @@ -52,8 +53,7 @@ const Combobox = forwardRef(
) => {
const componentRef = useRef(null);
const inputRef = useRef(null);
const [activeItemIndex, setActiveItemIndex] = useState(-1);
const [isActiveByKeyboard, setIsActiveByKeyboard] = useState(false);
const resultsContainerRef = useRef(null);
const [filterValue, setFilterValue] = useState("");
const mergedRef = useMergeRefs({ refs: [ref, componentRef] });
const onChangeCallback = useCallback(
Expand All @@ -66,38 +66,11 @@ const Combobox = forwardRef(
[setFilterValue, onFilterChanged]
);

const setActiveItemIndexKeyboardNav = useCallback(
index => {
setActiveItemIndex(index);
setIsActiveByKeyboard(true);
},
[setActiveItemIndex]
);

const onOptionClick = useCallback(
(_event, index, option, mouseClick) => {
if (option.disabled) return;
onClick && onClick(option);
setActiveItemIndex(index);
if (mouseClick) {
// set focus on input again
requestAnimationFrame(() => inputRef.current?.focus());
}
setIsActiveByKeyboard(!mouseClick);
if (clearFilterOnSelection) {
// clear filter after adding
onChangeCallback("");
}
},
[onClick, onChangeCallback, clearFilterOnSelection]
);

const onOptionHoverCB = useCallback(
(event, index, option) => {
setActiveItemIndex(-1);
onOptionHover(event, index, option);
},
[setActiveItemIndex, onOptionHover]
[onOptionHover]
);

const filteredOptions = useMemo(() => {
Expand All @@ -107,25 +80,53 @@ const Combobox = forwardRef(
return filter(filterValue, options);
}, [options, filterValue, filter, disableFilter]);

const isChildSelectable = useCallback(option => {
return option && !option.disabled;
}, []);
const [activeOptionIndex, setActiveOptionIndex] = useState(-1);

const isChildSelectable = useCallback(
index => {
return index !== undefined && filteredOptions[index] && !filteredOptions[index].disabled;
},
[filteredOptions]
);

const filteredOptionsIds = useMemo(
() => filteredOptions.map((option, index) => getOptionId(option?.id, index)),
[filteredOptions]
);

const onAddNewCallback = useCallback(() => {
onAddNew && onAddNew(filterValue);
// clear filter after adding
setFilterValue("");
}, [onAddNew, filterValue, setFilterValue]);

useListKeyboardNavigation(
inputRef,
filteredOptions,
activeItemIndex,
setActiveItemIndexKeyboardNav,
isChildSelectable,
onOptionClick
const overrideOnClick = useCallback(
(event, itemIndex) => {
onClick(filteredOptions[itemIndex]);
if (isChildSelectable(itemIndex)) {
setActiveOptionIndex(itemIndex);
}
if (clearFilterOnSelection) {
// clear filter after adding
onChangeCallback("");
}
},
[onClick, filteredOptions, isChildSelectable, clearFilterOnSelection, onChangeCallback]
);

const {
visualFocusItemIndex,
visualFocusItemId,
onItemClickCallback: onOptionClick
} = useActiveDescendantListFocus({
focusedElementRef: inputRef,
containerElementRef: resultsContainerRef,
focusedElementRole: useActiveDescendantListFocus.roles.COMBOBOX,
itemsIds: filteredOptionsIds,
onItemClick: overrideOnClick,
isItemSelectable: isChildSelectable
});

const hasResults = filteredOptions.length > 0;
const hasFilter = filterValue.length > 0;

Expand Down Expand Up @@ -184,7 +185,7 @@ const Combobox = forwardRef(
wrapperClassName="combobox--wrapper-search-wrapper"
className="combobox--wrapper-search"
inputAriaLabel="Search for content"
activeDescendant={`combobox-item-${activeItemIndex}`}
activeDescendant={visualFocusItemId}
id="combobox-search"
placeholder={placeholder}
size={size}
Expand All @@ -195,13 +196,14 @@ const Combobox = forwardRef(
/>
{stickyCategories && <StickyCategoryHeader label={activeCategoryLabel} />}
<ComboboxItems
ref={resultsContainerRef}
categories={categories}
options={filteredOptions}
filterValue={filterValue}
withCategoriesDivider={withCategoriesDivider}
optionRenderer={optionRenderer}
activeItemIndex={activeItemIndex}
isActiveByKeyboard={isActiveByKeyboard}
activeItemIndex={activeOptionIndex}
visualFocusItemIndex={visualFocusItemIndex}
onActiveCategoryChanged={onActiveCategoryChanged}
onOptionClick={onOptionClick}
onOptionEnter={onOptionHoverCB}
Expand Down Expand Up @@ -287,7 +289,11 @@ Combobox.propTypes = {
/**
* Using virtualized list for rendering only the items which visible to the user in any given user (performance optimization)
*/
renderOnlyVisibleOptions: PropTypes.bool
renderOnlyVisibleOptions: PropTypes.bool,
/**
* On option click callback
*/
onClick: PropTypes.func
};

Combobox.defaultProps = {
Expand Down Expand Up @@ -326,7 +332,8 @@ Combobox.defaultProps = {
stickyCategories: false,
optionRenderer: null,
clearFilterOnSelection: false,
renderOnlyVisibleOptions: false
renderOnlyVisibleOptions: false,
onClick: _optionData => {}
};

export default Combobox;
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ import React from "react";
import cx from "classnames";
import ComboboxOption from "components/Combobox/components/ComboboxOption/ComboboxOption";
import ComboboxCategory from "components/Combobox/components/ComboboxCategory/ComboboxCategory";
import Divider from "../../../Divider/Divider";
import { COMBOBOX_DIVIDER_ITEM, COMBOBOX_CATEGORY_ITEM, COMBOBOX_OPTION_ITEM } from "../ComboboxConstants";
import styles from "./ComboboxRenderers.module.scss";
import Divider from "../../Divider/Divider";
import { COMBOBOX_DIVIDER_ITEM, COMBOBOX_CATEGORY_ITEM, COMBOBOX_OPTION_ITEM } from "../components/ComboboxConstants";
import styles from "./ComboboxHelpers.module.scss";

const DIVIDER_HEIGHT = 17;
const CATEGORY_HEIGHT = 32;

export function getOptionId(id, index) {
return id !== undefined ? `combobox-item-${id}` : `combobox-item-${index}`;
}

export function createDividerItemObject({ categoryId }) {
return { type: COMBOBOX_DIVIDER_ITEM, height: DIVIDER_HEIGHT, id: `${categoryId}-divider` };
}
Expand All @@ -30,10 +34,6 @@ export function createOptionItemObject({
index,
optionRenderer,
isActive,
isActiveByKeyboard,
onOptionClick,
onOptionEnter,
onOptionLeave,
optionLineHeight,
shouldScrollToSelectedItem,
categoryId
Expand All @@ -47,10 +47,6 @@ export function createOptionItemObject({
id: option.id || index,
optionRenderer,
isActive,
isActiveByKeyboard,
onOptionClick,
onOptionEnter,
onOptionLeave,
optionLineHeight,
shouldScrollToSelectedItem,
categoryId
Expand Down Expand Up @@ -114,7 +110,7 @@ export function optionItemRenderer({
shouldScrollToSelectedItem,
activeItemIndex,
belongToCategory,
isActiveByKeyboard
visualFocusItemIndex
}) {
return (
<ComboboxOption
Expand All @@ -123,7 +119,7 @@ export function optionItemRenderer({
option={option}
optionRenderer={optionRenderer}
isActive={activeItemIndex === index}
isActiveByKeyboard={isActiveByKeyboard}
visualFocus={index === visualFocusItemIndex}
onOptionClick={onOptionClick}
onOptionHover={onOptionEnter}
onOptionLeave={onOptionLeave}
Expand Down
1 change: 1 addition & 0 deletions src/components/Combobox/__stories__/combobox.stories.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Combo box allowing users to filter longer lists to only the selections matching
{ id: "2", label: "Option 2" },
{ id: "3", label: "Option 3" }
],
onClick: () => alert("clicked"),
placeholder: "Placeholder text here",
clearFilterOnSelection: true
}}
Expand Down
9 changes: 9 additions & 0 deletions src/components/Combobox/__tests__/combobox-tests.jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ describe("Combobox tests", () => {
{ value: "yellow", label: "Yellow" }
];

it("should call item on click callback func when onClick", () => {
const onClickMock = () => console.log("%%%");
hadasfa marked this conversation as resolved.
Show resolved Hide resolved
const { getByLabelText } = render(<Combobox onClick={onClickMock} options={mockOptions} />);

fireEvent.click(getByLabelText("Yellow"));
// console.log(getByLabelText("Yellow"));
//expect(onClickMock.mock.calls.length).toBe(1);
});

it("should call callback func when onOptionHover", () => {
const onMouseOverMock = jest.fn();
const { getByLabelText } = render(<Combobox onOptionHover={onMouseOverMock} options={mockOptions} />);
Expand Down
Loading