fix(popover): correct position logic (#4498)

* fix(popover): invalid placement logic * chore(select): add story with popover position * chore: add changeset * chore: add pattern of popover story * chore: fix changeset
heroui-inc · Jan 5, 2025 · 33e0418 · 33e0418
1 parent 0c45fed
commit 33e0418
Show file tree

Hide file tree

Showing 10 changed files with 157 additions and 92 deletions.
diff --git a/.changeset/lazy-rice-wash.md b/.changeset/lazy-rice-wash.md
@@ -0,0 +1,6 @@
+---
+"@nextui-org/dropdown": patch
+"@nextui-org/popover": patch
+---
+
+Fix incorrect initial popover animation and arrow display (#4466)
diff --git a/packages/components/autocomplete/stories/autocomplete.stories.tsx b/packages/components/autocomplete/stories/autocomplete.stories.tsx
@@ -1187,3 +1187,23 @@ export const CustomItemHeight = {
     itemHeight: 40,
   },
 };
+
+export const PopoverTopOrBottom = {
+  args: {
+    ...defaultProps,
+  },
+  render: (args) => (
+    <div className="relative h-screen w-screen">
+      <div className="absolute top-0 p-8">
+        <div className="w-48">
+          <Template {...args} />
+        </div>
+      </div>
+      <div className="absolute top-1/2 p-8">
+        <div className="w-48">
+          <Template {...args} />
+        </div>
+      </div>
+    </div>
+  ),
+};
diff --git a/packages/components/dropdown/package.json b/packages/components/dropdown/package.json
@@ -48,7 +48,6 @@
     "@nextui-org/shared-utils": "workspace:*",
     "@react-aria/focus": "3.19.0",
     "@react-aria/menu": "3.16.0",
-    "@react-aria/overlays": "3.24.0",
     "@react-aria/utils": "3.26.0",
     "@react-stately/menu": "3.9.0",
     "@react-types/menu": "3.9.13"

diff --git a/packages/components/dropdown/src/use-dropdown.ts b/packages/components/dropdown/src/use-dropdown.ts
@@ -9,12 +9,11 @@ import {useMenuTrigger} from "@react-aria/menu";
 import {dropdown} from "@nextui-org/theme";
 import {clsx} from "@nextui-org/shared-utils";
 import {ReactRef, mergeRefs} from "@nextui-org/react-utils";
-import {ariaShouldCloseOnInteractOutside, toReactAriaPlacement} from "@nextui-org/aria-utils";
+import {ariaShouldCloseOnInteractOutside} from "@nextui-org/aria-utils";
 import {useMemo, useRef} from "react";
 import {mergeProps} from "@react-aria/utils";
 import {MenuProps} from "@nextui-org/menu";
 import {CollectionElement} from "@react-types/shared";
-import {useOverlayPosition} from "@react-aria/overlays";
 
 interface Props extends HTMLNextUIProps<"div"> {
   /**
@@ -78,8 +77,6 @@ const getCloseOnSelect = <T extends object>(
   return props?.closeOnSelect;
 };
 
-const DEFAULT_PLACEMENT = "bottom";
-
 export function useDropdown(props: UseDropdownProps): UseDropdownReturn {
   const globalContext = useProviderContext();
 
@@ -92,17 +89,13 @@ export function useDropdown(props: UseDropdownProps): UseDropdownReturn {
     isDisabled,
     type = "menu",
     trigger = "press",
-    placement: placementProp = DEFAULT_PLACEMENT,
+    placement = "bottom",
     closeOnSelect = true,
     shouldBlockScroll = true,
     classNames: classNamesProp,
     disableAnimation = globalContext?.disableAnimation ?? false,
     onClose,
     className,
-    containerPadding = 12,
-    offset = 7,
-    crossOffset = 0,
-    shouldFlip = true,
     ...otherProps
   } = props;
 
@@ -139,17 +132,6 @@ export function useDropdown(props: UseDropdownProps): UseDropdownReturn {
     [className],
   );
 
-  const {placement} = useOverlayPosition({
-    isOpen: state.isOpen,
-    targetRef: triggerRef,
-    overlayRef: popoverRef,
-    placement: toReactAriaPlacement(placementProp),
-    offset,
-    crossOffset,
-    shouldFlip,
-    containerPadding,
-  });
-
   const onMenuAction = (menuCloseOnSelect?: boolean) => {
     if (menuCloseOnSelect !== undefined && !menuCloseOnSelect) {
       return;
@@ -164,7 +146,7 @@ export function useDropdown(props: UseDropdownProps): UseDropdownReturn {
 
     return {
       state,
-      placement: placement || DEFAULT_PLACEMENT,
+      placement,
       ref: popoverRef,
       disableAnimation,
       shouldBlockScroll,

diff --git a/packages/components/dropdown/stories/dropdown.stories.tsx b/packages/components/dropdown/stories/dropdown.stories.tsx
@@ -835,3 +835,28 @@ export const WithShouldBlockScroll = {
     ...defaultProps,
   },
 };
+
+export const Placements = {
+  args: {
+    ...defaultProps,
+  },
+  render: (args) => (
+    <div className="inline-grid grid-cols-3 gap-4">
+      <Template {...args} label="Top Start" placement="top-start" />
+      <Template {...args} label="Top" placement="top" />
+      <Template {...args} label="Top End" placement="top-end" />
+
+      <Template {...args} label="Bottom Start" placement="bottom-start" />
+      <Template {...args} label="Bottom" placement="bottom" />
+      <Template {...args} label="Bottom End" placement="bottom-end" />
+
+      <Template {...args} label="Right Start" placement="right-start" />
+      <Template {...args} label="Right" placement="right" />
+      <Template {...args} label="Right End" placement="right-end" />
+
+      <Template {...args} label="Left Start" placement="left-start" />
+      <Template {...args} label="Left" placement="left" />
+      <Template {...args} label="Left End" placement="left-end" />
+    </div>
+  ),
+};
diff --git a/packages/components/popover/src/free-solo-popover.tsx b/packages/components/popover/src/free-solo-popover.tsx
@@ -58,7 +58,7 @@ const FreeSoloPopoverWrapper = forwardRef<"div", FreeSoloPopoverWrapperProps>(
         // @ts-ignore
         transformOrigin,
       };
-    } else {
+    } else if (placement) {
       style = {
         ...style,
         ...getTransformOrigins(placement === "center" ? "top" : placement),

diff --git a/packages/components/popover/src/popover-content.tsx b/packages/components/popover/src/popover-content.tsx
@@ -80,6 +80,9 @@ const PopoverContent = (props: PopoverContentProps) => {
     );
   }, [backdrop, disableAnimation, getBackdropProps]);
 
+  const style = placement
+    ? getTransformOrigins(placement === "center" ? "top" : placement)
+    : undefined;
   const contents = (
     <>
       {disableAnimation ? (
@@ -90,9 +93,7 @@ const PopoverContent = (props: PopoverContentProps) => {
             animate="enter"
             exit="exit"
             initial="initial"
-            style={{
-              ...getTransformOrigins(placement === "center" ? "top" : placement),
-            }}
+            style={style}
             variants={TRANSITION_VARIANTS.scaleSpringOpacity}
             {...motionProps}
           >

diff --git a/packages/components/popover/src/use-popover.ts b/packages/components/popover/src/use-popover.ts
@@ -8,6 +8,7 @@ import {OverlayTriggerState, useOverlayTriggerState} from "@react-stately/overla
 import {useFocusRing} from "@react-aria/focus";
 import {ariaHideOutside, useOverlayTrigger, usePreventScroll} from "@react-aria/overlays";
 import {OverlayTriggerProps} from "@react-types/overlays";
+import {getShouldUseAxisPlacement} from "@nextui-org/aria-utils";
 import {
   HTMLNextUIProps,
   mapPropsVariants,
@@ -152,7 +153,11 @@ export function usePopover(originalProps: UsePopoverProps) {
 
   const state = stateProp || innerState;
 
-  const {popoverProps, underlayProps, placement} = useReactAriaPopover(
+  const {
+    popoverProps,
+    underlayProps,
+    placement: ariaPlacement,
+  } = useReactAriaPopover(
     {
       triggerRef,
       isNonModal,
@@ -174,6 +179,16 @@ export function usePopover(originalProps: UsePopoverProps) {
     state,
   );
 
+  const placement = useMemo(() => {
+    // If ariaPlacement is null, popoverProps.style isn't set,
+    // so we return null to avoid an incorrect animation value.
+    if (!ariaPlacement) {
+      return null;
+    }
+
+    return getShouldUseAxisPlacement(ariaPlacement, placementProp) ? ariaPlacement : placementProp;
+  }, [ariaPlacement, placementProp]);
+
   const {triggerProps} = useOverlayTrigger({type: triggerType}, state, triggerRef);
 
   const {isFocusVisible, isFocused, focusProps} = useFocusRing();
@@ -206,7 +221,7 @@ export function usePopover(originalProps: UsePopoverProps) {
     "data-focus": dataAttr(isFocused),
     "data-arrow": dataAttr(showArrow),
     "data-focus-visible": dataAttr(isFocusVisible),
-    "data-placement": getArrowPlacement(placement || DEFAULT_PLACEMENT, placementProp),
+    "data-placement": ariaPlacement ? getArrowPlacement(ariaPlacement, placementProp) : undefined,
     ...mergeProps(focusProps, dialogPropsProp, props),
     className: slots.base({class: clsx(baseStyles)}),
     style: {
@@ -220,10 +235,10 @@ export function usePopover(originalProps: UsePopoverProps) {
       "data-slot": "content",
       "data-open": dataAttr(state.isOpen),
       "data-arrow": dataAttr(showArrow),
-      "data-placement": getArrowPlacement(placement || DEFAULT_PLACEMENT, placementProp),
+      "data-placement": ariaPlacement ? getArrowPlacement(ariaPlacement, placementProp) : undefined,
       className: slots.content({class: clsx(classNames?.content, props.className)}),
     }),
-    [slots, state.isOpen, showArrow, placement, placementProp, classNames],
+    [slots, state.isOpen, showArrow, placement, placementProp, classNames, ariaPlacement],
   );
 
   const onPress = useCallback(
@@ -307,7 +322,7 @@ export function usePopover(originalProps: UsePopoverProps) {
     classNames,
     showArrow,
     triggerRef,
-    placement: placement || DEFAULT_PLACEMENT,
+    placement,
     isNonModal,
     popoverRef: domRef,
     portalContainer,

diff --git a/packages/components/select/stories/select.stories.tsx b/packages/components/select/stories/select.stories.tsx
@@ -1495,3 +1495,23 @@ export const ValidationBehaviorAria = {
     ...defaultProps,
   },
 };
+
+export const PopoverTopOrBottom = {
+  args: {
+    ...defaultProps,
+  },
+  render: (args) => (
+    <div className="relative h-screen w-screen">
+      <div className="absolute top-0 p-8">
+        <div className="w-48">
+          <Template {...args} />
+        </div>
+      </div>
+      <div className="absolute top-1/2 p-8">
+        <div className="w-48">
+          <Template {...args} />
+        </div>
+      </div>
+    </div>
+  ),
+};