Skip to content

fix: combobox inside of popover bug #4049

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

Merged
merged 2 commits into from
Aug 29, 2024
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
6 changes: 6 additions & 0 deletions .changeset/healthy-cobras-whisper.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@twilio-paste/combobox": minor
"@twilio-paste/core": minor
---

[Combobox] Add new prop `usePortal` which defaults to `true`. The prop was introduced to address a bug when Comboboxes are placed in Popovers. usePortal should be set to false when using a Combobox inside a Popover in order to retain full functionality.
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export const ListBoxPositioner: React.FC<ListBoxPositionerProps> = ({ inputBoxRe
};
}
}

return {
position: "fixed",
top: inputBoxDimensions?.bottom,
Expand Down
35 changes: 35 additions & 0 deletions packages/paste-core/components/combobox/src/ListboxWrapper.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { Box } from "@twilio-paste/box";
import { Portal } from "@twilio-paste/reakit-library";
import * as React from "react";

import { ListBoxPositioner } from "./ListboxPositioner";

/*
* This component renders the listbox in an absolutely positioned Box when `usePortal={false}`.
* Portal and ListBoxPositioner shouldn't be used when the Combobox is placed in a Popover.
* Using Combobox in a Popover was previously causing interaction issues because of the Portal that Popover uses.
* Because ListBoxPositioner isn't used when `usePortal={false}`, the listbox won't change position based on the window
* (e.g. moving above the input box when the Combobox is at the bottom of the window).
*/

export const ListboxWrapper: React.FC<{
inputBoxRef: React.RefObject<HTMLElement>;
parentRef: React.RefObject<HTMLElement>;
usePortal: boolean;
children: React.ReactNode;
}> = ({ inputBoxRef, parentRef, usePortal, children }) => {
if (!usePortal)
return (
<Box position="absolute" top="100%" width="100%">
{children}
</Box>
);
return (
<Portal>
<ListBoxPositioner inputBoxRef={inputBoxRef} dropdownBoxRef={parentRef}>
{children}
</ListBoxPositioner>
</Portal>
);
};
ListboxWrapper.displayName = "ListboxWrapper";
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { HelpText } from "@twilio-paste/help-text";
import { ChevronDownIcon } from "@twilio-paste/icons/esm/ChevronDownIcon";
import { InputBox, InputChevronWrapper, getInputChevronIconColor } from "@twilio-paste/input-box";
import { Label } from "@twilio-paste/label";
import { Portal } from "@twilio-paste/reakit-library";
import { ScreenReaderOnly } from "@twilio-paste/screen-reader-only";
import { useUID } from "@twilio-paste/uid-library";
import { useWindowSize } from "@twilio-paste/utils";
Expand All @@ -14,7 +13,7 @@ import * as React from "react";
import { useVirtual } from "react-virtual";

import { ComboboxItems } from "../ComboboxItems";
import { ListBoxPositioner } from "../ListboxPositioner";
import { ListboxWrapper } from "../ListboxWrapper";
import { getHelpTextVariant } from "../helpers";
import { ComboboxListbox } from "../styles/ComboboxListbox";
import type { MultiselectComboboxProps } from "../types";
Expand All @@ -24,6 +23,7 @@ import { extractPropsFromState } from "./extractPropsFromState";
export const MultiselectCombobox = React.forwardRef<HTMLInputElement, MultiselectComboboxProps>(
(
{
usePortal = true,
element = "MULTISELECT_COMBOBOX",
disabled,
hasError,
Expand Down Expand Up @@ -365,32 +365,30 @@ export const MultiselectCombobox = React.forwardRef<HTMLInputElement, Multiselec
</InputChevronWrapper>
</Box>
</InputBox>
<Portal>
<ListBoxPositioner inputBoxRef={inputBoxRef} dropdownBoxRef={parentRef}>
<ComboboxListbox
{...getMenuProps({ ref: parentRef })}
element={`${element}_LISTBOX`}
hidden={!isOpen}
aria-multiselectable="true"
>
<ComboboxItems
ref={scrollToIndexRef}
items={items}
element={element}
getItemProps={getItemProps}
highlightedIndex={highlightedIndex}
selectedItems={selectedItems}
disabledItems={disabledItems}
totalSize={rowVirtualizer.totalSize}
virtualItems={rowVirtualizer.virtualItems}
optionTemplate={optionTemplate}
groupItemsBy={groupItemsBy}
groupLabelTemplate={groupLabelTemplate}
emptyState={emptyState}
/>
</ComboboxListbox>
</ListBoxPositioner>
</Portal>
<ListboxWrapper inputBoxRef={inputBoxRef} parentRef={parentRef} usePortal={usePortal}>
<ComboboxListbox
{...getMenuProps({ ref: parentRef })}
element={`${element}_LISTBOX`}
hidden={!isOpen}
aria-multiselectable="true"
>
<ComboboxItems
ref={scrollToIndexRef}
items={items}
element={element}
getItemProps={getItemProps}
highlightedIndex={highlightedIndex}
selectedItems={selectedItems}
disabledItems={disabledItems}
totalSize={rowVirtualizer.totalSize}
virtualItems={rowVirtualizer.virtualItems}
optionTemplate={optionTemplate}
groupItemsBy={groupItemsBy}
groupLabelTemplate={groupLabelTemplate}
emptyState={emptyState}
/>
</ComboboxListbox>
</ListboxWrapper>
{helpText && (
<HelpText id={helpTextId} variant={getHelpTextVariant(variant, hasError)} element={`${element}_HELP_TEXT`}>
{helpText}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ import { ChevronDownIcon } from "@twilio-paste/icons/esm/ChevronDownIcon";
import type { InputVariants } from "@twilio-paste/input";
import { InputBox, InputChevronWrapper, getInputChevronIconColor } from "@twilio-paste/input-box";
import { Label } from "@twilio-paste/label";
import { Portal } from "@twilio-paste/reakit-library";
import { useUID } from "@twilio-paste/uid-library";
import { useWindowSize } from "@twilio-paste/utils";
import * as React from "react";
import { useVirtual } from "react-virtual";

import { ComboboxItems } from "../ComboboxItems";
import { ListBoxPositioner } from "../ListboxPositioner";
import { ListboxWrapper } from "../ListboxWrapper";
import { visuallyHiddenStyles } from "../helpers";
import { ComboboxInputSelect } from "../styles/ComboboxInputSelect";
import { ComboboxInputWrapper } from "../styles/ComboboxInputWrapper";
Expand All @@ -36,6 +35,7 @@ const getHelpTextVariant = (variant: InputVariants, hasError: boolean | undefine
const Combobox = React.forwardRef<HTMLInputElement, ComboboxProps>(
(
{
usePortal = true,
autocomplete,
disabled,
element = "COMBOBOX",
Expand Down Expand Up @@ -180,26 +180,24 @@ const Combobox = React.forwardRef<HTMLInputElement, ComboboxProps>(
)}
</ComboboxInputWrapper>
</InputBox>
<Portal>
<ListBoxPositioner inputBoxRef={inputBoxRef} dropdownBoxRef={parentRef}>
<ComboboxListbox hidden={!isOpen} element={`${element}_LISTBOX`} {...getMenuProps({ ref: parentRef })}>
<ComboboxItems
ref={scrollToIndexRef}
items={items}
element={element}
getItemProps={getItemProps}
highlightedIndex={highlightedIndex}
disabledItems={disabledItems}
optionTemplate={optionTemplate}
groupItemsBy={groupItemsBy}
groupLabelTemplate={groupLabelTemplate}
totalSize={rowVirtualizer.totalSize}
virtualItems={rowVirtualizer.virtualItems}
emptyState={emptyState}
/>
</ComboboxListbox>
</ListBoxPositioner>
</Portal>
<ListboxWrapper inputBoxRef={inputBoxRef} parentRef={parentRef} usePortal={usePortal}>
<ComboboxListbox hidden={!isOpen} element={`${element}_LISTBOX`} {...getMenuProps({ ref: parentRef })}>
<ComboboxItems
ref={scrollToIndexRef}
items={items}
element={element}
getItemProps={getItemProps}
highlightedIndex={highlightedIndex}
disabledItems={disabledItems}
optionTemplate={optionTemplate}
groupItemsBy={groupItemsBy}
groupLabelTemplate={groupLabelTemplate}
totalSize={rowVirtualizer.totalSize}
virtualItems={rowVirtualizer.virtualItems}
emptyState={emptyState}
/>
</ComboboxListbox>
</ListboxWrapper>
{helpText && (
<HelpText id={helpTextId} variant={getHelpTextVariant(variant, hasError)}>
{helpText}
Expand Down
8 changes: 8 additions & 0 deletions packages/paste-core/components/combobox/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ export type HighlightedIndexChanges = {
};

export interface ComboboxProps extends Omit<InputProps, "id" | "type" | "value" | "autoComplete"> {
/**
* Determines whether the Combobox Listbox (options list) is rendered inside a Portal. Defaults to `true`. Use `false` if you are using Combobox inside a Paste Popover to prevent interaction bugs.
*
* @type {boolean}
* @memberof ComboboxProps
* @default true
*/
usePortal?: boolean;
/**
* Activates the autocomplete/typeahead feature
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { SearchIcon } from "@twilio-paste/icons/esm/SearchIcon";
import { Label } from "@twilio-paste/label";
import { MediaBody, MediaFigure, MediaObject } from "@twilio-paste/media-object";
import { Modal, ModalBody, ModalHeader, ModalHeading } from "@twilio-paste/modal";
import { Popover, PopoverButton, PopoverContainer } from "@twilio-paste/popover";
import { Option, Select } from "@twilio-paste/select";
import { Text } from "@twilio-paste/text";
import { useUID } from "@twilio-paste/uid-library";
Expand Down Expand Up @@ -885,3 +886,14 @@ ComboboxInModal.parameters = {
disable: true,
},
};

export const ComboboxInPopover: StoryFn = () => {
return (
<PopoverContainer baseId="popover-example">
<PopoverButton variant="primary">Open</PopoverButton>
<Popover aria-label="Popover">
<Combobox items={items} labelText="Select an item" usePortal={false} />
</Popover>
</PopoverContainer>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { AttachIcon } from "@twilio-paste/icons/esm/AttachIcon";
import { InformationIcon } from "@twilio-paste/icons/esm/InformationIcon";
import { MediaBody, MediaFigure, MediaObject } from "@twilio-paste/media-object";
import { Modal, ModalBody, ModalHeader, ModalHeading } from "@twilio-paste/modal";
import { Popover, PopoverButton, PopoverContainer } from "@twilio-paste/popover";
import { Text } from "@twilio-paste/text";
import { useUID } from "@twilio-paste/uid-library";
import filter from "lodash/filter";
Expand Down Expand Up @@ -676,6 +677,30 @@ MultiselectComboboxInModal.parameters = {
},
};

export const MultiselectComboboxInPopover: StoryFn = () => {
const [inputValue, setInputValue] = React.useState("");
const filteredItems = React.useMemo(() => getFilteredItems(inputValue), [inputValue]);

return (
<PopoverContainer baseId="popover-example">
<PopoverButton variant="primary">Open</PopoverButton>
<Popover aria-label="Popover">
<Box width="size30">
<MultiselectCombobox
usePortal={false}
selectedItemsLabelText="items:"
items={filteredItems}
labelText="Select an item"
onInputValueChange={({ inputValue: newInputValue = "" }) => {
setInputValue(newInputValue);
}}
/>
</Box>
</Popover>
</PopoverContainer>
);
};

// eslint-disable-next-line import/no-default-export
export default {
title: "Components/Combobox/MultiselectCombobox",
Expand Down
14 changes: 14 additions & 0 deletions packages/paste-core/components/combobox/type-docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -6740,6 +6740,13 @@
"required": false,
"externalProp": true
},
"usePortal": {
"type": "boolean",
"defaultValue": true,
"required": false,
"externalProp": false,
"description": "Determines whether the Combobox Listbox (options list) is rendered inside a Portal. Defaults to `true`. Use `false` if you are using Combobox inside a Paste Popover to prevent interaction bugs."
},
"variant": {
"type": "InputVariants",
"defaultValue": "default",
Expand Down Expand Up @@ -8715,6 +8722,13 @@
"required": false,
"externalProp": true
},
"usePortal": {
"type": "boolean",
"defaultValue": true,
"required": false,
"externalProp": false,
"description": "Determines whether the Combobox Listbox (options list) is rendered inside a Portal. Defaults to `true`. Use `false` if you are using Combobox inside a Paste Popover to prevent interaction bugs."
},
"variant": {
"type": "InputVariants",
"defaultValue": "default",
Expand Down
25 changes: 25 additions & 0 deletions packages/paste-website/src/component-examples/ComboboxExamples.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,3 +502,28 @@ render(
<EmptyStateCombobox />
)
`.trim();

export const popoverExample = `

const items = [
"Alert",
"Heading",
"List",
"Paragraph",
];

const PopoverCombobox = () => {
return (
<PopoverContainer baseId="popover-example">
<PopoverButton variant="primary">Open</PopoverButton>
<Popover aria-label="Popover">
<Combobox items={items} labelText="Select an item" usePortal={false} />
</Popover>
</PopoverContainer>
);
};

render(
<PopoverCombobox />
)
`.trim();
Original file line number Diff line number Diff line change
Expand Up @@ -624,3 +624,48 @@ render(
<MultiselectComboboxExample />
)
`.trim();

export const popoverExample = `

const items = [
"Alert",
"Heading",
"List",
"Paragraph",
];

function getFilteredItems(inputValue) {
const lowerCasedInputValue = inputValue.toLowerCase();

return items.filter(function filterItems(item) {
return item.toLowerCase().includes(lowerCasedInputValue);
});
}

const PopoverCombobox = () => {
const [inputValue, setInputValue] = React.useState("");
const filteredItems = React.useMemo(() => getFilteredItems(inputValue), [inputValue]);
return (
<PopoverContainer baseId="popover-example">
<PopoverButton variant="primary">Open</PopoverButton>
<Popover aria-label="Popover">
<Box width="size30">
<MultiselectCombobox
usePortal={false}
selectedItemsLabelText="items:"
items={filteredItems}
labelText="Select an item"
onInputValueChange={({ inputValue: newInputValue = "" }) => {
setInputValue(newInputValue);
}}
/>
</Box>
</Popover>
</PopoverContainer>
);
};

render(
<PopoverCombobox />
)
`.trim();
Loading
Loading