From 020b04973eeae9693c7f8167547eaf9bb22f9fdb Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Thu, 16 Feb 2023 12:29:30 -0500 Subject: [PATCH 1/8] fix(MultiSelect): a11y - move filter tag as sibling of menu trigger --- .../components/MultiSelect/MultiSelect.tsx | 59 ++++++++++++------- .../components/multiselect/_multiselect.scss | 23 ++++++-- 2 files changed, 55 insertions(+), 27 deletions(-) diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index 8a81fbc1d30d..974d55461f3f 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -213,7 +213,7 @@ export interface MultiSelectProps /** * `onMenuChange` is a utility for this controlled component to communicate to a - * consuming component that the menu was opend(`true`)/closed(`false`). + * consuming component that the menu was open(`true`)/closed(`false`). */ onMenuChange?(open: boolean): void; @@ -315,7 +315,8 @@ const MultiSelect = React.forwardRef(function MultiSelect( const { current: multiSelectInstanceId } = useRef(getInstanceId()); const [highlightedIndex, setHighlightedIndex] = useState(); const [isFocused, setIsFocused] = useState(false); - const [isOpen, setIsOpen] = useState(open); + const [inputFocused, setInputFocused] = useState(false); + const [isOpen, setIsOpen] = useState(open || false); const [prevOpenProp, setPrevOpenProp] = useState(open); const [topItems, setTopItems] = useState([]); const { @@ -347,7 +348,14 @@ const MultiSelect = React.forwardRef(function MultiSelect( items, }); - const toggleButtonProps = getToggleButtonProps(); + const toggleButtonProps = getToggleButtonProps({ + onFocus: () => { + setInputFocused(true); + }, + onBlur: () => { + setInputFocused(false); + }, + }); const mergedRef = mergeRefs(toggleButtonProps.ref, ref); const selectedItems = selectedItem as ItemType[]; @@ -470,6 +478,13 @@ const MultiSelect = React.forwardRef(function MultiSelect( } }; + const multiSelectFieldWrapperClasses = cx( + `${prefix}--list-box__field--wrapper`, + { + [`${prefix}--list-box__field--wrapper--input-focused`]: inputFocused, + } + ); + const handleFocus = (evt: React.FocusEvent) => { evt.target.classList.contains(`${prefix}--tag__close-icon`) ? setIsFocused(false) @@ -529,15 +544,7 @@ const MultiSelect = React.forwardRef(function MultiSelect( className={`${prefix}--list-box__invalid-icon ${prefix}--list-box__invalid-icon--warning`} /> )} - + + {isOpen && // eslint-disable-next-line @typescript-eslint/no-non-null-assertion @@ -737,7 +754,7 @@ MultiSelect.propTypes = { /** * `onMenuChange` is a utility for this controlled component to communicate to a - * consuming component that the menu was opend(`true`)/closed(`false`). + * consuming component that the menu was open(`true`)/closed(`false`). */ onMenuChange: PropTypes.func, diff --git a/packages/styles/scss/components/multiselect/_multiselect.scss b/packages/styles/scss/components/multiselect/_multiselect.scss index fcf021977cdc..53f1f99501ec 100644 --- a/packages/styles/scss/components/multiselect/_multiselect.scss +++ b/packages/styles/scss/components/multiselect/_multiselect.scss @@ -17,12 +17,19 @@ /// @access public /// @group multi-select @mixin multiselect { - .#{$prefix}--multi-select .#{$prefix}--tag { - min-width: auto; - margin: 0 $spacing-03 0 0; + .#{$prefix}--multi-select .#{$prefix}--list-box__field--wrapper { + display: inline-flex; + width: 100%; + height: calc(100% + 1px); + align-items: center; + } + + .#{$prefix}--multi-select .#{$prefix}--list-box__field:focus { + @include focus-outline('reset'); } - .#{$prefix}--multi-select--filterable .#{$prefix}--tag { + .#{$prefix}--multi-select .#{$prefix}--tag { + min-width: auto; margin: 0 $spacing-03 0 $spacing-05; } @@ -75,12 +82,16 @@ outline: none; } - .#{$prefix}--multi-select--filterable--input-focused { + .#{$prefix}--multi-select--filterable--input-focused, + .#{$prefix}--multi-select + .#{$prefix}--list-box__field--wrapper--input-focused { @include focus-outline('outline'); } .#{$prefix}--multi-select--filterable.#{$prefix}--multi-select--selected - .#{$prefix}--text-input { + .#{$prefix}--text-input, + .#{$prefix}--multi-select.#{$prefix}--multi-select--selected + .#{$prefix}--list-box__field { padding-left: 0; } From 7b44a83d73d42a23cc36482605412ad5612de93c Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Thu, 16 Feb 2023 14:25:44 -0500 Subject: [PATCH 2/8] fix(Multiselect): remove tag from tab order --- .../components/ListBox/ListBoxSelection.tsx | 34 +++++++------------ .../components/MultiSelect/MultiSelect.tsx | 8 +++-- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/packages/react/src/components/ListBox/ListBoxSelection.tsx b/packages/react/src/components/ListBox/ListBoxSelection.tsx index f3c32675d41a..f79b51f0b9f1 100644 --- a/packages/react/src/components/ListBox/ListBoxSelection.tsx +++ b/packages/react/src/components/ListBox/ListBoxSelection.tsx @@ -18,7 +18,9 @@ export interface ListBoxSelectionProps { * Specify a function to be invoked when a user interacts with the clear * selection element. */ - clearSelection(event: MouseEvent | KeyboardEvent): void; + clearSelection( + event: MouseEvent | KeyboardEvent + ): void; /** * Specify whether or not the clear selection element should be disabled @@ -29,7 +31,9 @@ export interface ListBoxSelectionProps { * Specify an optional `onClearSelection` handler that is called when the underlying * element is cleared */ - onClearSelection?(event: MouseEvent | KeyboardEvent): void; + onClearSelection?( + event: MouseEvent | KeyboardEvent + ): void; /** * Whether or not the Dropdown is readonly @@ -50,7 +54,7 @@ export interface ListBoxSelectionProps { translateWithId(messageId: string, args?: Record): string; } -export type ListBoxSelectionComponent = React.FC +export type ListBoxSelectionComponent = React.FC; /** * `ListBoxSelection` is used to provide controls for clearing a selection, in @@ -80,20 +84,6 @@ const ListBoxSelection: ListBoxSelectionComponent = ({ onClearSelection(event); } }; - const handleOnKeyDown = (event: KeyboardEvent) => { - event.stopPropagation(); - if (disabled || readOnly) { - return; - } - - // When a user hits ENTER, we'll clear the selection - if (match(event.code, keys.Enter)) { - clearSelection(event); - if (onClearSelection) { - onClearSelection(event); - } - } - }; const description = selectionCount ? t('clear.all') : t('clear.selection'); const tagClasses = cx( `${prefix}--tag`, @@ -103,6 +93,8 @@ const ListBoxSelection: ListBoxSelectionComponent = ({ [`${prefix}--tag--disabled`]: disabled, } ); + + /* eslint-disable jsx-a11y/click-events-have-key-events */ return selectionCount ? (
@@ -110,10 +102,9 @@ const ListBoxSelection: ListBoxSelectionComponent = ({
@@ -124,16 +115,15 @@ const ListBoxSelection: ListBoxSelectionComponent = ({
{selectionCount}
); -} +}; export const translationIds = { 'clear.all': 'clear.all', diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index 974d55461f3f..e2939e4c02c8 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -472,9 +472,11 @@ const MultiSelect = React.forwardRef(function MultiSelect( } const onKeyDown = (e) => { - if (match(e, keys.Delete) && !disabled) { - clearSelection(); - e.stopPropagation(); + if (!disabled) { + if (match(e, keys.Delete) || match(e, keys.Escape)) { + clearSelection(); + e.stopPropagation(); + } } }; From 010d1f7d2f2e8c880ffab8a02336ba6127ace7b3 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Thu, 16 Feb 2023 14:48:20 -0500 Subject: [PATCH 3/8] fix(FilterableMultiselect): remove clear, tag from tab order --- .../components/ListBox/ListBoxSelection.tsx | 1 - .../ListBox/next/ListBoxSelection.js | 22 ++----------------- .../MultiSelect/FilterableMultiSelect.js | 16 ++++++++++++++ 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/packages/react/src/components/ListBox/ListBoxSelection.tsx b/packages/react/src/components/ListBox/ListBoxSelection.tsx index f79b51f0b9f1..e88a88cbdc5a 100644 --- a/packages/react/src/components/ListBox/ListBoxSelection.tsx +++ b/packages/react/src/components/ListBox/ListBoxSelection.tsx @@ -9,7 +9,6 @@ import cx from 'classnames'; import React from 'react'; import PropTypes from 'prop-types'; import { Close } from '@carbon/icons-react'; -import { match, keys } from '../../internal/keyboard'; import { usePrefix } from '../../internal/usePrefix'; import { KeyboardEvent, MouseEvent } from 'react'; diff --git a/packages/react/src/components/ListBox/next/ListBoxSelection.js b/packages/react/src/components/ListBox/next/ListBoxSelection.js index 6beba16636db..d7fbe6c67bd6 100644 --- a/packages/react/src/components/ListBox/next/ListBoxSelection.js +++ b/packages/react/src/components/ListBox/next/ListBoxSelection.js @@ -9,7 +9,6 @@ import cx from 'classnames'; import React from 'react'; import PropTypes from 'prop-types'; import { Close } from '@carbon/icons-react'; -import { match, keys } from '../../../internal/keyboard'; import { usePrefix } from '../../../internal/usePrefix'; /** @@ -51,21 +50,6 @@ function ListBoxSelection({ } } - function onKeyDown(event) { - event.stopPropagation(); - if (disabled) { - return; - } - - // When a user hits ENTER, we'll clear the selection - if (match(event, keys.Enter)) { - clearSelection(event); - if (onClearSelection) { - onClearSelection(event); - } - } - } - if (selectionCount) { return (
@@ -77,8 +61,7 @@ function ListBoxSelection({ className={`${prefix}--tag__close-icon`} disabled={disabled} onClick={onClick} - onKeyDown={onKeyDown} - tabIndex={disabled ? -1 : 0} + tabIndex={-1} title={description} type="button"> @@ -94,8 +77,7 @@ function ListBoxSelection({ className={className} disabled={disabled} onClick={onClick} - onKeyDown={onKeyDown} - tabIndex={disabled ? -1 : 0} + tabIndex={-1} title={description} type="button"> diff --git a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js index cbb0e39d2a23..caf790bc1ceb 100644 --- a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js +++ b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js @@ -299,6 +299,22 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect( event.stopPropagation(); } + if (!disabled) { + if (match(event, keys.Delete) || match(event, keys.Escape)) { + if (isOpen) { + handleOnMenuChange(true); + event.stopPropagation(); + } else if (!isOpen) { + clearInputValue(); + event.stopPropagation(); + if (event.target.value === '') { + clearSelection(); + event.stopPropagation(); + } + } + } + } + if (match(event, keys.Tab)) { handleOnMenuChange(false); } From 6fdfe96d2e7022d3a2fb26bf83916aec3be52dc2 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Thu, 16 Feb 2023 15:10:46 -0500 Subject: [PATCH 4/8] fix(ComboBox): retain input value when menu is open and esc is pressed --- packages/react/src/components/ComboBox/ComboBox.tsx | 8 ++++++++ .../src/components/MultiSelect/FilterableMultiSelect.js | 7 ++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx index 0a6c3321ead9..93bcfe1e6071 100644 --- a/packages/react/src/components/ComboBox/ComboBox.tsx +++ b/packages/react/src/components/ComboBox/ComboBox.tsx @@ -480,6 +480,14 @@ const ComboBox = React.forwardRef((props: ComboBoxProps, ref) => { if (match(event, keys.Enter) && !inputValue) { toggleMenu(); } + + if (match(event, keys.Escape) && inputValue) { + if (event.target === textInput.current && isOpen) { + toggleMenu(); + event.preventDownshiftDefault = true; + event.persist(); + } + } }, }); diff --git a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js index caf790bc1ceb..18a358aa276d 100644 --- a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js +++ b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js @@ -303,14 +303,11 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect( if (match(event, keys.Delete) || match(event, keys.Escape)) { if (isOpen) { handleOnMenuChange(true); + clearInputValue(); event.stopPropagation(); } else if (!isOpen) { - clearInputValue(); + clearSelection(); event.stopPropagation(); - if (event.target.value === '') { - clearSelection(); - event.stopPropagation(); - } } } } From ed164b9bdb798663dd55341573b286a07507d62e Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Fri, 17 Feb 2023 08:57:00 -0500 Subject: [PATCH 5/8] fix(FilterableMultiSelect): a few more UX tweaks --- .../react/src/components/MultiSelect/FilterableMultiSelect.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js index 18a358aa276d..3fd1f4d79e10 100644 --- a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js +++ b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js @@ -306,6 +306,7 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect( clearInputValue(); event.stopPropagation(); } else if (!isOpen) { + clearInputValue(); clearSelection(); event.stopPropagation(); } @@ -321,6 +322,7 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect( }, onBlur: () => { setInputFocused(false); + setInputValue(''); }, }); From 8ba57252ec2eec6f0060e30cb238d49dda3a921b Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Fri, 24 Feb 2023 10:13:00 -0500 Subject: [PATCH 6/8] style(MultiSelect): update styles to fix invalid issues when focused --- .../src/components/MultiSelect/FilterableMultiSelect.js | 3 +++ .../src/components/MultiSelect/MultiSelect.stories.js | 1 + .../react/src/components/MultiSelect/MultiSelect.tsx | 3 +++ .../components/fluid-multiselect/_fluid-multiselect.scss | 9 +++++++++ packages/styles/scss/components/form/_form.scss | 2 +- 5 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js index 3fd1f4d79e10..0c84ed1bb614 100644 --- a/packages/react/src/components/MultiSelect/FilterableMultiSelect.js +++ b/packages/react/src/components/MultiSelect/FilterableMultiSelect.js @@ -87,6 +87,7 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect( const wrapperClasses = cx( `${prefix}--multi-select__wrapper`, + `${prefix}--multi-select--filterable__wrapper`, `${prefix}--list-box__wrapper`, [enabled ? containerClassName : null], { @@ -243,6 +244,8 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect( [enabled ? null : containerClassName], { [`${prefix}--multi-select--invalid`]: invalid, + [`${prefix}--multi-select--invalid--focused`]: + invalid && inputFocused, [`${prefix}--multi-select--open`]: isOpen, [`${prefix}--multi-select--inline`]: inline, [`${prefix}--multi-select--selected`]: selectedItem.length > 0, diff --git a/packages/react/src/components/MultiSelect/MultiSelect.stories.js b/packages/react/src/components/MultiSelect/MultiSelect.stories.js index f0eb0f9c658d..bcbe1bf9edc8 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.stories.js +++ b/packages/react/src/components/MultiSelect/MultiSelect.stories.js @@ -302,6 +302,7 @@ export const _FilterableWithLayer = () => { return (
( [`${prefix}--form__helper-text--disabled`]: disabled, }); + console.log(inputFocused); + const className = cx( `${prefix}--multi-select`, [enabled ? null : containerClassName], { [`${prefix}--multi-select--invalid`]: invalid, + [`${prefix}--multi-select--invalid--focused`]: invalid && inputFocused, [`${prefix}--multi-select--warning`]: showWarning, [`${prefix}--multi-select--inline`]: inline, [`${prefix}--multi-select--selected`]: diff --git a/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss b/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss index d14cbe279d4d..e9da9803daa0 100644 --- a/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss +++ b/packages/styles/scss/components/fluid-multiselect/_fluid-multiselect.scss @@ -19,6 +19,15 @@ @use '../fluid-combo-box'; @mixin fluid-multiselect { + .#{$prefix}--multi-select__wrapper.#{$prefix}--list-box__wrapper--fluid--focus:not(.#{$prefix}--multi-select--filterable__wrapper) { + outline-offset: -2px; + } + + .#{$prefix}--list-box__wrapper--fluid + .#{$prefix}--tag.#{$prefix}--tag--filter { + margin-top: 1.25rem; + } + // Filterable .#{$prefix}--list-box__wrapper--fluid .#{$prefix}--multi-select--filterable--input-focused { diff --git a/packages/styles/scss/components/form/_form.scss b/packages/styles/scss/components/form/_form.scss index 0bc87fc07d6d..5920fea010a3 100644 --- a/packages/styles/scss/components/form/_form.scss +++ b/packages/styles/scss/components/form/_form.scss @@ -125,7 +125,7 @@ $input-label-weight: 400 !default; > .#{$prefix}--text-area--invalid:not(:focus), .#{$prefix}--select-input__wrapper[data-invalid] .#{$prefix}--select-input:not(:focus), - .#{$prefix}--list-box[data-invalid]:not(:focus), + .#{$prefix}--list-box[data-invalid]:not(.#{$prefix}--multi-select--invalid--focused), .#{$prefix}--combo-box[data-invalid]:not(.#{$prefix}--multi-select--selected) .#{$prefix}--text-input:not(:focus) { @include focus-outline('invalid'); From f9f738a6d60f47a63a1869a20c8074686282ec93 Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Fri, 24 Feb 2023 10:30:33 -0500 Subject: [PATCH 7/8] style(FluidMultiselect): adjust focus styles when invalid --- .../react/src/components/MultiSelect/MultiSelect.stories.js | 1 - .../components/fluid-multiselect/_fluid-multiselect.scss | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react/src/components/MultiSelect/MultiSelect.stories.js b/packages/react/src/components/MultiSelect/MultiSelect.stories.js index bcbe1bf9edc8..f0eb0f9c658d 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.stories.js +++ b/packages/react/src/components/MultiSelect/MultiSelect.stories.js @@ -302,7 +302,6 @@ export const _FilterableWithLayer = () => { return (
Date: Mon, 27 Feb 2023 11:32:49 -0500 Subject: [PATCH 8/8] chore(log): remove console log --- packages/react/src/components/MultiSelect/MultiSelect.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index a4c778abd167..0b4dcc670458 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -409,8 +409,6 @@ const MultiSelect = React.forwardRef(function MultiSelect( [`${prefix}--form__helper-text--disabled`]: disabled, }); - console.log(inputFocused); - const className = cx( `${prefix}--multi-select`, [enabled ? null : containerClassName],