Skip to content

Commit

Permalink
fix: filterable multiselect readonly state implemented (carbon-design…
Browse files Browse the repository at this point in the history
…-system#17662)

* fix: filterable multiselect readonly state implemented

* fix: reviewed changes added

* fix: resovled pr review comments

* feat: added readonly state for listboxselection

* fix: removed conditional class on readonly matching disabled

---------

Co-authored-by: Preeti Bansal <[email protected]>
  • Loading branch information
2 people authored and annawen1 committed Oct 22, 2024
1 parent eeb5352 commit 9767f72
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 10 deletions.
24 changes: 19 additions & 5 deletions packages/react/src/components/ListBox/next/ListBoxSelection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const defaultTranslations: Record<TranslationKey, string> = {
function defaultTranslateWithId(id: TranslationKey): string {
return defaultTranslations[id];
}

export interface ListBoxSelectionProps {
/**
* Specify a function to be invoked when a user interacts with the clear
Expand All @@ -57,6 +58,11 @@ export interface ListBoxSelectionProps {
* Specify whether or not the clear selection element should be disabled
*/
disabled?: boolean;
/**
* Whether or not the listbox is readonly
*/
readOnly?: boolean;

/**
* Specify an optional `onClearSelection` handler that is called when the underlying
* element is cleared
Expand Down Expand Up @@ -86,6 +92,7 @@ function ListBoxSelection({
selectionCount,
translateWithId: t = defaultTranslateWithId,
disabled,
readOnly,
onClearSelection,
...rest
}: ListBoxSelectionProps) {
Expand All @@ -106,7 +113,7 @@ function ListBoxSelection({

function onClick(event: React.MouseEvent<HTMLButtonElement, MouseEvent>) {
event.stopPropagation();
if (disabled) {
if (disabled || readOnly) {
return;
}
clearSelection(event);
Expand All @@ -126,11 +133,12 @@ function ListBoxSelection({
<button
aria-label={description}
className={`${prefix}--tag__close-icon`}
disabled={disabled}
disabled={disabled || readOnly}
onClick={onClick}
tabIndex={-1}
title={description}
type="button">
type="button"
aria-disabled={readOnly ? true : undefined}>
<Close />
</button>
</div>
Expand All @@ -142,11 +150,12 @@ function ListBoxSelection({
{...rest}
aria-label={description}
className={className}
disabled={disabled}
disabled={disabled || readOnly}
onClick={onClick}
tabIndex={-1}
title={description}
type="button">
type="button"
aria-disabled={readOnly ? true : undefined}>
<Close />
</button>
);
Expand All @@ -164,6 +173,11 @@ ListBoxSelection.propTypes = {
*/
disabled: PropTypes.bool,

/**
* Whether or not the listbox is readonly
*/
readOnly: PropTypes.bool,

/**
* Specify an optional `onClearSelection` handler that is called when the underlying
* element is cleared
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,11 @@ export interface FilterableMultiSelectProps<ItemType>
*/
placeholder?: string;

/**
* Whether or not the filterable multiselect is readonly
*/
readOnly?: boolean;

/**
* Specify feedback (mode) of the selection.
* `top`: selected item jumps to top
Expand Down Expand Up @@ -335,6 +340,7 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
onChange,
onMenuChange,
placeholder,
readOnly,
titleText,
type,
selectionFeedback = 'top-after-reopen',
Expand Down Expand Up @@ -505,9 +511,11 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
};

function handleMenuChange(forceIsOpen: boolean): void {
const nextIsOpen = forceIsOpen ?? !isOpen;
setIsOpen(nextIsOpen);
validateHighlightFocus();
if (!readOnly) {
const nextIsOpen = forceIsOpen ?? !isOpen;
setIsOpen(nextIsOpen);
validateHighlightFocus();
}
}

useEffect(() => {
Expand Down Expand Up @@ -689,6 +697,7 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
[`${prefix}--multi-select--selected`]:
controlledSelectedItems?.length > 0,
[`${prefix}--multi-select--filterable--input-focused`]: inputFocused,
[`${prefix}--multi-select--readonly`]: readOnly,
}
);

Expand Down Expand Up @@ -798,6 +807,28 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
}
};

const mergedRef = mergeRefs(textInput, inputProps.ref);

const readOnlyEventHandlers = readOnly
? {
onClick: (evt: React.MouseEvent<HTMLInputElement>) => {
// NOTE: does not prevent click
evt.preventDefault();
// focus on the element as per readonly input behavior
if (mergedRef.current !== undefined) {
mergedRef.current.focus();
}
},
onKeyDown: (evt: React.KeyboardEvent<HTMLInputElement>) => {
const selectAccessKeys = ['ArrowDown', 'ArrowUp', ' ', 'Enter'];
// This prevents the select from opening for the above keys
if (selectAccessKeys.includes(evt.key)) {
evt.preventDefault();
}
},
}
: {};

const clearSelectionContent =
controlledSelectedItems.length > 0 ? (
<span className={`${prefix}--visually-hidden`}>
Expand Down Expand Up @@ -832,13 +863,14 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
invalidText={invalidText}
warn={warn}
warnText={warnText}
isOpen={isOpen}
isOpen={!readOnly && isOpen}
size={size}>
<div
className={`${prefix}--list-box__field`}
ref={autoAlign ? refs.setReference : null}>
{controlledSelectedItems.length > 0 && (
<ListBoxSelection
readOnly={readOnly}
clearSelection={() => {
clearSelection();
if (textInput.current) {
Expand All @@ -853,7 +885,9 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
<input
className={inputClasses}
{...inputProps}
ref={mergeRefs(textInput, inputProps.ref)}
ref={mergedRef}
{...readOnlyEventHandlers}
readOnly={readOnly}
/>
{invalid && (
<WarningFilled className={`${prefix}--list-box__invalid-icon`} />
Expand All @@ -868,6 +902,7 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
clearSelection={clearInputValue}
disabled={disabled}
translateWithId={translateWithId}
readOnly={readOnly}
onMouseUp={(event: MouseEvent) => {
// If we do not stop this event from propagating,
// it seems like Downshift takes our event and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import React from 'react';
import { act, render, screen } from '@testing-library/react';
import { getByText } from '@carbon/test-utils/dom';
import userEvent from '@testing-library/user-event';
import FilterableMultiSelect from '../FilterableMultiSelect';
import {
Expand Down Expand Up @@ -63,6 +64,28 @@ describe('FilterableMultiSelect', () => {
expect(mockProps.onMenuChange).toHaveBeenCalledWith(false);
});

it('should not be interactive if readonly', async () => {
const items = generateItems(4, generateGenericItem);
const label = 'test-label';
const { container } = render(
<FilterableMultiSelect
id="test"
readOnly={true}
label={label}
items={items}
/>
);
await waitForPosition();

// eslint-disable-next-line testing-library/prefer-screen-queries
const labelNode = getByText(container, label);
await userEvent.click(labelNode);

expect(
// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
container.querySelector('[aria-expanded="true"][aria-haspopup="listbox"]')
).toBeFalsy();
});
it('should initially have the menu open when open prop is provided', async () => {
render(<FilterableMultiSelect {...mockProps} open />);
await waitForPosition();
Expand Down

0 comments on commit 9767f72

Please sign in to comment.