From c4c9b0a39ab5374056f3a543fe858ec5147779d2 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Tue, 26 Sep 2023 17:10:09 -0400 Subject: [PATCH 1/8] add Popover docs and fix focus --- apps/website/src/pages/docs/popover.mdx | 64 ++++++++++++++++ examples/Popover.focus.tsx | 54 +++++++++++++ examples/Popover.main.tsx | 18 +++++ examples/Popover.placement.tsx | 46 +++++++++++ examples/index.tsx | 13 ++++ .../src/core/Popover/Popover.tsx | 76 ++++++++++++------- 6 files changed, 243 insertions(+), 28 deletions(-) create mode 100644 apps/website/src/pages/docs/popover.mdx create mode 100644 examples/Popover.focus.tsx create mode 100644 examples/Popover.main.tsx create mode 100644 examples/Popover.placement.tsx diff --git a/apps/website/src/pages/docs/popover.mdx b/apps/website/src/pages/docs/popover.mdx new file mode 100644 index 00000000000..16ec8e5b067 --- /dev/null +++ b/apps/website/src/pages/docs/popover.mdx @@ -0,0 +1,64 @@ +--- +title: Popover +description: An overlay dialog placed next to a trigger element. +layout: ./_layout.astro +propsPath: '@itwin/itwinui-react/esm/core/Popover/Popover.d.ts' +group: utilities +thumbnail: #TODO +--- + +import PropsTable from '~/components/PropsTable.astro'; +import LiveExample from '~/components/LiveExample.astro'; +import * as AllExamples from 'examples'; + +Popover is a utility component for displaying overlay content in a dialog that is placed relative to a trigger element. + + + + + +By default, Popover does not add any styling. The `applyBackground` prop can be used to add the recommended background, box-shadow, border, etc. + +## Usage + +The content shown inside the Popover is passed using the `content` prop. The trigger element is specified by the child element that Popover wraps around. + +For everything to work correctly, the trigger element must: + +- be a button +- forward its ref +- delegate (spread) any arbitrary props + +If you use a native ` + + + + } + > + + + ); +}; diff --git a/examples/Popover.main.tsx b/examples/Popover.main.tsx new file mode 100644 index 00000000000..ac2a8f66879 --- /dev/null +++ b/examples/Popover.main.tsx @@ -0,0 +1,18 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import * as React from 'react'; +import { Button, Popover } from '@itwin/itwinui-react'; + +export default () => { + return ( + + + + ); +}; diff --git a/examples/Popover.placement.tsx b/examples/Popover.placement.tsx new file mode 100644 index 00000000000..f83900c23d4 --- /dev/null +++ b/examples/Popover.placement.tsx @@ -0,0 +1,46 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import * as React from 'react'; +import { Button, LabeledSelect, Popover } from '@itwin/itwinui-react'; + +export default () => { + const [placement, setPlacement] = + React.useState<(typeof placements)[number]>('bottom-start'); + + return ( + + ({ value: p, label: p }))} + value={placement} + onChange={setPlacement} + style={{ minWidth: '20ch' }} + /> + + } + applyBackground + placement={placement} + > + + + ); +}; + +const placements = [ + 'bottom', + 'bottom-start', + 'bottom-end', + 'top', + 'top-start', + 'top-end', + 'left', + 'left-start', + 'left-end', + 'right', + 'right-start', + 'right-end', +] as const; diff --git a/examples/index.tsx b/examples/index.tsx index c2a663384d8..3e33d39c09c 100644 --- a/examples/index.tsx +++ b/examples/index.tsx @@ -822,6 +822,19 @@ export { OverlaySubExample }; // ---------------------------------------------------------------------------- +import { default as PopoverMainExampleRaw } from './Popover.main'; +export const PopoverMainExample = withThemeProvider(PopoverMainExampleRaw); + +import { default as PopoverPlacementExampleRaw } from './Popover.placement'; +export const PopoverPlacementExample = withThemeProvider( + PopoverPlacementExampleRaw, +); + +import { default as PopoverFocusExampleRaw } from './Popover.focus'; +export const PopoverFocusExample = withThemeProvider(PopoverFocusExampleRaw); + +// ---------------------------------------------------------------------------- + import { default as ProgressLinearMainExampleRaw } from './ProgressLinear.main'; const ProgressLinearMainExample = withThemeProvider( ProgressLinearMainExampleRaw, diff --git a/packages/itwinui-react/src/core/Popover/Popover.tsx b/packages/itwinui-react/src/core/Popover/Popover.tsx index 074f7948c34..16dba6fbec5 100644 --- a/packages/itwinui-react/src/core/Popover/Popover.tsx +++ b/packages/itwinui-react/src/core/Popover/Popover.tsx @@ -23,12 +23,14 @@ import { useFocus, safePolygon, useRole, + FloatingPortal, } from '@floating-ui/react'; import type { SizeOptions, Placement } from '@floating-ui/react'; import { Box, cloneElementWithRef, useControlledState, + useId, useMergedRefs, } from '../utils/index.js'; import type { PolymorphicForwardRefComponent } from '../utils/index.js'; @@ -54,12 +56,10 @@ type PopoverOptions = { onVisibleChange?: (visible: boolean) => void; /** * If true, the popover will close when clicking outside it. + * + * @default true */ closeOnOutsideClick?: boolean; - /** - * Whether the popover should match the width of the trigger. - */ - matchWidth?: boolean; }; // keep public api small to start with @@ -101,6 +101,10 @@ type PopoverInternalProps = { */ trigger?: Partial>; role?: 'dialog' | 'menu' | 'listbox'; + /** + * Whether the popover should match the width of the trigger. + */ + matchWidth?: boolean; }; // ---------------------------------------------------------------------------- @@ -193,8 +197,16 @@ export const usePopover = (options: PopoverOptions & PopoverInternalProps) => { // ---------------------------------------------------------------------------- type PopoverPublicProps = { + /** + * Content displayed inside the popover. + */ content?: React.ReactNode; children?: React.ReactNode; + /** + * Whether the popover adds recommended CSS for background-color, box-shadow, etc. + * + * @default false + */ applyBackground?: boolean; } & PortalProps & PopoverOptions; @@ -209,16 +221,15 @@ export const Popover = React.forwardRef((props, forwardedRef) => { // // popover options visible, - placement, + placement = 'bottom-start', onVisibleChange, - closeOnOutsideClick, - matchWidth, + closeOnOutsideClick = true, // // dom props className, children, content, - applyBackground, + applyBackground = false, ...rest } = props; @@ -227,7 +238,6 @@ export const Popover = React.forwardRef((props, forwardedRef) => { placement, onVisibleChange, closeOnOutsideClick, - matchWidth, role: 'dialog', }); @@ -239,36 +249,46 @@ export const Popover = React.forwardRef((props, forwardedRef) => { setPopoverElement, ); + const triggerId = `${useId()}-trigger`; + const hasAriaLabel = !!props['aria-labelledby'] || !!props['aria-label']; + return ( <> {cloneElementWithRef(children, (children) => ({ + id: children.props.id || triggerId, ...popover.getReferenceProps(children.props), ref: popover.refs.setReference, }))} {popover.open ? ( - - + - - {content} - - - + + {content} + + + + ) : null} From d146a859002a0e53538c30395b813a2ef02eee71 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 27 Sep 2023 12:33:29 -0400 Subject: [PATCH 2/8] fix `DropdownMenu` types --- packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx b/packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx index a9e5bdfda3d..f006205a0ad 100644 --- a/packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx +++ b/packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx @@ -16,7 +16,7 @@ import type { PortalProps, } from '../utils/index.js'; import { Menu } from '../Menu/Menu.js'; -import { Popover, usePopover } from '../Popover/Popover.js'; +import { usePopover } from '../Popover/Popover.js'; export type DropdownMenuProps = { /** @@ -37,7 +37,7 @@ export type DropdownMenuProps = { */ children: React.ReactNode; } & Pick< - React.ComponentProps, + Parameters[0], 'visible' | 'onVisibleChange' | 'placement' | 'matchWidth' > & React.ComponentPropsWithoutRef<'ul'> & From 94396bced954bab8acff5caee3362ac1838e0eee Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 27 Sep 2023 12:50:56 -0400 Subject: [PATCH 3/8] slight reword --- apps/website/src/pages/docs/popover.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/website/src/pages/docs/popover.mdx b/apps/website/src/pages/docs/popover.mdx index 16ec8e5b067..499b4c95980 100644 --- a/apps/website/src/pages/docs/popover.mdx +++ b/apps/website/src/pages/docs/popover.mdx @@ -41,7 +41,7 @@ Popover handles positioning using an external library called [Floating UI](float ### Portals -It is important to know that before calculating the position, the popover gets [portaled](https://react.dev/reference/react-dom/createPortal) into the nearest `ThemeProvider` to avoid [stacking context](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Understanding_z-index/Stacking_context) issues. This behavior can be controlled using the Popover's `portal` prop or the ThemeProvider's `portalContainer` prop. The portaling technique can often lead to issues with keyboard accessibility; Popover is designed to handle these accessibility considerations. +It is important to know that before calculating the position, the popover gets [portaled](https://react.dev/reference/react-dom/createPortal) into the nearest `ThemeProvider` to avoid [stacking context](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Understanding_z-index/Stacking_context) issues. This behavior can be controlled using the Popover's `portal` prop or the ThemeProvider's `portalContainer` prop. Using portals can often lead to issues with keyboard accessibility, so Popover adds some additional logic (described below). ## Accessibility From bd5ac0e38cc044bb363897171cfaf25695c168fc Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Wed, 27 Sep 2023 15:25:27 -0400 Subject: [PATCH 4/8] fix Popover focus --- .../src/core/DropdownMenu/DropdownMenu.tsx | 4 +- .../src/core/Popover/Popover.tsx | 76 ++++++++++++------- 2 files changed, 50 insertions(+), 30 deletions(-) diff --git a/packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx b/packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx index a9e5bdfda3d..f006205a0ad 100644 --- a/packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx +++ b/packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx @@ -16,7 +16,7 @@ import type { PortalProps, } from '../utils/index.js'; import { Menu } from '../Menu/Menu.js'; -import { Popover, usePopover } from '../Popover/Popover.js'; +import { usePopover } from '../Popover/Popover.js'; export type DropdownMenuProps = { /** @@ -37,7 +37,7 @@ export type DropdownMenuProps = { */ children: React.ReactNode; } & Pick< - React.ComponentProps, + Parameters[0], 'visible' | 'onVisibleChange' | 'placement' | 'matchWidth' > & React.ComponentPropsWithoutRef<'ul'> & diff --git a/packages/itwinui-react/src/core/Popover/Popover.tsx b/packages/itwinui-react/src/core/Popover/Popover.tsx index 074f7948c34..16dba6fbec5 100644 --- a/packages/itwinui-react/src/core/Popover/Popover.tsx +++ b/packages/itwinui-react/src/core/Popover/Popover.tsx @@ -23,12 +23,14 @@ import { useFocus, safePolygon, useRole, + FloatingPortal, } from '@floating-ui/react'; import type { SizeOptions, Placement } from '@floating-ui/react'; import { Box, cloneElementWithRef, useControlledState, + useId, useMergedRefs, } from '../utils/index.js'; import type { PolymorphicForwardRefComponent } from '../utils/index.js'; @@ -54,12 +56,10 @@ type PopoverOptions = { onVisibleChange?: (visible: boolean) => void; /** * If true, the popover will close when clicking outside it. + * + * @default true */ closeOnOutsideClick?: boolean; - /** - * Whether the popover should match the width of the trigger. - */ - matchWidth?: boolean; }; // keep public api small to start with @@ -101,6 +101,10 @@ type PopoverInternalProps = { */ trigger?: Partial>; role?: 'dialog' | 'menu' | 'listbox'; + /** + * Whether the popover should match the width of the trigger. + */ + matchWidth?: boolean; }; // ---------------------------------------------------------------------------- @@ -193,8 +197,16 @@ export const usePopover = (options: PopoverOptions & PopoverInternalProps) => { // ---------------------------------------------------------------------------- type PopoverPublicProps = { + /** + * Content displayed inside the popover. + */ content?: React.ReactNode; children?: React.ReactNode; + /** + * Whether the popover adds recommended CSS for background-color, box-shadow, etc. + * + * @default false + */ applyBackground?: boolean; } & PortalProps & PopoverOptions; @@ -209,16 +221,15 @@ export const Popover = React.forwardRef((props, forwardedRef) => { // // popover options visible, - placement, + placement = 'bottom-start', onVisibleChange, - closeOnOutsideClick, - matchWidth, + closeOnOutsideClick = true, // // dom props className, children, content, - applyBackground, + applyBackground = false, ...rest } = props; @@ -227,7 +238,6 @@ export const Popover = React.forwardRef((props, forwardedRef) => { placement, onVisibleChange, closeOnOutsideClick, - matchWidth, role: 'dialog', }); @@ -239,36 +249,46 @@ export const Popover = React.forwardRef((props, forwardedRef) => { setPopoverElement, ); + const triggerId = `${useId()}-trigger`; + const hasAriaLabel = !!props['aria-labelledby'] || !!props['aria-label']; + return ( <> {cloneElementWithRef(children, (children) => ({ + id: children.props.id || triggerId, ...popover.getReferenceProps(children.props), ref: popover.refs.setReference, }))} {popover.open ? ( - - + - - {content} - - - + + {content} + + + + ) : null} From 2e949e65d8a794a3bcb0710df4a7b9db90e247c4 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Mon, 2 Oct 2023 18:15:39 -0400 Subject: [PATCH 5/8] update example --- apps/website/src/pages/docs/popover.mdx | 4 +-- examples/Popover.focus.tsx | 35 ++++++++++++++----------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/apps/website/src/pages/docs/popover.mdx b/apps/website/src/pages/docs/popover.mdx index 499b4c95980..dcacd0187f4 100644 --- a/apps/website/src/pages/docs/popover.mdx +++ b/apps/website/src/pages/docs/popover.mdx @@ -51,9 +51,9 @@ The popover should generally be labeled using [`aria-labelledby`](https://develo When the popover opens, keyboard focus will be moved to the popover. When it closes, keyboard focus will move back to the trigger element. Additionally, keyboard focus will move back to the trigger when tabbing out of it. -If you have a good candidate for receiving focus, then you can manually `focus()` it using a [ref](https://react.dev/learn/manipulating-the-dom-with-refs). This should be done thoughtfully, and the element receiving focus should usually be located near the beginning of the popover, with not too much content preceding it. +If you have a good candidate for receiving focus, then you can manually `focus()` it using a [ref](https://react.dev/learn/manipulating-the-dom-with-refs) or use `autoFocus` where possible. This should be done thoughtfully, and the element receiving focus should usually be located near the beginning of the popover, with not too much content preceding it. -The following example shows how you can focus a button when the popover opens. This button is preceded by a heading and some supplementary text, associated with the popover using `aria-labelledby` and `aria-describedby` respectively. As a result, when the popover opens, both the heading and the description are automatically exposed to a screen reader user. +The following example shows how you can focus an input when the popover opens. This button is preceded by a heading associated with the popover using `aria-labelledby`. As a result, when the popover opens, the heading is automatically announced to a screen reader user. diff --git a/examples/Popover.focus.tsx b/examples/Popover.focus.tsx index 15b79422673..0b4cede1921 100644 --- a/examples/Popover.focus.tsx +++ b/examples/Popover.focus.tsx @@ -3,12 +3,20 @@ * See LICENSE.md in the project root for license terms and full copyright notice. *--------------------------------------------------------------------------------------------*/ import * as React from 'react'; -import { Button, Flex, Popover, Surface, Text } from '@itwin/itwinui-react'; +import { + Button, + Flex, + IconButton, + LabeledInput, + Popover, + Surface, + Text, +} from '@itwin/itwinui-react'; +import { SvgSettings } from '@itwin/itwinui-icons-react'; export default () => { const idPrefix = React.useId(); const headingId = `${idPrefix}-label`; - const descriptionId = `${idPrefix}-description`; const [isOpen, setIsOpen] = React.useState(false); @@ -16,7 +24,6 @@ export default () => { { - Terms and conditions + Settings - - - (TODO: improve text) Lorem ipsum dolor sit amet consectetur - adipisicing elit. Voluptatem, voluptates impedit rem incidunt - excepturi accusamus facilis doloribus saepe quibusdam - reiciendis. - + + + + } > - + + + ); }; From 14a8346c44937be70a2de8a65feebd6cd4d104da Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Mon, 2 Oct 2023 18:18:53 -0400 Subject: [PATCH 6/8] typo --- apps/website/src/pages/docs/popover.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/website/src/pages/docs/popover.mdx b/apps/website/src/pages/docs/popover.mdx index dcacd0187f4..f9b4aa94db3 100644 --- a/apps/website/src/pages/docs/popover.mdx +++ b/apps/website/src/pages/docs/popover.mdx @@ -53,7 +53,7 @@ When the popover opens, keyboard focus will be moved to the popover. When it clo If you have a good candidate for receiving focus, then you can manually `focus()` it using a [ref](https://react.dev/learn/manipulating-the-dom-with-refs) or use `autoFocus` where possible. This should be done thoughtfully, and the element receiving focus should usually be located near the beginning of the popover, with not too much content preceding it. -The following example shows how you can focus an input when the popover opens. This button is preceded by a heading associated with the popover using `aria-labelledby`. As a result, when the popover opens, the heading is automatically announced to a screen reader user. +The following example shows how you can focus an input when the popover opens. This input is preceded by a heading associated with the popover using `aria-labelledby`. As a result, when the popover opens, the heading is automatically announced to a screen reader user, ensuring they didn't miss any content located before the input. From 960ec84a446adbad0dfc81f20165f43ff52c47b8 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Mon, 2 Oct 2023 18:19:35 -0400 Subject: [PATCH 7/8] remove propsPath from frontmatter --- apps/website/src/pages/docs/popover.mdx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/website/src/pages/docs/popover.mdx b/apps/website/src/pages/docs/popover.mdx index f9b4aa94db3..11abbe88287 100644 --- a/apps/website/src/pages/docs/popover.mdx +++ b/apps/website/src/pages/docs/popover.mdx @@ -2,7 +2,6 @@ title: Popover description: An overlay dialog placed next to a trigger element. layout: ./_layout.astro -propsPath: '@itwin/itwinui-react/esm/core/Popover/Popover.d.ts' group: utilities thumbnail: #TODO --- @@ -61,4 +60,4 @@ The following example shows how you can focus an input when the popover opens. T ## Props - + From 22f7001682a6c7e6e07c1694db529be13b6c77a9 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Mon, 2 Oct 2023 18:38:28 -0400 Subject: [PATCH 8/8] cleanup + comment --- examples/Popover.focus.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/examples/Popover.focus.tsx b/examples/Popover.focus.tsx index 0b4cede1921..1a079aefc56 100644 --- a/examples/Popover.focus.tsx +++ b/examples/Popover.focus.tsx @@ -15,8 +15,7 @@ import { import { SvgSettings } from '@itwin/itwinui-icons-react'; export default () => { - const idPrefix = React.useId(); - const headingId = `${idPrefix}-label`; + const headingId = `${React.useId()}-label`; const [isOpen, setIsOpen] = React.useState(false); @@ -36,7 +35,9 @@ export default () => { + {/* this will be focused when popover opens */} +