Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes cumulative layout shift (CLS) issues by moving popover attribute setup from client-side useEffect to SSR-compatible direct attribute/style application. The core change generates a stable popupId in the root component and uses it consistently across trigger and popup elements through context, eliminating the need for post-render DOM manipulation.
Key Changes:
- Generated
popupIdonce in root components usinguseId()and shared via context - Moved
popover="manual",commandfor, and anchor styles fromuseEffectto direct JSX attributes - Added
styleprop support to popup components for customization
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/react/src/components/Popover.tsx | Refactored to generate popupId in root, apply attributes directly in JSX, and support custom popup styles |
| packages/react/src/components/Tooltip.tsx | Applied same refactoring pattern as Popover for consistent SSR-compatible attribute handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| commandfor: popupId ?? undefined, | ||
| style: { | ||
| ...existingStyle, | ||
| ...(popupId ? { anchorName: `--${popupId}` as any } : {}), |
There was a problem hiding this comment.
The CSS property should be anchor-name (kebab-case) instead of anchorName (camelCase). React's style prop uses camelCase for JavaScript, but CSS Anchor Positioning uses anchor-name as the property name. This should be '--anchor-name': --${popupId}`` or use DOM manipulation.
| commandfor: popupId ?? undefined, | ||
| style: { | ||
| ...existingStyle, | ||
| ...(popupId ? { anchorName: `--${popupId}` as any } : {}), |
There was a problem hiding this comment.
The CSS property should be anchor-name (kebab-case) instead of anchorName (camelCase). React's style prop uses camelCase for JavaScript, but CSS Anchor Positioning uses anchor-name as the property name. This should be '--anchor-name': --${popupId}`` or use DOM manipulation.
| ...(popupId ? { anchorName: `--${popupId}` as any } : {}), | |
| ...(popupId ? { 'anchor-name': `--${popupId}` } : {}), |
| role="tooltip" | ||
| popover="manual" | ||
| style={{ | ||
| ...(popupId ? { positionAnchor: `--${popupId}` as any } : {}), |
There was a problem hiding this comment.
The CSS property should be position-anchor (kebab-case) instead of positionAnchor (camelCase). React's style prop uses camelCase for JavaScript, but CSS Anchor Positioning uses position-anchor as the property name. This should be 'position-anchor': --${popupId}`` or use DOM manipulation.
| className={className} | ||
| popover="manual" | ||
| style={{ | ||
| ...(popupId ? { positionAnchor: `--${popupId}` as any } : {}), |
There was a problem hiding this comment.
The CSS property should be position-anchor (kebab-case) instead of positionAnchor (camelCase). React's style prop uses camelCase for JavaScript, but CSS Anchor Positioning uses position-anchor as the property name. This should be 'position-anchor': --${popupId}`` or use DOM manipulation.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This pull request refactors and improves the handling of popup positioning and anchoring in both the
PopoverandTooltipcomponents. The changes focus on consistently generating and using a uniquepopupIdfor each popup, simplifying how popup and trigger elements are linked, and allowing custom styles to be passed to popups. This results in more reliable positioning, easier customization, and cleaner code.Popover & Tooltip popup/trigger linking and positioning:
popupId(usinguseIdand a custom format) in bothPopoverandTooltipcontexts, and propagated it through context and props for reliable identification and anchoring. [1] [2]popupIdinstead of generating or passing it separately, and removed redundantuseEffectlogic for setting attributes/styles. [1] [2] [3] [4]commandforandanchorNamestyles based on thepopupId, improving the linking between trigger and popup for positioning. [1] [2]popover="manual"and use thepositionAnchorstyle property with the generatedpopupId, ensuring correct positioning and anchoring. [1] [2]Customization and extensibility:
styleprops on bothPopoverPopupandTooltipPopup, allowing consumers to pass additional styles to popups. [1] [2]