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

fix(ListBox): a11y - move filter tag as sibling of menu trigger, esc key improvements #13173

Merged
merged 13 commits into from
Mar 3, 2023
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
8 changes: 8 additions & 0 deletions packages/react/src/components/ComboBox/ComboBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
},
});

Expand Down
23 changes: 4 additions & 19 deletions packages/react/src/components/ListBox/ListBoxSelection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -84,20 +83,6 @@ const ListBoxSelection: ListBoxSelectionComponent = ({
onClearSelection(event);
}
};
const handleOnKeyDown = (event: KeyboardEvent<HTMLDivElement>) => {
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`,
Expand All @@ -107,17 +92,18 @@ const ListBoxSelection: ListBoxSelectionComponent = ({
[`${prefix}--tag--disabled`]: disabled,
}
);

/* eslint-disable jsx-a11y/click-events-have-key-events */
return selectionCount ? (
<div className={tagClasses}>
<span className={`${prefix}--tag__label`} title={`${selectionCount}`}>
{selectionCount}
</span>
<div
role="button"
tabIndex={disabled ? -1 : 0}
tabIndex={-1}
className={`${prefix}--tag__close-icon`}
onClick={handleOnClick}
onKeyDown={handleOnKeyDown}
aria-label={t('clear.all')}
title={description}
aria-disabled={readOnly ? true : undefined}>
Expand All @@ -128,9 +114,8 @@ const ListBoxSelection: ListBoxSelectionComponent = ({
<div
role="button"
className={className}
tabIndex={disabled ? -1 : 0}
tabIndex={-1}
onClick={handleOnClick}
onKeyDown={handleOnKeyDown}
aria-label={description}
title={description}>
{selectionCount}
Expand Down
22 changes: 2 additions & 20 deletions packages/react/src/components/ListBox/next/ListBoxSelection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -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 (
<div className={tagClasses}>
Expand All @@ -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">
<Close />
Expand All @@ -94,8 +77,7 @@ function ListBoxSelection({
className={className}
disabled={disabled}
onClick={onClick}
onKeyDown={onKeyDown}
tabIndex={disabled ? -1 : 0}
tabIndex={-1}
title={description}
type="button">
<Close />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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],
{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -299,6 +302,20 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect(
event.stopPropagation();
}

if (!disabled) {
if (match(event, keys.Delete) || match(event, keys.Escape)) {
if (isOpen) {
handleOnMenuChange(true);
clearInputValue();
event.stopPropagation();
} else if (!isOpen) {
clearInputValue();
clearSelection();
event.stopPropagation();
}
}
}

if (match(event, keys.Tab)) {
handleOnMenuChange(false);
}
Expand All @@ -308,6 +325,7 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect(
},
onBlur: () => {
setInputFocused(false);
setInputValue('');
},
});

Expand Down
68 changes: 44 additions & 24 deletions packages/react/src/components/MultiSelect/MultiSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ export interface MultiSelectProps<ItemType>

/**
* `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;

Expand Down Expand Up @@ -315,7 +315,8 @@ const MultiSelect = React.forwardRef(function MultiSelect<ItemType>(
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 {
Expand Down Expand Up @@ -347,7 +348,14 @@ const MultiSelect = React.forwardRef(function MultiSelect<ItemType>(
items,
});

const toggleButtonProps = getToggleButtonProps();
const toggleButtonProps = getToggleButtonProps({
onFocus: () => {
setInputFocused(true);
},
onBlur: () => {
setInputFocused(false);
},
});
const mergedRef = mergeRefs(toggleButtonProps.ref, ref);

const selectedItems = selectedItem as ItemType[];
Expand Down Expand Up @@ -406,6 +414,7 @@ const MultiSelect = React.forwardRef(function MultiSelect<ItemType>(
[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`]:
Expand Down Expand Up @@ -464,12 +473,21 @@ const MultiSelect = React.forwardRef(function MultiSelect<ItemType>(
}

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();
}
}
};

const multiSelectFieldWrapperClasses = cx(
`${prefix}--list-box__field--wrapper`,
{
[`${prefix}--list-box__field--wrapper--input-focused`]: inputFocused,
}
);

const handleFocus = (evt: React.FocusEvent<HTMLDivElement>) => {
evt.target.classList.contains(`${prefix}--tag__close-icon`)
? setIsFocused(false)
Expand Down Expand Up @@ -529,15 +547,7 @@ const MultiSelect = React.forwardRef(function MultiSelect<ItemType>(
className={`${prefix}--list-box__invalid-icon ${prefix}--list-box__invalid-icon--warning`}
/>
)}
<button
type="button"
className={`${prefix}--list-box__field`}
disabled={disabled}
aria-disabled={disabled || readOnly}
{...toggleButtonProps}
ref={mergedRef}
onKeyDown={onKeyDown}
{...readOnlyEventHandlers}>
<div className={multiSelectFieldWrapperClasses}>
{selectedItems.length > 0 && (
<ListBox.Selection
readOnly={readOnly}
Expand All @@ -548,14 +558,24 @@ const MultiSelect = React.forwardRef(function MultiSelect<ItemType>(
disabled={disabled}
/>
)}
<span id={fieldLabelId} className={`${prefix}--list-box__label`}>
{label}
</span>
<ListBox.MenuIcon
isOpen={!!isOpen}
translateWithId={translateWithId}
/>
</button>
<button
type="button"
className={`${prefix}--list-box__field`}
disabled={disabled}
aria-disabled={disabled || readOnly}
{...toggleButtonProps}
ref={mergedRef}
onKeyDown={onKeyDown}
{...readOnlyEventHandlers}>
<span id={fieldLabelId} className={`${prefix}--list-box__label`}>
{label}
</span>
<ListBox.MenuIcon
isOpen={isOpen}
translateWithId={translateWithId}
/>
</button>
</div>
<ListBox.Menu aria-multiselectable="true" {...getMenuProps()}>
{isOpen &&
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
Expand Down Expand Up @@ -737,7 +757,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,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@
@use '../fluid-combo-box';

@mixin fluid-multiselect {
.#{$prefix}--multi-select__wrapper.#{$prefix}--list-box__wrapper--fluid--focus:not(.#{$prefix}--multi-select--filterable__wrapper)
.#{$prefix}--list-box__field--wrapper--input-focused {
outline: none;
}

.#{$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 {
Expand Down
2 changes: 1 addition & 1 deletion packages/styles/scss/components/form/_form.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
23 changes: 17 additions & 6 deletions packages/styles/scss/components/multiselect/_multiselect.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down