-
Notifications
You must be signed in to change notification settings - Fork 11
fix(popover): arrow tracks trigger on collision shift #533
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
base: main
Are you sure you want to change the base?
Changes from all commits
d194931
d0fc992
4d20861
b91ff6b
d0948d8
4f91f56
7770ba1
66cafc6
068d402
c278169
2e9951c
466a057
ab4d8a5
1825205
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -130,6 +130,9 @@ export const popup = componentStyle({ | |
| borderRadius: vars.size.borderRadius[300], | ||
| boxShadow: vars.shadow.md, | ||
|
|
||
| paddingBlock: vars.size.space[150], | ||
| paddingInline: vars.size.space[200], | ||
|
|
||
| backgroundColor: vars.color.background.overlay[100], | ||
|
|
||
| transformOrigin: 'var(--transform-origin)', | ||
|
|
@@ -156,9 +159,6 @@ export const content = componentStyle({ | |
| width: '100%', | ||
| height: '100%', | ||
|
|
||
| paddingBlock: vars.size.space[150], | ||
| paddingInline: vars.size.space[200], | ||
|
|
||
| whiteSpace: 'nowrap', | ||
| transition: `opacity calc(${durationVar} * 0.5) ease, transform ${durationVar} ${easingVar}`, | ||
|
|
||
|
|
@@ -195,23 +195,6 @@ export const arrow = componentStyle({ | |
| transition: `left ${durationVar} ${easingVar}`, | ||
|
|
||
| selectors: { | ||
| '&[data-side="top"]': { | ||
| bottom: '-8px', | ||
| transform: 'rotate(-180deg)', | ||
| }, | ||
| '&[data-side="right"]': { | ||
| left: '-12px', | ||
| transform: 'rotate(-90deg)', | ||
| }, | ||
| '&[data-side="bottom"]': { | ||
| top: '-8px', | ||
| transform: 'rotate(0deg)', | ||
| }, | ||
| '&[data-side="left"]': { | ||
| right: '-12px', | ||
| transform: 'rotate(90deg)', | ||
| }, | ||
|
|
||
|
Comment on lines
-198
to
-214
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have moved the logic responsible for the arrow’s position and rotation to within
|
||
| '&[data-starting-style], &[data-ending-style]': { | ||
| opacity: 0, | ||
| }, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,15 @@ | ||
| 'use client'; | ||
|
|
||
| import type { CSSProperties, ComponentPropsWithoutRef, ReactElement } from 'react'; | ||
| import { forwardRef, useEffect, useMemo, useRef, useState } from 'react'; | ||
| import type { ComponentProps, ReactElement, RefObject } from 'react'; | ||
| import { forwardRef, useEffect, useRef, useState } from 'react'; | ||
|
|
||
| import { NavigationMenu as BaseNavigationMenu } from '@base-ui/react/navigation-menu'; | ||
| import { ChevronDownOutlineIcon } from '@vapor-ui/icons'; | ||
|
|
||
| import { useArrowPosition } from '~/hooks/use-arrow-position'; | ||
| import { useMutationObserverRef } from '~/hooks/use-mutation-observer-ref'; | ||
| import { useRenderElement } from '~/hooks/use-render-element'; | ||
| import { useVaporId } from '~/hooks/use-vapor-id'; | ||
| import { createContext } from '~/libs/create-context'; | ||
| import { vars } from '~/styles/themes.css'; | ||
| import { cn } from '~/utils/cn'; | ||
|
|
@@ -21,6 +23,24 @@ import type { VaporUIComponentProps } from '~/utils/types'; | |
| import type { LinkVariants, ListVariants } from './navigation-menu.css'; | ||
| import * as styles from './navigation-menu.css'; | ||
|
|
||
| /* ------------------------------------------------------------------------------------------------- | ||
| * NavigationMenuArrowContext (internal) | ||
| * -----------------------------------------------------------------------------------------------*/ | ||
|
|
||
| interface NavigationMenuArrowContextValue { | ||
| activeTriggerElement: Element | null; | ||
| setActiveTriggerElement: (element: Element | null) => void; | ||
| positionerRef: RefObject<HTMLElement | null>; | ||
| } | ||
|
|
||
| const [NavigationMenuArrowProvider, useNavigationMenuArrowContext] = | ||
| createContext<NavigationMenuArrowContextValue>({ | ||
| name: 'NavigationMenuArrowContext', | ||
| strict: false, | ||
| hookName: 'useNavigationMenuArrowContext', | ||
| providerName: 'NavigationMenuRoot', | ||
| }); | ||
|
|
||
| type NavigationMenuVariants = ListVariants & LinkVariants; | ||
| type NavigationMenuSharedProps = NavigationMenuVariants & { disabled?: boolean }; | ||
| type NavigationMenuContextType = NavigationMenuSharedProps; | ||
|
|
@@ -47,15 +67,22 @@ export const NavigationMenuRoot = forwardRef<HTMLElement, NavigationMenuRoot.Pro | |
|
|
||
| const { direction } = variantProps; | ||
|
|
||
| const [activeTriggerElement, setActiveTriggerElement] = useState<Element | null>(null); | ||
| const positionerRef = useRef<HTMLElement>(null); | ||
|
Comment on lines
+70
to
+71
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unlike tooltips and popovers, the navigation menu contains multiple triggers within the Root. As a result, there are limitations to specifying the trigger element using a ref, so the element is managed via state. |
||
|
|
||
| return ( | ||
| <NavigationMenuProvider value={variantProps}> | ||
| <BaseNavigationMenu.Root | ||
| ref={ref} | ||
| orientation={direction} | ||
| className={className} | ||
| {...otherProps} | ||
| /> | ||
| </NavigationMenuProvider> | ||
| <NavigationMenuArrowProvider | ||
| value={{ activeTriggerElement, setActiveTriggerElement, positionerRef }} | ||
| > | ||
| <NavigationMenuProvider value={variantProps}> | ||
| <BaseNavigationMenu.Root | ||
| ref={ref} | ||
| orientation={direction} | ||
| className={className} | ||
| {...otherProps} | ||
| /> | ||
| </NavigationMenuProvider> | ||
| </NavigationMenuArrowProvider> | ||
| ); | ||
| }, | ||
| ); | ||
|
|
@@ -149,13 +176,35 @@ export const NavigationMenuTriggerPrimitive = forwardRef< | |
| >((props, ref) => { | ||
| const { disabled: disabledProp, className, ...componentProps } = resolveStyles(props); | ||
| const { size, disabled: contextDisabled } = useNavigationMenuContext(); | ||
| const { setActiveTriggerElement } = useNavigationMenuArrowContext() ?? {}; | ||
|
|
||
| const internalRef = useRef<HTMLButtonElement>(null); | ||
| const composedRef = composeRefs(internalRef, ref); | ||
|
|
||
| useEffect(() => { | ||
| const node = internalRef.current; | ||
| if (!node || !setActiveTriggerElement) return; | ||
|
|
||
| if (node.hasAttribute('data-popup-open')) { | ||
| setActiveTriggerElement(node); | ||
| } | ||
|
|
||
| const observer = new MutationObserver(() => { | ||
| if (node.hasAttribute('data-popup-open')) { | ||
| setActiveTriggerElement(node); | ||
| } | ||
| }); | ||
| observer.observe(node, { attributes: true, attributeFilter: ['data-popup-open'] }); | ||
|
|
||
| return () => observer.disconnect(); | ||
| }, [setActiveTriggerElement]); | ||
|
|
||
| const disabled = disabledProp ?? contextDisabled; | ||
| const dataAttrs = createDataAttributes({ disabled }); | ||
|
|
||
| return ( | ||
| <BaseNavigationMenu.Trigger | ||
| ref={ref} | ||
| ref={composedRef} | ||
| disabled={disabled} | ||
| className={cn(styles.link({ size }), styles.trigger, className)} | ||
| {...dataAttrs} | ||
|
|
@@ -260,10 +309,12 @@ export const NavigationMenuPositionerPrimitive = forwardRef< | |
| className, | ||
| ...componentProps | ||
| } = resolveStyles(props); | ||
| const { positionerRef } = useNavigationMenuArrowContext() ?? {}; | ||
| const composedRef = composeRefs(positionerRef, ref); | ||
|
|
||
| return ( | ||
| <BaseNavigationMenu.Positioner | ||
| ref={ref} | ||
| ref={composedRef} | ||
| side={side} | ||
| align={align} | ||
| sideOffset={sideOffset} | ||
|
|
@@ -290,7 +341,15 @@ export const NavigationMenuPopupPrimitive = forwardRef< | |
| const [side, setSide] = useState<NavigationMenuPositionerPrimitive.Props['side']>(); | ||
| const [align, setAlign] = useState<NavigationMenuPositionerPrimitive.Props['align']>(); | ||
|
|
||
| const position = useMemo(() => getArrowPosition({ side, align }), [side, align]); | ||
| const { activeTriggerElement, positionerRef } = useNavigationMenuArrowContext() ?? {}; | ||
| const arrowDimensions = { width: 16, height: 8 }; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As the |
||
| const arrowStyle = useArrowPosition({ | ||
| triggerElement: activeTriggerElement ?? null, | ||
| positionerElement: positionerRef?.current ?? null, | ||
| side: side ?? 'bottom', | ||
| align: align ?? 'center', | ||
| arrowDimensions, | ||
| }); | ||
|
MaxLee-dev marked this conversation as resolved.
|
||
|
|
||
| const popupRef = useRef<HTMLDivElement>(null); | ||
| const composedRef = composeRefs(popupRef, ref); | ||
|
|
@@ -326,7 +385,13 @@ export const NavigationMenuPopupPrimitive = forwardRef< | |
| className={cn(styles.popup, className)} | ||
| {...componentProps} | ||
| > | ||
| <BaseNavigationMenu.Arrow ref={arrowRef} style={position} className={styles.arrow}> | ||
| <BaseNavigationMenu.Arrow | ||
| ref={arrowRef} | ||
| style={{ | ||
| ...arrowStyle, | ||
| }} | ||
| className={styles.arrow} | ||
| > | ||
| <ArrowIcon /> | ||
| </BaseNavigationMenu.Arrow> | ||
|
|
||
|
|
@@ -343,34 +408,8 @@ const extractPositions = (dataset: DOMStringMap) => { | |
| }; | ||
|
|
||
| /* -----------------------------------------------------------------------------------------------*/ | ||
|
|
||
| type ArrowPositionProps = Pick<NavigationMenuPositionerPrimitive.Props, 'side' | 'align'> & { | ||
| offset?: number; | ||
| }; | ||
|
|
||
| const getArrowPosition = ({ | ||
| side = 'top', | ||
| align = 'center', | ||
| offset = 12, | ||
| }: ArrowPositionProps): CSSProperties => { | ||
| const positionMap = { | ||
| 'top-start': { left: offset, right: 'unset' }, | ||
| 'top-end': { left: 'unset', right: offset }, | ||
| 'bottom-start': { left: offset, right: 'unset' }, | ||
| 'bottom-end': { left: 'unset', right: offset }, | ||
| 'left-start': { top: offset, bottom: 'unset' }, | ||
| 'left-end': { top: 'unset', bottom: offset }, | ||
| 'right-start': { top: offset, bottom: 'unset' }, | ||
| 'right-end': { top: 'unset', bottom: offset }, | ||
| }; | ||
|
|
||
| const key = `${side}-${align}` as keyof typeof positionMap; | ||
| return positionMap[key] || {}; | ||
| }; | ||
|
|
||
| /* -----------------------------------------------------------------------------------------------*/ | ||
|
|
||
| const ArrowIcon = (props: ComponentPropsWithoutRef<'svg'>) => { | ||
| const ArrowIcon = (props: ComponentProps<'svg'>) => { | ||
| const clipId = useVaporId(); | ||
| return ( | ||
| <svg | ||
| width="16" | ||
|
|
@@ -380,19 +419,25 @@ const ArrowIcon = (props: ComponentPropsWithoutRef<'svg'>) => { | |
| xmlns="http://www.w3.org/2000/svg" | ||
| {...props} | ||
| > | ||
| <path | ||
| d="M7.06543 1.17969C7.56267 0.620294 8.43733 0.620294 8.93457 1.17969L14.3301 7.25H1.66992L7.06543 1.17969Z" | ||
| stroke={vars.color.border.normal} | ||
| strokeWidth="1" | ||
| /> | ||
| <path | ||
| d="M8.75926 1.8858C8.36016 1.42019 7.63984 1.42019 7.24074 1.8858L2 8H14L8.75926 1.8858Z" | ||
| fill="currentColor" | ||
| /> | ||
| <g clipPath={`url(#${clipId})`}> | ||
| <path | ||
| d="M7.06543 1.17969C7.56267 0.620294 8.43733 0.620294 8.93457 1.17969L14.3301 7.25H1.66992L7.06543 1.17969Z" | ||
| stroke={vars.color.border.normal} | ||
| strokeWidth="1.5" | ||
| /> | ||
| <path | ||
| d="M8.75926 1.8858C8.36016 1.42019 7.63984 1.42019 7.24074 1.8858L2 8H14L8.75926 1.8858Z" | ||
| fill={vars.color.background.overlay[100]} | ||
| /> | ||
| </g> | ||
| <defs> | ||
| <clipPath id={clipId}> | ||
| <rect width="16" height="8" fill="white" /> | ||
| </clipPath> | ||
| </defs> | ||
| </svg> | ||
| ); | ||
| }; | ||
|
|
||
| /* ------------------------------------------------------------------------------------------------- | ||
| * NavigationMenu.ViewportPrimitive | ||
| * -----------------------------------------------------------------------------------------------*/ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,28 +20,7 @@ export const arrow = componentStyle({ | |
| display: 'flex', | ||
| color: vars.color.background.overlay[100], // It's background-color, but since it's an SVG, it's specified as color. | ||
|
|
||
| width: vars.size.dimension[100], | ||
| height: vars.size.dimension[200], | ||
|
|
||
| transform: 'rotate(180deg)', | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As the icons for all components are positioned relative to the bottom arrow, this transform has been removed. |
||
| width: vars.size.dimension[200], | ||
| height: vars.size.dimension[100], | ||
| zIndex: 1, | ||
|
|
||
| selectors: { | ||
| '&[data-side="top"]': { | ||
| bottom: '-11px', | ||
| transform: 'rotate(-90deg)', | ||
| }, | ||
| '&[data-side="right"]': { | ||
| left: '-7px', | ||
| transform: 'rotate(0deg)', | ||
| }, | ||
| '&[data-side="bottom"]': { | ||
| top: '-11px', | ||
| transform: 'rotate(90deg)', | ||
| }, | ||
| '&[data-side="left"]': { | ||
| right: '-7px', | ||
| transform: 'rotate(180deg)', | ||
| }, | ||
| }, | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other popup elements with padding were in the
popupcontainer, but since thenavigation-menuwas in thecontentcontainer, I moved it to thepopupcontainer to maintain consistency.