Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
d88fb18 to
fb43e41
Compare
4c2aaa7 to
39ac55d
Compare
4c256ce to
04d7132
Compare
ccf6596 to
a1c6efa
Compare
04d7132 to
937fbf7
Compare
a1c6efa to
5cedc29
Compare
937fbf7 to
53aa689
Compare
5cedc29 to
df968a9
Compare
53aa689 to
b7b29be
Compare
df968a9 to
b2f87dd
Compare
b7b29be to
cfa2969
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new React Popover compound component API (Popover.Root / Popover.Trigger / Popover.Popup / Popover.Arrow) built on top of the existing @videojs/core popover core + DOM handle, providing positioning via CSS Anchor Positioning with a JS measurement fallback.
Changes:
- Introduces
PopoverRootto wirePopoverCore+createPopover()interaction state into a React context. - Adds compound parts (
Trigger,Popup,Arrow) that consume context, apply ARIA/data attrs, and integrate positioning/interaction props. - Exposes the new
Popovernamespace from the React package entrypoint.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/ui/popover/popover-context.tsx | Adds context + hook for sharing core/popover/state/ids across parts. |
| packages/react/src/ui/popover/popover-root.tsx | Implements root state provider bridging PopoverCore and createPopover() into React. |
| packages/react/src/ui/popover/popover-trigger.tsx | Implements trigger part, applying trigger ARIA and anchor-name styling. |
| packages/react/src/ui/popover/popover-popup.tsx | Implements popup part with CSS-anchor positioning and JS fallback with scroll/resize tracking. |
| packages/react/src/ui/popover/popover-arrow.tsx | Implements decorative arrow part with state data attrs + aria-hidden. |
| packages/react/src/ui/popover/index.parts.ts | Defines the public compound-part exports (Root/Trigger/Popup/Arrow). |
| packages/react/src/ui/popover/index.ts | Exports the Popover namespace for consumer imports. |
| packages/react/src/index.ts | Re-exports Popover from the package root API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b2f87dd to
b32ce25
Compare
d12949b to
7b9790e
Compare
b32ce25 to
4bdf4e4
Compare
7b9790e to
35b0dea
Compare
4bdf4e4 to
4ac9f96
Compare
35b0dea to
7d1a6d5
Compare
09ea72a to
a0a2419
Compare
7d1a6d5 to
4674e32
Compare
a0a2419 to
3f1f6db
Compare
dad7c4d to
4615176
Compare
f98efea to
80df696
Compare
4615176 to
0b15451
Compare
- Defer initial open from connectedCallback to firstUpdated for reliable property resolution after upgrade - Skip getBoundingClientRect/resolveOffsets when CSS Anchor Positioning is supported (avoids unnecessary layout measurements) - Use applyElementProps for trigger cleanup instead of manual removeAttribute calls - Remove stale anchor-name inline style on trigger cleanup
Aligns with the TransitionState.open → active rename in core.
80df696 to
a171811
Compare
Add React compound component wrappers for the popover UI: - PopoverRoot: manages state, controlled/uncontrolled open, useId sanitization - PopoverTrigger: anchor name style, focus event remapping (onFocusIn→onFocus) - PopoverPopup: CSS Anchor Positioning with JS fallback, scroll/resize tracking - PopoverArrow: decorative arrow with data-attribute state binding - Barrel exports via Popover namespace (Popover.Root, Popover.Trigger, etc.) Positioning functions now return camelCase keys directly compatible with React's style prop, eliminating the need for toCamelCase/toReactStyle conversion helpers.
- Re-add useLatestRef (moved from core PR scope) - Extract useSafeId hook for CSS-safe identifiers from useId() - Use forwardRef generic params instead of ForwardedRef in function sig - Rename interaction.open to interaction.active per core rename
0b15451 to
5129fd2
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #220
Depends on #652
Summary
React compound components for the popover, following the
Popover.Root/Popover.Trigger/Popover.Popuppattern.API Surface
Usage
Components
Popover.RootPopover.Trigger<button>Popover.Popup<div>Popover.Arrow<div>Root Props
side'top' | 'bottom' | 'left' | 'right''bottom'align'start' | 'center' | 'end''center'modalboolean | 'trap-focus'falseopenbooleandefaultOpenbooleanfalseonOpenChange(open, details) => void{ reason }on state changeopenOnHoverbooleanfalsedelaynumber300closeDelaynumber300closeOnEscapebooleantruecloseOnOutsideClickbooleantruePart Props
Trigger,Popup, andArrowaccept:render— render prop for custom elements (render={<a />})className— static or state-aware (className={(state) => ...})style— static or state-awareref— forwarded refContext
usePopoverContext()returns{ state, popover, core, anchorName, popupId }for advanced use cases.Implementation details
onFocusIn/onFocusOutfrom core → React'sonFocus/onBluruseId()output sanitized for valid CSS<dashed-ident>(strips colons)CSSProperties, no conversion neededTesting
pnpm -F @videojs/react test