feat(Popup)!: fix review comments

gravity-ui · Nov 18, 2024 · 35e6b09 · 35e6b09
1 parent c49485c
commit 35e6b09
Show file tree

Hide file tree

Showing 11 changed files with 111 additions and 106 deletions.
diff --git a/src/components/ActionTooltip/ActionTooltip.tsx b/src/components/ActionTooltip/ActionTooltip.tsx
@@ -60,7 +60,7 @@ export function ActionTooltip(props: ActionTooltipProps) {
                 style={style}
                 open={tooltipVisible && !disabled}
                 placement={placement}
-                anchorEl={anchorElement ?? undefined}
+                anchorEl={anchorElement}
                 disableEscapeKeyDown
                 disableOutsideClick
                 disableLayer

diff --git a/src/components/Popover/README.md b/src/components/Popover/README.md
diff --git a/src/components/Popover/index.ts b/src/components/Popover/index.ts
@@ -4,6 +4,6 @@ export type {
     PopoverProps,
     PopoverInstanceProps,
     PopoverAnchorRef,
-    PopoverAnchorEl,
+    PopoverAnchorElement,
 } from './types';
 export {PopoverBehavior} from './config';
diff --git a/src/components/Popover/types.ts b/src/components/Popover/types.ts
@@ -1,4 +1,4 @@
-import type {PopupAnchorEl, PopupAnchorRef, PopupOffset, PopupProps} from '../Popup';
+import type {PopupAnchorElement, PopupAnchorRef, PopupOffset, PopupProps} from '../Popup';
 
 import type {ButtonsProps} from './components/Buttons/Buttons';
 import type {ContentProps} from './components/Content/Content';
@@ -92,7 +92,7 @@ export type PopoverBehaviorProps = {
 
 export type PopoverTheme = 'info' | 'special' | 'announcement';
 export type PopoverAnchorRef = PopupAnchorRef;
-export type PopoverAnchorEl = PopupAnchorEl;
+export type PopoverAnchorElement = PopupAnchorElement;
 
 export type PopoverDefaultProps = {
     /** Whether the tooltip initially opened */

diff --git a/src/components/Popup/Popup.tsx b/src/components/Popup/Popup.tsx
@@ -34,8 +34,9 @@ import {getCSSTransitionClassNames} from '../utils/transition';
 
 import {PopupArrow} from './PopupArrow';
 import {AUTO_PLACEMENTS} from './constants';
-import type {PopupAnchorEl, PopupAnchorRef, PopupOffset, PopupPlacement} from './types';
-import {getAnchorElementAndRef, getOffsetValue} from './utils';
+import {useAnchor} from './hooks';
+import type {PopupAnchorElement, PopupAnchorRef, PopupOffset, PopupPlacement} from './types';
+import {getOffsetValue} from './utils';
 
 import './Popup.scss';
 
@@ -47,20 +48,20 @@ export interface PopupProps extends DOMProps, LayerExtendableProps, QAProps {
     keepMounted?: boolean;
     /** Render an arrow pointing to the anchor */
     hasArrow?: boolean;
-    /** floating-ui strategy */
+    /** Floating UI strategy */
     strategy?: Strategy;
     /** floating element placement */
     placement?: PopupPlacement;
     /** floating element offset relative to anchor */
     offset?: PopupOffset;
     /** floating element anchor */
-    anchorEl?: PopupAnchorEl;
+    anchorEl?: PopupAnchorElement | null;
     /** floating element anchor ref object */
     anchorRef?: PopupAnchorRef;
-    /** floating-ui middlewares. If set, they will completely overwrite the default middlewares. */
+    /** Floating UI middlewares. If set, they will completely overwrite the default middlewares. */
     middlewares?: Middleware[];
-    /** floating-ui root context to provide interactions */
-    rootContext?: FloatingRootContext<ReferenceType>;
+    /** Floating UI context to provide interactions */
+    floatingContext?: FloatingRootContext<ReferenceType>;
     /** Additional floating element props to provide interactions */
     floatingProps?: Record<string, unknown>;
     /** Do not use `LayerManager` on stacking popups */
@@ -116,11 +117,11 @@ export function Popup({
     hasArrow = false,
     open,
     strategy,
-    placement = 'bottom-start',
+    placement = 'top',
     offset = 4,
     anchorEl,
     anchorRef,
-    rootContext,
+    floatingContext,
     floatingProps,
     disableEscapeKeyDown,
     disableOutsideClick,
@@ -158,8 +159,7 @@ export function Popup({
     const containerRef = React.useRef<HTMLDivElement>(null);
     const [arrowElement, setArrowElement] = React.useState<HTMLElement | null>(null);
 
-    const anchor = getAnchorElementAndRef(anchorEl, anchorRef);
-
+    const anchor = useAnchor(anchorEl, anchorRef);
     const offsetValue = getOffsetValue(offset, hasArrow);
 
     let placementValue: Placement | undefined;
@@ -209,7 +209,7 @@ export function Popup({
         placement: actualPlacement,
         middlewareData,
     } = useFloating({
-        rootContext,
+        floatingContext,
         strategy,
         placement: placementValue,
         open,

diff --git a/src/components/Popup/README.md b/src/components/Popup/README.md
@@ -8,15 +8,13 @@
 import {Popup} from '@gravity-ui/uikit';
 ```
 
-`Popup` can be used to display floating content above the page. It is a wrapper around [floating-ui](https://floating-ui.com)
+`Popup` can be used to display floating content above the page. It is a wrapper around [Floating UI](https://floating-ui.com)
 with some defaults. To manage `Popup` visibility, use the `open` property.
 The `Popup` child components are rendered inside the [`Portal`](../Portal) component. To disable this behavior, use the `disablePortal` property.
 
 ## Anchor
 
-To specify the anchor of a floating element, you can use either the `anchorEl` or `anchorRef` property.
-The `anchorRef` property accepts a ref object of a DOM element.
-The `anchorEl` property accepts a DOM element itself and has a higher priority.
+To specify the anchor of a floating element, you can use either the `anchorEl` property.
 
 <!--LANDING_BLOCK
 
@@ -28,7 +26,7 @@ const [open, setOpen] = React.useState(false);
 <Button ref={buttonRef} onClick={() => setOpen((prevOpen) => !prevOpen)}>
   Toggle Popup
 </Button>
-<Popup anchorRef={buttonRef} open={open} placement="bottom">
+<Popup anchorEl={buttonRef.current} open={open} placement="bottom">
   Content
 </Popup>
 `}>
@@ -46,7 +44,7 @@ const [open, setOpen] = React.useState(false);
 <Button ref={buttonRef} onClick={() => setOpen((prevOpen) => !prevOpen)}>
   Toggle Popup
 </Button>
-<Popup anchorRef={buttonRef} open={open} placement="bottom">
+<Popup anchorEl={buttonRef.current} open={open} placement="bottom">
   Content
 </Popup>
 ```
@@ -67,18 +65,18 @@ It is also acceptable to use the values `auto`, `auto-start`, `auto-end` to use
 const boxRef = React.useRef(null);
 
 <div ref={boxRef} />
-<Popup open anchorRef={boxRef} placement="top-start">Top Start</Popup>
-<Popup open anchorRef={boxRef} placement="top">Top</Popup>
-<Popup open anchorRef={boxRef} placement="top-end">Top End</Popup>
-<Popup open anchorRef={boxRef} placement="right-start">Right Start</Popup>
-<Popup open anchorRef={boxRef} placement="right">Right</Popup>
-<Popup open anchorRef={boxRef} placement="right-end">Right End</Popup>
-<Popup open anchorRef={boxRef} placement="bottom-end">Bottom End</Popup>
-<Popup open anchorRef={boxRef} placement="bottom">Bottom</Popup>
-<Popup open anchorRef={boxRef} placement="bottom-start">Bottom Start</Popup>
-<Popup open anchorRef={boxRef} placement="left-end">Left End</Popup>
-<Popup open anchorRef={boxRef} placement="left">Left</Popup>
-<Popup open anchorRef={boxRef} placement="left-start">Left Start</Popup>
+<Popup open anchorEl={boxRef.current} placement="top-start">Top Start</Popup>
+<Popup open anchorEl={boxRef.current} placement="top">Top</Popup>
+<Popup open anchorEl={boxRef.current} placement="top-end">Top End</Popup>
+<Popup open anchorEl={boxRef.current} placement="right-start">Right Start</Popup>
+<Popup open anchorEl={boxRef.current} placement="right">Right</Popup>
+<Popup open anchorEl={boxRef.current} placement="right-end">Right End</Popup>
+<Popup open anchorEl={boxRef.current} placement="bottom-end">Bottom End</Popup>
+<Popup open anchorEl={boxRef.current} placement="bottom">Bottom</Popup>
+<Popup open anchorEl={boxRef.current} placement="bottom-start">Bottom Start</Popup>
+<Popup open anchorEl={boxRef.current} placement="left-end">Left End</Popup>
+<Popup open anchorEl={boxRef.current} placement="left">Left</Popup>
+<Popup open anchorEl={boxRef.current} placement="left-start">Left Start</Popup>
 `}>
     <UIKitExamples.PopupPlacementExample/>
 </ExampleBlock>
@@ -89,8 +87,7 @@ LANDING_BLOCK-->
 
 | Name                 | Description                                                                                |                             Type                              |             Default              |
 | :------------------- | :----------------------------------------------------------------------------------------- | :-----------------------------------------------------------: | :------------------------------: |
-| anchorEl             | Anchor element. Can also be `VirtualElement`                                               |                        `PopupAnchorEl`                        |                                  |
-| anchorRef            | A ref object of an anchor element.                                                         |               `React.RefObject<PopupAnchorEl>`                |                                  |
+| anchorEl             | Anchor element. Can also be `VirtualElement`                                               |                     `PopupAnchorElement`                      |                                  |
 | autoFocus            | While open, the focus will be set to the first interactive element in the content          |                           `boolean`                           |             `false`              |
 | children             | Any React content                                                                          |                       `React.ReactNode`                       |                                  |
 | className            | HTML `class` attribute for root node                                                       |                           `string`                            |                                  |
@@ -104,9 +101,9 @@ LANDING_BLOCK-->
 | hasArrow             | Render an arrow pointing to the anchor                                                     |                           `boolean`                           |             `false`              |
 | id                   | HTML `id` attribute                                                                        |                           `string`                            |                                  |
 | keepMounted          | `Popup` will not be removed from the DOM upon hiding                                       |                           `boolean`                           |             `false`              |
-| middlewares          | `Floating-ui` middlewares. If set, they will completely overwrite the default middlewares. |                      `Array<Middleware>`                      |                                  |
-| offset               | `Floating-ui` offset value                                                                 |                         `PopupOffset`                         |               `4`                |
-| rootContext          | `Floating-ui` root context to provide interactions                                         |                     `FloatingRootContext`                     |                                  |
+| middlewares          | `Floating UI` middlewares. If set, they will completely overwrite the default middlewares. |                      `Array<Middleware>`                      |                                  |
+| offset               | `Floating UI` offset value                                                                 |                         `PopupOffset`                         |               `4`                |
+| floatingContext      | `Floating UI` context to provide interactions                                              |                     `FloatingRootContext`                     |                                  |
 | floatingProps        | Additional floating element props to provide interactions                                  |                   `Record<string, unknown>`                   |                                  |
 | onBlur               | `blur` event handler                                                                       |                          `Function`                           |                                  |
 | onClose              | Handle `Popup` close event                                                                 |                          `Function`                           |                                  |
@@ -121,15 +118,15 @@ LANDING_BLOCK-->
 | onTransitionExit     | On start close popup animation                                                             |                          `Function`                           |                                  |
 | onTransitionExited   | On finish close popup animation                                                            |                          `Function`                           |                                  |
 | open                 | Manages `Popup` visibility                                                                 |                           `boolean`                           |             `false`              |
-| placement            | `Floating-ui` placement                                                                    | `Placement` `Array<Placement>` `auto` `auto-start` `auto-end` |                                  |
+| placement            | `Floating UI` placement                                                                    | `Placement` `Array<Placement>` `auto` `auto-start` `auto-end` |              `top`               |
 | qa                   | Test attribute (`data-qa`)                                                                 |                           `string`                            |                                  |
 | restoreFocus         | If true, the focus will return to the anchor element                                       |                           `boolean`                           |             `false`              |
 | restoreFocusRef      | Element the focus will be restored to                                                      |                       `React.RefObject`                       |                                  |
 | aria-labelledby      | `aria-labelledby` attribute, prefer this attribute if you have visible caption             |                           `string`                            |                                  |
 | aria-label           | `aria-label` attribute, use this attribute only if you didn't have visible caption         |                           `string`                            |                                  |
 | aria-modal           | The `aria-modal` attribute indicates whether an element is modal when displayed.           |                         `Booleanish`                          |       value of `focusTrap`       |
 | role                 | The accessibility role for popup                                                           |                           `string`                            | `dialog` if `aria-modal` is true |
-| strategy             | `floating-ui` positioning strategy                                                         |                      `absolute` `fixed`                       |            `absolute`            |
+| strategy             | `Floating UI` positioning strategy                                                         |                      `absolute` `fixed`                       |            `absolute`            |
 | style                | HTML `style` attribute for root node                                                       |                           `string`                            |                                  |
 
 ## CSS API

diff --git a/src/components/Popup/__stories__/Popup.stories.tsx b/src/components/Popup/__stories__/Popup.stories.tsx
@@ -24,7 +24,12 @@ export const Default: Story = {
 
         return (
             <React.Fragment>
-                <Popup {...props} open={open} anchorRef={anchorRef} onClose={() => setOpen(false)}>
+                <Popup
+                    {...props}
+                    open={open}
+                    anchorEl={anchorRef.current}
+                    onClose={() => setOpen(false)}
+                >
                     <div style={{padding: 10}}>Popup content</div>
                 </Popup>
                 <div

diff --git a/src/components/Popup/hooks.ts b/src/components/Popup/hooks.ts
@@ -0,0 +1,24 @@
+import React from 'react';
+
+import type {PopupAnchorElement, PopupAnchorRef} from './types';
+
+export function useAnchor(
+    anchorEl: PopupAnchorElement | null | undefined,
+    anchorRef: PopupAnchorRef | undefined,
+): {
+    element: PopupAnchorElement | null | undefined;
+    ref: PopupAnchorRef | undefined;
+} {
+    const anchorElementRef = React.useRef(anchorEl ?? null);
+    React.useEffect(() => {
+        anchorElementRef.current = anchorEl ?? null;
+    }, [anchorEl]);
+
+    if (anchorEl !== undefined) {
+        return {element: anchorEl, ref: anchorElementRef};
+    } else if (anchorRef) {
+        return {element: anchorRef.current, ref: anchorRef};
+    }
+
+    return {element: undefined, ref: undefined};
+}
diff --git a/src/components/Popup/types.ts b/src/components/Popup/types.ts
@@ -4,9 +4,9 @@ import type {AUTO_PLACEMENTS} from './constants';
 
 export type PopupPlacement = (typeof AUTO_PLACEMENTS)[number] | Placement | Placement[];
 
-export type PopupAnchorEl = Element | VirtualElement;
+export type PopupAnchorElement = Element | VirtualElement;
 
-export type PopupAnchorRef = React.RefObject<PopupAnchorEl>;
+export type PopupAnchorRef = React.RefObject<PopupAnchorElement>;
 
 type RemoveFunction<T> = T extends Function ? never : T;
 

diff --git a/src/components/Popup/utils.ts b/src/components/Popup/utils.ts
@@ -1,25 +1,5 @@
-import type {VirtualElement} from '@floating-ui/react';
-
 import {ARROW_SIZE} from './constants';
-import type {PopupAnchorEl, PopupAnchorRef, PopupOffset} from './types';
-
-export function getAnchorElementAndRef(
-    anchorEl: PopupAnchorEl | undefined,
-    anchorRef: PopupAnchorRef | undefined,
-) {
-    let element: Element | VirtualElement | undefined;
-    let ref: React.RefObject<Element | VirtualElement> | undefined;
-
-    if (anchorEl) {
-        element = anchorEl;
-        ref = {current: anchorEl};
-    } else if (anchorRef) {
-        ref = anchorRef;
-        element = anchorRef.current ?? undefined;
-    }
-
-    return {element, ref};
-}
+import type {PopupOffset} from './types';
 
 export function getOffsetValue(offset: PopupOffset, hasArrow: boolean | undefined) {
     let offsetValue = offset;

diff --git a/src/components/Tooltip/Tooltip.tsx b/src/components/Tooltip/Tooltip.tsx
@@ -59,7 +59,7 @@ export const Tooltip = (props: TooltipProps) => {
                 style={style}
                 open={tooltipVisible && !disabled}
                 placement={placement}
-                anchorEl={anchorElement ?? undefined}
+                anchorEl={anchorElement}
                 disablePortal={disablePortal}
                 disableEscapeKeyDown
                 disableOutsideClick