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

feat(multi-select, simple-select): add listActionButton & onListAction #7044

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
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
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ will have the same width as the input.

### With Action Button

Default Action Button will be rendered when the `listActionButton` prop is set to `true` on the Component.
Setting the `listActionButton` prop to `true` renders a default `"Add New Item"` `Button`. However, a custom `Button` component can be passed as the `listActionButton` value via a node.

A custom `Button` Component could be passed as the `listActionButton` value.
We recommend this pattern for loading/adding new options to the `FilterableSelect`.

<Canvas of={FilterableSelectStories.WithActionButton} />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,7 @@ export const WithActionButton: Story = () => {
label="color"
value={value}
onChange={(event) => setValue(event.target.value)}
listActionButton={
<Button iconType="add" iconPosition="after">
Add a New Element
</Button>
}
listActionButton
onListAction={() => setIsOpen(true)}
>
{optionList}
Expand All @@ -384,7 +380,7 @@ export const WithActionButton: Story = () => {
onCancel={() => setIsOpen(false)}
title="Dialog component triggered on action"
>
<Button onClick={addNew}>Add new</Button>
<Button onClick={addNew}>Add a New Element</Button>
damienrobson-sage marked this conversation as resolved.
Show resolved Hide resolved
</Dialog>
</Box>
);
Expand Down
82 changes: 82 additions & 0 deletions src/components/select/multi-select/components.test-pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,65 @@ export const MultiSelectMultiColumnsComponent = (
);
};

export const MultiSelectWithActionButtonComponent = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (non-blocking): might be worth just using the list action button to add the new options so it's a bit simpler, opening a dialog seems a bit left field to me

Copy link
Contributor

Choose a reason for hiding this comment

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

Leave this for now until we've had confirmation about how it should behave when the button is clicked

const [value, setValue] = useState<string[]>([]);

function onChangeHandler(event: React.ChangeEvent<HTMLInputElement>) {
setValue((event.target.value as unknown) as string[]);
}

const [isOpen, setIsOpen] = useState(false);
const [optionList, setOptionList] = useState([
<Option text="Amber" value="amber" key="Amber" />,
<Option text="Black" value="black" key="Black" />,
<Option text="Blue" value="blue" key="Blue" />,
<Option text="Brown" value="brown" key="Brown" />,
<Option text="Green" value="green" key="Green" />,
<Option text="Amber" value="amber1" key="Amber1" />,
<Option text="Black" value="black1" key="Black1" />,
<Option text="Blue" value="blue1" key="Blue1" />,
<Option text="Brown" value="brown1" key="Brown1" />,
<Option text="Green" value="green1" key="Green1" />,
]);
function addNew() {
const counter = optionList.length.toString();
setOptionList((newOptionList) => [
...newOptionList,
<Option
text={`New${counter}`}
value={`val${counter}`}
key={`New${counter}`}
/>,
]);
setIsOpen(false);
setValue((prevValue) => [...prevValue, `val${counter}`]);
}
return (
<>
<MultiSelect
label="color"
value={value}
onChange={onChangeHandler}
listActionButton={
<Button iconType="add" iconPosition="after">
Add a New Element
</Button>
}
onListAction={() => setIsOpen(true)}
>
{optionList}
</MultiSelect>
<Dialog
open={isOpen}
onCancel={() => setIsOpen(false)}
title="Dialog component triggered on action"
>
<Button onClick={addNew}>Add new</Button>
</Dialog>
</>
);
};

export const MultiSelectMaxOptionsComponent = (
props: Partial<MultiSelectProps>
) => {
Expand Down Expand Up @@ -365,6 +424,29 @@ export const MultiSelectCustomColorComponent = (
);
};

export const MultiSelectListActionEventComponent = (
props: Partial<MultiSelectProps>
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (non-blocking): you could make onListAction Required here so it's flagged if someone ever uses it and doesn't pass the callback etc

) => {
return (
<MultiSelect
label="color"
labelInline
{...props}
listActionButton={
<Button iconType="add" iconPosition="after">
Add a New Element
</Button>
}
>
<Option text="Amber" value="1" />
<Option text="Black" value="2" />
<Option text="Blue" value="3" />
<Option text="Brown" value="4" />
<Option text="Green" value="5" />
</MultiSelect>
);
};

