Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(popover): correct position logic #4498

Merged
merged 5 commits into from
Jan 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/lazy-rice-wash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@nextui-org/dropdown": patch
"@nextui-org/popover": patch
---

Fix incorrect initial popover animation and arrow display (#4466)
Original file line number Diff line number Diff line change
Expand Up @@ -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>
),
};
1 change: 0 additions & 1 deletion packages/components/dropdown/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
24 changes: 3 additions & 21 deletions packages/components/dropdown/src/use-dropdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"> {
/**
Expand Down Expand Up @@ -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();

Expand All @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -164,7 +146,7 @@ export function useDropdown(props: UseDropdownProps): UseDropdownReturn {

return {
state,
placement: placement || DEFAULT_PLACEMENT,
placement,
ref: popoverRef,
disableAnimation,
shouldBlockScroll,
Expand Down
25 changes: 25 additions & 0 deletions packages/components/dropdown/stories/dropdown.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
),
};
2 changes: 1 addition & 1 deletion packages/components/popover/src/free-solo-popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const FreeSoloPopoverWrapper = forwardRef<"div", FreeSoloPopoverWrapperProps>(
// @ts-ignore
transformOrigin,
};
} else {
} else if (placement) {
style = {
...style,
...getTransformOrigins(placement === "center" ? "top" : placement),
Expand Down
7 changes: 4 additions & 3 deletions packages/components/popover/src/popover-content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ const PopoverContent = (props: PopoverContentProps) => {
);
}, [backdrop, disableAnimation, getBackdropProps]);

const style = placement
? getTransformOrigins(placement === "center" ? "top" : placement)
: undefined;
const contents = (
<>
{disableAnimation ? (
Expand All @@ -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}
>
Expand Down
25 changes: 20 additions & 5 deletions packages/components/popover/src/use-popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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]);
Comment on lines +182 to +190
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect initial placement caused animation to be applied in the wrong direction.


const {triggerProps} = useOverlayTrigger({type: triggerType}, state, triggerRef);

const {isFocusVisible, isFocused, focusProps} = useFocusRing();
Expand Down Expand Up @@ -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: {
Expand All @@ -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(
Expand Down Expand Up @@ -307,7 +322,7 @@ export function usePopover(originalProps: UsePopoverProps) {
classNames,
showArrow,
triggerRef,
placement: placement || DEFAULT_PLACEMENT,
placement,
isNonModal,
popoverRef: domRef,
portalContainer,
Expand Down
20 changes: 20 additions & 0 deletions packages/components/select/stories/select.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
),
};
Loading
Loading