Skip to content

Commit

Permalink
fix: combobox inside of popover bug (#4049)
Browse files Browse the repository at this point in the history
  • Loading branch information
nkrantz authored Aug 29, 2024
1 parent e95c712 commit 37acfcd
Show file tree
Hide file tree
Showing 13 changed files with 254 additions and 51 deletions.
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

0 comments on commit 37acfcd

Please sign in to comment.