Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
32f12f3 to
eaa462c
Compare
4c2aaa7 to
39ac55d
Compare
6b854fe to
b2c8ae1
Compare
df968a9 to
b2f87dd
Compare
There was a problem hiding this comment.
Pull request overview
Adds an HTML-layer media-popover custom element that wraps the core popover primitive and discovers its trigger via commandfor="<popover-id>", wiring up ARIA, event handlers, and positioning styles.
Changes:
- Introduces
PopoverElement(media-popover) with reactive properties for open state, interactions, and placement. - Adds definition module to register the custom element.
- Exports the element from the package entrypoint.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/html/src/ui/popover/popover-element.ts | Implements the media-popover element: state sync, trigger discovery, ARIA/data attrs, and positioning. |
| packages/html/src/index.ts | Exposes PopoverElement from the HTML package public API. |
| packages/html/src/define/ui/popover.ts | Registers the custom element via customElements.define. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f98efea to
80df696
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
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📦 Bundle Size Report
Total: 49.31 kB · +889 B · +1.8% Entry BreakdownSubpath sizes are the additional bytes on top of the root entry point, measured by bundling root + subpath together and subtracting the root-only size.
|
| Entry | Base | PR | Diff | % | |
|---|---|---|---|---|---|
. |
4.19 kB | 4.19 kB | 0 B | 0% | ✅ |
./dom |
5.91 kB | 5.91 kB | 0 B | 0% | ✅ |
| total | 10.11 kB | 10.11 kB | 0 B | 0% |
@videojs/element
| Entry | Base | PR | Diff | % | |
|---|---|---|---|---|---|
. |
817 B | 817 B | 0 B | 0% | ✅ |
./context |
823 B | 823 B | 0 B | 0% | ✅ |
| total | 1.60 kB | 1.60 kB | 0 B | 0% |
@videojs/html
| Entry | Base | PR | Diff | % | |
|---|---|---|---|---|---|
. |
13.07 kB | 13.97 kB | +922 B | +6.9% | 🔺 |
./video |
1.07 kB | 1.06 kB | -9 B | -0.8% | 🔽 |
./audio |
1.05 kB | 1.05 kB | -9 B | -0.8% | 🔽 |
./background |
1.06 kB | 1.04 kB | -15 B | -1.4% | 🔽 |
| total | 16.25 kB | 17.12 kB | +889 B | +5.3% |
@videojs/icons
| Entry | Base | PR | Diff | % | |
|---|---|---|---|---|---|
./react |
2.27 kB | 2.27 kB | 0 B | 0% | ✅ |
./html |
1.52 kB | 1.52 kB | 0 B | 0% | ✅ |
| total | 3.79 kB | 3.79 kB | 0 B | 0% |
@videojs/store
| Entry | Base | PR | Diff | % | |
|---|---|---|---|---|---|
. |
1.29 kB | 1.29 kB | 0 B | 0% | ✅ |
./html |
468 B | 468 B | 0 B | 0% | ✅ |
./react |
199 B | 199 B | 0 B | 0% | ✅ |
| total | 1.94 kB | 1.94 kB | 0 B | 0% |
@videojs/utils
| Entry | Base | PR | Diff | % | |
|---|---|---|---|---|---|
./array |
104 B | 104 B | 0 B | 0% | ✅ |
./dom |
928 B | 928 B | 0 B | 0% | ✅ |
./events |
227 B | 227 B | 0 B | 0% | ✅ |
./function |
197 B | 197 B | 0 B | 0% | ✅ |
./object |
119 B | 119 B | 0 B | 0% | ✅ |
./predicate |
265 B | 265 B | 0 B | 0% | ✅ |
./string |
148 B | 148 B | 0 B | 0% | ✅ |
./style |
185 B | 185 B | 0 B | 0% | ✅ |
./time |
478 B | 478 B | 0 B | 0% | ✅ |
./number |
158 B | 158 B | 0 B | 0% | ✅ |
| total | 2.74 kB | 2.74 kB | 0 B | 0% |
ℹ️ How to interpret
Sizes are minified + brotli, measured with esbuild.
Package totals are computed as root size + marginal subpath costs.
Subpath marginal cost = (root + subpath bundled together) − root alone.
| Icon | Meaning |
|---|---|
| ✅ | No change |
| 🔺 | Increased ≤ 10% |
| 🔴 | Increased > 10% |
| 🔽 | Decreased |
| 🆕 | New (no baseline) |
Run pnpm size locally to check current sizes.
Refs #220
Depends on #615
Summary
HTML custom element for the popover, using the
commandforattribute pattern for trigger discovery.API Surface
Element
Attributes
sidestring'bottom'alignstring'center'modalbooleanfalseopenbooleanfalsedefault-openbooleanfalseopen-on-hoverbooleanfalsedelaynumber300close-delaynumber300close-on-escapebooleantrueclose-on-outside-clickbooleantrueEvents
open-change{ open, reason }Trigger Discovery
The trigger is discovered via
commandforlinkage — any element withcommandfor="<popover-id>"becomes the trigger. ARIA attributes (aria-expanded,aria-haspopup,aria-controls) and anchor positioning styles are applied to the trigger automatically.Implementation details
SnapshotControllerviatrack()on reconnect to avoid leaking stale controllersmodalattribute usesBooleantype (notString)Testing
pnpm -F @videojs/html test