export const MultiSelectWithManyOptionsAndVirtualScrolling = () => (
<MultiSelect
name="virtualised"
Expand Down
31 changes: 31 additions & 0 deletions src/components/select/multi-select/multi-select.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import React, {
} from "react";
import invariant from "invariant";

import { ButtonProps } from "../../button";
import { filterOutStyledSystemSpacingProps } from "../../../style/utils";
import SelectTextbox, {
FormInputPropTypes,
Expand Down Expand Up @@ -55,6 +56,8 @@ export interface MultiSelectProps
defaultValue?: string[] | Record<string, unknown>[];
/** If true the loader animation is displayed in the option list */
isLoading?: boolean;
/** True for default text button or a Button Component to be rendered */
listActionButton?: boolean | React.ReactElement<ButtonProps>;
/** When true component will work in multi column mode.
* Children should consist of OptionRow components in this mode
*/
Expand All @@ -65,6 +68,8 @@ export interface MultiSelectProps
onFilterChange?: (filterText: string) => void;
/** A custom callback for when the dropdown menu opens */
onOpen?: () => void;
/** A callback for when the Action Button is triggered */
onListAction?: () => void;
/** If true the Component opens on focus */
openOnFocus?: boolean;
/** SelectList table header, should consist of multiple th elements.
Expand Down Expand Up @@ -122,7 +127,9 @@ export const MultiSelect = React.forwardRef<HTMLInputElement, MultiSelectProps>(
onKeyDown,
openOnFocus = false,
noResultsMessage,
listActionButton,
placeholder,
onListAction,
isLoading,
tableHeader,
multiColumn,
Expand Down Expand Up @@ -444,6 +451,17 @@ export const MultiSelect = React.forwardRef<HTMLInputElement, MultiSelectProps>(
};
}, [handleGlobalClick]);

useEffect(() => {
const hasListActionButton = listActionButton !== undefined;
const onListActionMissingMessage =
"onListAction prop required when using listActionButton prop";

invariant(
!hasListActionButton || (hasListActionButton && onListAction),
onListActionMissingMessage
);
}, [listActionButton, onListAction]);

const onFilterChange = useStableCallback(
onFilterChangeProp as (filterTextArg: unknown) => void
);
Expand Down Expand Up @@ -529,6 +547,11 @@ export const MultiSelect = React.forwardRef<HTMLInputElement, MultiSelectProps>(
isMouseDownReported.current = true;
}

function handleOnListAction() {
setOpenState(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: as the list closes when the button is clicked, will this really work for lazy loading?

@harpalsingh fyi

onListAction?.();
}

function handleTextboxFocus(event: React.FocusEvent<HTMLInputElement>) {
const triggerFocus = () => onFocus?.(event);

Expand Down Expand Up @@ -579,6 +602,12 @@ export const MultiSelect = React.forwardRef<HTMLInputElement, MultiSelectProps>(
selectionConfirmed,
} = optionData;

if (selectionType === "tab") {
setOpenState(false);
textboxRef?.focus();
return;
}

if (selectionType === "navigationKey") {
setHighlightedValue(newValue);
setActiveDescendantId(selectedOptionId);
Expand Down Expand Up @@ -682,11 +711,13 @@ export const MultiSelect = React.forwardRef<HTMLInputElement, MultiSelectProps>(
filterText={filterText.trim()}
highlightedValue={highlightedValue}
noResultsMessage={noResultsMessage}
listActionButton={listActionButton}
isLoading={isLoading}
tableHeader={tableHeader}
multiColumn={multiColumn}
listPlacement={listWidth !== undefined ? placement : listPlacement}
listMaxHeight={listMaxHeight}
onListAction={handleOnListAction}
flipEnabled={flipEnabled}
multiselectValues={actualValue}
isOpen={isOpen}
Expand Down
8 changes: 8 additions & 0 deletions src/components/select/multi-select/multi-select.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ If there is no `id` prop specified on an object, then the exact objects will be

<Canvas of={MultiSelectStories.WithObjectAsValue} />

### With Action Button

Setting the `listActionButton` prop to `true` renders a default `"Add New Item"` `Button`. However, a custom `Button` component can be passed as the `listActionButton` value via a node.

We recommend this pattern for loading/adding new options to the `MultiSelect`.

<Canvas of={MultiSelectStories.WithActionButton} />

### With isLoading prop

When `isLoading` prop is passed, a loader will be appended at the end of the Select List. That functionality could be used to load the options asynchronously.
Expand Down
105 changes: 105 additions & 0 deletions src/components/select/multi-select/multi-select.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ import {
MultiSelectLazyLoadTwiceComponent,
MultiSelectObjectAsValueComponent,
MultiSelectMultiColumnsComponent,
MultiSelectWithActionButtonComponent,
MultiSelectCustomColorComponent,
MultiSelectLongPillComponent,
MultiSelectOnFilterChangeEventComponent,
MultiSelectListActionEventComponent,
MultiSelectWithManyOptionsAndVirtualScrolling,
MultiSelectNestedInDialog,
MultiSelectErrorOnChangeNewValidation,
Expand Down Expand Up @@ -893,6 +895,82 @@ test.describe("MultiSelect component", () => {
).toBeVisible();
});

test("should render list options with an action button and trigger Dialog on action", async ({
mount,
page,
}) => {
await mount(<MultiSelectWithActionButtonComponent />);

await dropdownButton(page).click();
await expect(selectListWrapper(page)).toBeVisible();
const addElementButtonElement = page.locator('[data-component="button"]');
await expect(addElementButtonElement).toBeVisible();
await expect(addElementButtonElement).toHaveText("Add a New Element");
const iconElement = page.locator('[type="add"]');
await expect(iconElement).toBeVisible();
await addElementButtonElement.click();
await expect(alertDialogPreview(page)).toBeVisible();
});

test("should render list options with an action button that is visible without scrolling and without affecting the list height", async ({
mount,
page,
}) => {
await mount(<MultiSelectWithActionButtonComponent />);

await dropdownButton(page).click();
await expect(selectListWrapper(page)).toBeVisible();
await expect(page.locator('[data-component="button"]')).toBeInViewport();
const selectListHeight = await selectListWrapper(
page
).evaluate((wrapperElement) =>
parseInt(
window.getComputedStyle(wrapperElement).getPropertyValue("height")
)
);
await expect(selectListHeight).toBeGreaterThan(220);
await expect(selectListHeight).toBeLessThan(250);
});

test("when navigating with the keyboard, the selected option is not hidden behind an action button", async ({
mount,
page,
}) => {
await mount(<MultiSelectWithActionButtonComponent />);

await dropdownButton(page).click();
const inputElement = commonDataElementInputPreview(page);
for (let i = 0; i < 5; i++) {
// eslint-disable-next-line no-await-in-loop
await inputElement.focus();
// eslint-disable-next-line no-await-in-loop
await inputElement.press("ArrowDown");
}
await expect(selectOptionByText(page, "Green").nth(0)).toBeInViewport();
});

test("should add new list option from Add new Dialog", async ({
mount,
page,
}) => {
await mount(<MultiSelectWithActionButtonComponent />);

const newOption = "New10";
await dropdownButton(page).click();
await expect(selectListWrapper(page)).toBeVisible();
const addElementButtonElement = page.locator('[data-component="button"]');
await expect(addElementButtonElement).toBeVisible();
await addElementButtonElement.click();
await expect(alertDialogPreview(page)).toBeVisible();
await page.waitForTimeout(250);
const addNewButtonElement = page.getByRole("button", {
name: "Add New",
});
await expect(addNewButtonElement).toBeVisible();
await addNewButtonElement.click();
await expect(multiSelectPill(page)).toHaveAttribute("title", newOption);
});

[
["3", "Blue"],
["7", "Pink"],
Expand Down Expand Up @@ -1273,6 +1351,23 @@ test.describe("Check events for MultiSelect component", () => {
await expect(callbackArguments.length).toBe(1);
await expect(callbackArguments[0]).toBe(text);
});

test("should call onListAction event when the Action Button is clicked", async ({
mount,
page,
}) => {
let callbackCount = 0;
const callback = () => {
callbackCount += 1;
};
await mount(
<MultiSelectListActionEventComponent onListAction={callback} />
);

await dropdownButton(page).click();
await page.locator('[data-component="button"]').click();
await expect(callbackCount).toBe(1);
});
});

test.describe("Check virtual scrolling", () => {
Expand Down Expand Up @@ -1899,6 +1994,16 @@ test.describe("Accessibility tests for MultiSelect component", () => {
});
});

test("should pass accessibility tests with an action button and trigger Dialog on action", async ({
mount,
page,
}) => {
await mount(<MultiSelectWithActionButtonComponent />);

await dropdownButton(page).click();
await checkAccessibility(page, undefined, "scrollable-region-focusable");
});

test("should pass accessibility tests with virtual scrolling", async ({
mount,
page,
Expand Down
Loading
Loading