From 4c0b7b682bb93c716cb28bc8351e3b77bcb82338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cnora?= Date: Tue, 27 Aug 2024 16:04:44 -0400 Subject: [PATCH 1/2] chore: attach combobox portal to input box dom node --- .../combobox/src/ListboxPositioner.tsx | 2 +- .../src/multiselect/MultiselectCombobox.tsx | 62 +++++++++++-------- .../combobox/src/singleselect/Combobox.tsx | 50 ++++++++------- .../combobox/stories/Combobox.stories.tsx | 12 ++++ .../stories/MultiselectCombobox.stories.tsx | 24 +++++++ 5 files changed, 101 insertions(+), 49 deletions(-) diff --git a/packages/paste-core/components/combobox/src/ListboxPositioner.tsx b/packages/paste-core/components/combobox/src/ListboxPositioner.tsx index 2600af38ea..06f786c8d9 100644 --- a/packages/paste-core/components/combobox/src/ListboxPositioner.tsx +++ b/packages/paste-core/components/combobox/src/ListboxPositioner.tsx @@ -57,8 +57,8 @@ export const ListBoxPositioner: React.FC = ({ inputBoxRe }; } } - return { + // TODO: adjust placement for comboboxes inside of portals (popovers/modals) position: "fixed", top: inputBoxDimensions?.bottom, left: inputBoxDimensions?.left, diff --git a/packages/paste-core/components/combobox/src/multiselect/MultiselectCombobox.tsx b/packages/paste-core/components/combobox/src/multiselect/MultiselectCombobox.tsx index 911324b43c..4a4f1164ee 100644 --- a/packages/paste-core/components/combobox/src/multiselect/MultiselectCombobox.tsx +++ b/packages/paste-core/components/combobox/src/multiselect/MultiselectCombobox.tsx @@ -5,12 +5,12 @@ 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"; import includes from "lodash/includes"; import * as React from "react"; +import { createPortal } from "react-dom"; import { useVirtual } from "react-virtual"; import { ComboboxItems } from "../ComboboxItems"; @@ -275,6 +275,12 @@ export const MultiselectCombobox = React.forwardRef { + setDomReady(true); + }, []); + return ( - - - - - + {domReady && + createPortal( + + + , + inputBoxRef.current as Element, + )} {helpText && ( {helpText} diff --git a/packages/paste-core/components/combobox/src/singleselect/Combobox.tsx b/packages/paste-core/components/combobox/src/singleselect/Combobox.tsx index a79a355c7d..d7925c600c 100644 --- a/packages/paste-core/components/combobox/src/singleselect/Combobox.tsx +++ b/packages/paste-core/components/combobox/src/singleselect/Combobox.tsx @@ -5,10 +5,10 @@ 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 { createPortal } from "react-dom"; import { useVirtual } from "react-virtual"; import { ComboboxItems } from "../ComboboxItems"; @@ -143,6 +143,12 @@ const Combobox = React.forwardRef( [items, internalSelectedItem], ); + const [domReady, setDomReady] = React.useState(false); + + React.useEffect(() => { + setDomReady(true); + }, []); + return ( @@ -180,26 +186,28 @@ const Combobox = React.forwardRef( )} - - - - - + {domReady && + createPortal( + + + , + inputBoxRef.current as Element, + )} {helpText && ( {helpText} diff --git a/packages/paste-core/components/combobox/stories/Combobox.stories.tsx b/packages/paste-core/components/combobox/stories/Combobox.stories.tsx index 5ec77fac05..ca9dd1b334 100644 --- a/packages/paste-core/components/combobox/stories/Combobox.stories.tsx +++ b/packages/paste-core/components/combobox/stories/Combobox.stories.tsx @@ -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, usePopoverState } from "@twilio-paste/popover"; import { Option, Select } from "@twilio-paste/select"; import { Text } from "@twilio-paste/text"; import { useUID } from "@twilio-paste/uid-library"; @@ -885,3 +886,14 @@ ComboboxInModal.parameters = { disable: true, }, }; + +export const ComboboxInPopover: StoryFn = () => { + return ( + + Open + + + + + ); +}; diff --git a/packages/paste-core/components/combobox/stories/MultiselectCombobox.stories.tsx b/packages/paste-core/components/combobox/stories/MultiselectCombobox.stories.tsx index face3ee2e9..f4ba105a0c 100644 --- a/packages/paste-core/components/combobox/stories/MultiselectCombobox.stories.tsx +++ b/packages/paste-core/components/combobox/stories/MultiselectCombobox.stories.tsx @@ -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, usePopoverState } from "@twilio-paste/popover"; import { Text } from "@twilio-paste/text"; import { useUID } from "@twilio-paste/uid-library"; import filter from "lodash/filter"; @@ -676,6 +677,29 @@ MultiselectComboboxInModal.parameters = { }, }; +export const MultiselectComboboxInPopover: StoryFn = () => { + const [inputValue, setInputValue] = React.useState(""); + const filteredItems = React.useMemo(() => getFilteredItems(inputValue), [inputValue]); + + return ( + + Open + + + { + setInputValue(newInputValue); + }} + /> + + + + ); +}; + // eslint-disable-next-line import/no-default-export export default { title: "Components/Combobox/MultiselectCombobox", From 09d9390f4471ecda858cecb2068304375d0debc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cnora?= Date: Wed, 28 Aug 2024 16:41:58 -0400 Subject: [PATCH 2/2] feat(combobox): add usePortal prop --- .changeset/healthy-cobras-whisper.md | 6 ++ .../combobox/src/ListboxPositioner.tsx | 1 - .../combobox/src/ListboxWrapper.tsx | 35 +++++++++++ .../src/multiselect/MultiselectCombobox.tsx | 62 ++++++++----------- .../combobox/src/singleselect/Combobox.tsx | 50 ++++++--------- .../components/combobox/src/types.ts | 8 +++ .../combobox/stories/Combobox.stories.tsx | 4 +- .../stories/MultiselectCombobox.stories.tsx | 3 +- .../components/combobox/type-docs.json | 14 +++++ .../component-examples/ComboboxExamples.ts | 25 ++++++++ .../MultiselectComboboxExamples.ts | 45 ++++++++++++++ .../src/pages/components/combobox/index.mdx | 19 ++++++ .../components/multiselect-combobox/index.mdx | 19 ++++++ 13 files changed, 221 insertions(+), 70 deletions(-) create mode 100644 .changeset/healthy-cobras-whisper.md create mode 100644 packages/paste-core/components/combobox/src/ListboxWrapper.tsx diff --git a/.changeset/healthy-cobras-whisper.md b/.changeset/healthy-cobras-whisper.md new file mode 100644 index 0000000000..245439d124 --- /dev/null +++ b/.changeset/healthy-cobras-whisper.md @@ -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. diff --git a/packages/paste-core/components/combobox/src/ListboxPositioner.tsx b/packages/paste-core/components/combobox/src/ListboxPositioner.tsx index 06f786c8d9..7d7c486f28 100644 --- a/packages/paste-core/components/combobox/src/ListboxPositioner.tsx +++ b/packages/paste-core/components/combobox/src/ListboxPositioner.tsx @@ -58,7 +58,6 @@ export const ListBoxPositioner: React.FC = ({ inputBoxRe } } return { - // TODO: adjust placement for comboboxes inside of portals (popovers/modals) position: "fixed", top: inputBoxDimensions?.bottom, left: inputBoxDimensions?.left, diff --git a/packages/paste-core/components/combobox/src/ListboxWrapper.tsx b/packages/paste-core/components/combobox/src/ListboxWrapper.tsx new file mode 100644 index 0000000000..52feb5d964 --- /dev/null +++ b/packages/paste-core/components/combobox/src/ListboxWrapper.tsx @@ -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; + parentRef: React.RefObject; + usePortal: boolean; + children: React.ReactNode; +}> = ({ inputBoxRef, parentRef, usePortal, children }) => { + if (!usePortal) + return ( + + {children} + + ); + return ( + + + {children} + + + ); +}; +ListboxWrapper.displayName = "ListboxWrapper"; diff --git a/packages/paste-core/components/combobox/src/multiselect/MultiselectCombobox.tsx b/packages/paste-core/components/combobox/src/multiselect/MultiselectCombobox.tsx index 4a4f1164ee..60e8900920 100644 --- a/packages/paste-core/components/combobox/src/multiselect/MultiselectCombobox.tsx +++ b/packages/paste-core/components/combobox/src/multiselect/MultiselectCombobox.tsx @@ -10,11 +10,10 @@ import { useUID } from "@twilio-paste/uid-library"; import { useWindowSize } from "@twilio-paste/utils"; import includes from "lodash/includes"; import * as React from "react"; -import { createPortal } from "react-dom"; 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"; @@ -24,6 +23,7 @@ import { extractPropsFromState } from "./extractPropsFromState"; export const MultiselectCombobox = React.forwardRef( ( { + usePortal = true, element = "MULTISELECT_COMBOBOX", disabled, hasError, @@ -275,12 +275,6 @@ export const MultiselectCombobox = React.forwardRef { - setDomReady(true); - }, []); - return ( - {domReady && - createPortal( - - - , - inputBoxRef.current as Element, - )} + + + {helpText && ( {helpText} diff --git a/packages/paste-core/components/combobox/src/singleselect/Combobox.tsx b/packages/paste-core/components/combobox/src/singleselect/Combobox.tsx index d7925c600c..7c26b25738 100644 --- a/packages/paste-core/components/combobox/src/singleselect/Combobox.tsx +++ b/packages/paste-core/components/combobox/src/singleselect/Combobox.tsx @@ -8,11 +8,10 @@ import { Label } from "@twilio-paste/label"; import { useUID } from "@twilio-paste/uid-library"; import { useWindowSize } from "@twilio-paste/utils"; import * as React from "react"; -import { createPortal } from "react-dom"; 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"; @@ -36,6 +35,7 @@ const getHelpTextVariant = (variant: InputVariants, hasError: boolean | undefine const Combobox = React.forwardRef( ( { + usePortal = true, autocomplete, disabled, element = "COMBOBOX", @@ -143,12 +143,6 @@ const Combobox = React.forwardRef( [items, internalSelectedItem], ); - const [domReady, setDomReady] = React.useState(false); - - React.useEffect(() => { - setDomReady(true); - }, []); - return ( @@ -186,28 +180,24 @@ const Combobox = React.forwardRef( )} - {domReady && - createPortal( - - - , - inputBoxRef.current as Element, - )} + + + {helpText && ( {helpText} diff --git a/packages/paste-core/components/combobox/src/types.ts b/packages/paste-core/components/combobox/src/types.ts index feba49dd0f..d062223a8e 100644 --- a/packages/paste-core/components/combobox/src/types.ts +++ b/packages/paste-core/components/combobox/src/types.ts @@ -42,6 +42,14 @@ export type HighlightedIndexChanges = { }; export interface ComboboxProps extends Omit { + /** + * 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 * diff --git a/packages/paste-core/components/combobox/stories/Combobox.stories.tsx b/packages/paste-core/components/combobox/stories/Combobox.stories.tsx index ca9dd1b334..2f95af2ed4 100644 --- a/packages/paste-core/components/combobox/stories/Combobox.stories.tsx +++ b/packages/paste-core/components/combobox/stories/Combobox.stories.tsx @@ -9,7 +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, usePopoverState } from "@twilio-paste/popover"; +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"; @@ -892,7 +892,7 @@ export const ComboboxInPopover: StoryFn = () => { Open - + ); diff --git a/packages/paste-core/components/combobox/stories/MultiselectCombobox.stories.tsx b/packages/paste-core/components/combobox/stories/MultiselectCombobox.stories.tsx index f4ba105a0c..6a515a2a9e 100644 --- a/packages/paste-core/components/combobox/stories/MultiselectCombobox.stories.tsx +++ b/packages/paste-core/components/combobox/stories/MultiselectCombobox.stories.tsx @@ -7,7 +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, usePopoverState } from "@twilio-paste/popover"; +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"; @@ -687,6 +687,7 @@ export const MultiselectComboboxInPopover: StoryFn = () => { ) `.trim(); + +export const popoverExample = ` + +const items = [ + "Alert", + "Heading", + "List", + "Paragraph", +]; + +const PopoverCombobox = () => { + return ( + + Open + + + + + ); +}; + +render( + +) +`.trim(); diff --git a/packages/paste-website/src/component-examples/MultiselectComboboxExamples.ts b/packages/paste-website/src/component-examples/MultiselectComboboxExamples.ts index 63a53c372b..fba8818a6a 100644 --- a/packages/paste-website/src/component-examples/MultiselectComboboxExamples.ts +++ b/packages/paste-website/src/component-examples/MultiselectComboboxExamples.ts @@ -624,3 +624,48 @@ render( ) `.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 ( + + Open + + + { + setInputValue(newInputValue); + }} + /> + + + + ); +}; + +render( + +) +`.trim(); diff --git a/packages/paste-website/src/pages/components/combobox/index.mdx b/packages/paste-website/src/pages/components/combobox/index.mdx index 159211ee97..e3dd9ecbe5 100644 --- a/packages/paste-website/src/pages/components/combobox/index.mdx +++ b/packages/paste-website/src/pages/components/combobox/index.mdx @@ -22,6 +22,8 @@ import {CloseIcon} from '@twilio-paste/icons/esm/CloseIcon'; import {Text} from '@twilio-paste/text'; import {UnorderedList, ListItem} from '@twilio-paste/list'; import {Callout, CalloutHeading, CalloutText} from '@twilio-paste/callout'; +import {InlineCode} from '@twilio-paste/inline-code' +import { Popover, PopoverButton, PopoverContainer } from "@twilio-paste/popover"; import Changelog from '@twilio-paste/combobox/CHANGELOG.md'; import {SidebarCategoryRoutes} from '../../../constants'; import { @@ -37,6 +39,7 @@ import { groupedComboboxExample, groupedLabelComboboxExample, stateHookCombobox, + popoverExample } from '../../../component-examples/ComboboxExamples'; import packageJson from '@twilio-paste/combobox/package.json'; import ComponentPageLayout from '../../../layouts/ComponentPageLayout'; @@ -286,6 +289,22 @@ Use an empty state to indicate to a user that their input does not match any val A Combobox consists of a label, an input and a listbox. + + Combobox and Popover + + The Combobox listbox is rendered in a Reakit Portal to control positioning and maintain accessibility. When Combobox is placed in a Popover (which is also a Reakit Portal under the hood), add usePortal=false to your Combobox to prevent interaction bugs caused by nested portals. + + + + + {popoverExample} + + ### Positioning form fields Stack form fields vertically with `$space-80` spacing between each field. Avoid placing multiple form fields on the diff --git a/packages/paste-website/src/pages/components/multiselect-combobox/index.mdx b/packages/paste-website/src/pages/components/multiselect-combobox/index.mdx index f46d37fb30..c6ffd17228 100644 --- a/packages/paste-website/src/pages/components/multiselect-combobox/index.mdx +++ b/packages/paste-website/src/pages/components/multiselect-combobox/index.mdx @@ -22,6 +22,8 @@ import {CloseIcon} from '@twilio-paste/icons/esm/CloseIcon'; import {AttachIcon} from '@twilio-paste/icons/esm/AttachIcon'; import {UnorderedList, ListItem} from '@twilio-paste/list'; import {Callout, CalloutHeading, CalloutText} from '@twilio-paste/callout'; +import { Popover, PopoverButton, PopoverContainer } from "@twilio-paste/popover"; +import {InlineCode} from '@twilio-paste/inline-code' import {DoDont, Do, Dont} from '../../../components/DoDont'; import filter from 'lodash/filter'; import {SidebarCategoryRoutes} from '../../../constants'; @@ -38,6 +40,7 @@ import { emptyStateExample, maxHeightExample, stateHookExample, + popoverExample } from '../../../component-examples/MultiselectComboboxExamples'; import packageJson from '@twilio-paste/combobox/package.json'; import ComponentPageLayout from '../../../layouts/ComponentPageLayout'; @@ -281,6 +284,22 @@ Use an empty state to indicate to a user that their input does not match any val A Multiselect Combobox is comprised of a label, an input and a listbox. + + Combobox and Popover + + The Multiselect Combobox listbox is rendered in a Reakit Portal to control positioning and maintain accessibility. When Multiselect Combobox is placed in a Popover (which is also a Reakit Portal under the hood), add usePortal=false to your Multiselect Combobox to prevent interaction bugs caused by nested portals. + + + + + {popoverExample} + + ### Positioning form fields Stack form fields vertically with `$space-80` spacing between each field. Avoid placing multiple form fields on the