From 49108e482c2a09ea6ee5c02acded815820f505b7 Mon Sep 17 00:00:00 2001 From: Moshe Zemah Date: Sun, 6 Jun 2021 20:27:37 +0300 Subject: [PATCH 1/2] Combobox fixes --- src/components/Combobox/Combobox.jsx | 59 ++++++++++++++----- src/components/Combobox/Combobox.scss | 4 +- .../Combobox/__stories__/combobox.stories.js | 45 +++++++++----- src/hooks/useListKeyboardNavigation.js | 10 ++-- 4 files changed, 83 insertions(+), 35 deletions(-) diff --git a/src/components/Combobox/Combobox.jsx b/src/components/Combobox/Combobox.jsx index 9025579787..1584a9de1a 100644 --- a/src/components/Combobox/Combobox.jsx +++ b/src/components/Combobox/Combobox.jsx @@ -11,7 +11,7 @@ import Button from "../Button/Button"; import useListKeyboardNavigation from "../../hooks/useListKeyboardNavigation"; import "./Combobox.scss"; -const renderOption = (index, option, isActive, onOptionClick, onOptionHover) => { +const renderOption = (index, option, isActive, isActiveByKeyboard, onOptionClick, onOptionHover) => { const { id, leftIcon, rightIcon, label, iconSize = 16, disabled, selected, ariaLabel } = option; return ( // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions @@ -21,8 +21,13 @@ const renderOption = (index, option, isActive, onOptionClick, onOptionHover) => ariaLabel={ariaLabel} id={`combox-item-${id}`} onMouseEnter={onOptionHover} - onClick={event => onOptionClick(event, index)} - className={cx("combobox-option", { disabled, selected, active: isActive })} + onClick={event => onOptionClick(event, index, option, true)} + className={cx("combobox-option", { + disabled, + selected, + active: isActive, + "active-outline": isActiveByKeyboard && isActive + })} > {leftIcon && ( { + setActiveItemIndex(index); + setIsActiveByKeyboard(true); + }, + [setActiveItemIndex] + ); + const onOptionClick = useCallback( - (_event, optionIndex) => { - onClick && onClick(options[optionIndex]); + (_event, index, option, mouseClick) => { + onClick && onClick(option); + setActiveItemIndex(index); + if (mouseClick) { + // set focus on input again + inputRef.current.focus(); + } + setIsActiveByKeyboard(!mouseClick); }, - [onClick, options] + [onClick] ); const onOptionHover = useCallback( @@ -71,13 +91,15 @@ const Combobox = forwardRef( [setActiveItemIndex] ); + const filterdOptions = useMemo(() => { + return options.filter(({ label }) => !filterValue || label.includes(filterValue)); + }, [options, filterValue]); + const renderedItems = useMemo(() => { - return options - .filter(({ label }) => !filterValue || label.includes(filterValue)) - .map((option, index) => { - return renderOption(index, option, activeItemIndex === index, onOptionClick, onOptionHover); - }); - }, [options, filterValue, onOptionClick, activeItemIndex, onOptionHover]); + return filterdOptions.map((option, index) => { + return renderOption(index, option, activeItemIndex === index, isActiveByKeyboard, onOptionClick, onOptionHover); + }); + }, [options, filterValue, activeItemIndex, isActiveByKeyboard, onOptionClick, onOptionHover]); const onChangeCallback = useCallback( value => { @@ -86,15 +108,22 @@ const Combobox = forwardRef( [setFilterValue] ); - const isChildSelectable = useCallback((index, currentOptions) => { - return currentOptions[index] && !currentOptions[index].disabled; + const isChildSelectable = useCallback(option => { + return option && !option.disabled; }, []); const onAddNewCallback = useCallback(() => { onAddNew && onAddNew(filterValue); }, [onAddNew, filterValue]); - useListKeyboardNavigation(inputRef, options, activeItemIndex, setActiveItemIndex, isChildSelectable, onOptionClick); + useListKeyboardNavigation( + inputRef, + filterdOptions, + activeItemIndex, + setActiveItemIndexKeyboardNav, + isChildSelectable, + onOptionClick + ); const hasResults = renderedItems.length > 0; const hasFilter = filterValue.length > 0; diff --git a/src/components/Combobox/Combobox.scss b/src/components/Combobox/Combobox.scss index d1c29adff6..b85def86dd 100644 --- a/src/components/Combobox/Combobox.scss +++ b/src/components/Combobox/Combobox.scss @@ -76,7 +76,9 @@ $icon-margin: 4px; &.active { position: relative; - @include focus-style-css(); + &.active-outline { + @include focus-style-css(); + } } .option-label { diff --git a/src/components/Combobox/__stories__/combobox.stories.js b/src/components/Combobox/__stories__/combobox.stories.js index be56486439..22d5bed7b5 100644 --- a/src/components/Combobox/__stories__/combobox.stories.js +++ b/src/components/Combobox/__stories__/combobox.stories.js @@ -1,4 +1,4 @@ -import React from "react"; +import React, { useState } from "react"; import { action } from "@storybook/addon-actions"; import { text, boolean, number, select } from "@storybook/addon-knobs"; import { withPerformance } from "storybook-addon-performance"; @@ -15,22 +15,39 @@ import StoryWrapper from "../../../StoryBookComponents/StoryWrapper/StoryWrapper import "./combobox.stories.scss"; export const Sandbox = () => ( -
- console.log("Add new ", value)} - addNewLabel={value => `+ Add new ${value}`} - onClick={option => console.log("Clicked on ", option.label)} - options={[ - { id: "1", label: "first", leftIcon: "fa fa-star-o" }, - { id: "2", label: "second", rightIcon: "fa fa-star-o", selected: true }, - { id: "3", label: "disabled", rightIcon: "fa fa-star-o" }, - { id: "4", label: "fourth", disabled: true } - ]} - /> +
+
); +const getOptions = selectedId => { + return [ + { id: "1", label: "first", leftIcon: "fa fa-star-o", selected: selectedId === "1" }, + { id: "2", label: "second", rightIcon: "fa fa-star-o", selected: selectedId === "2" }, + { id: "3", label: "disabled", disabled: true, rightIcon: "fa fa-star-o", selected: selectedId === "3" }, + { id: "4", label: "fourth", selected: selectedId === "4" } + ]; +}; + +const ComboboxWrapper = () => { + const [selectedId, setSelectedId] = useState("2"); + + return ( +
+ console.log("Add new ", value)} + addNewLabel={value => `+ Add new ${value}`} + onClick={option => { + console.log("Clicked on ", option.label); + setSelectedId(option.id); + }} + options={getOptions(selectedId)} + /> +
+ ); +}; + export default { title: "Components/Combobox", component: Combobox, diff --git a/src/hooks/useListKeyboardNavigation.js b/src/hooks/useListKeyboardNavigation.js index 20e54f63d6..8e0cd3e005 100644 --- a/src/hooks/useListKeyboardNavigation.js +++ b/src/hooks/useListKeyboardNavigation.js @@ -25,9 +25,9 @@ export default function useListKeyboardNavigation( if (direction === ARROW_DIRECTIONS.DOWN) { if (activeItemIndex >= options.length - 1) return; - for (let offset = 1; offset < options.length; offset++) { + for (let offset = 1; offset <= options.length; offset++) { const nextIndex = activeItemIndex + offset; - if (isChildSelectable(nextIndex, options)) { + if (isChildSelectable(options[nextIndex])) { newIndex = nextIndex; break; } @@ -38,7 +38,7 @@ export default function useListKeyboardNavigation( } else { for (let offset = 1; offset <= activeItemIndex; offset++) { const prevIndex = activeItemIndex - offset; - if (isChildSelectable(prevIndex, options)) { + if (isChildSelectable(options[prevIndex])) { newIndex = prevIndex; break; } @@ -61,8 +61,8 @@ export default function useListKeyboardNavigation( const onEnterClickCallback = useCallback( event => { const hasValidIndex = activeItemIndex || activeItemIndex === 0; - if (!onClick || !hasValidIndex || !isChildSelectable(activeItemIndex, options)) return; - onClick(event, activeItemIndex); + if (!onClick || !hasValidIndex || !isChildSelectable(options[activeItemIndex])) return; + onClick(event, activeItemIndex, options[activeItemIndex]); }, [activeItemIndex, onClick, isChildSelectable, options] ); From b56b10d61f838dd89b52010e770f8584e1392bcd Mon Sep 17 00:00:00 2001 From: Moshe Zemah Date: Sun, 6 Jun 2021 20:27:53 +0300 Subject: [PATCH 2/2] Combobox fixes --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index db6d7a0e4b..a41ceabebb 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "monday-ui-react-core", - "version": "0.3.37", + "version": "0.3.38", "description": "Official monday.com UI resources for application development in React.js", "main": "dist/main.js", "scripts": {