refactor(packages)!: replace button availability with disabled and hidden state#1474
Open
mihar-22 wants to merge 12 commits into
Open
refactor(packages)!: replace button availability with disabled and hidden state#1474mihar-22 wants to merge 12 commits into
mihar-22 wants to merge 12 commits into
Conversation
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📦 Bundle Size Report🎨 @videojs/html — no changesPresets (7)
Media (10)
Players (5)
Skins (30)
UI Components (39)
Sizes are marginal over the root entry point. ⚛️ @videojs/react — no changesPresets (7)
Media (9)
Skins (27)
UI Components (33)
Sizes are marginal over the root entry point. 🧩 @videojs/core — no changesEntries (14)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (10)
📦 @videojs/spf — no changesEntries (4)
ℹ️ How to interpretJS sizes are initial static graph totals (minified + brotli). Lazy dynamic chunks are shown separately when present.
Run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Covers the rationale for using aria-disabled over HTML disabled, HTML hidden for unsupported features, and separate data-disabled/data-hidden styling hooks across cast, fullscreen, and pip buttons. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dden state Cast, fullscreen, and pip buttons now expose `disabled` (non-interactive) and `hidden` (unsupported) state derived from `availability` and the `disabled` prop, instead of relying on the raw availability enum at the attribute layer. - `getAttrs` returns `aria-disabled` from state and the native HTML `hidden` attribute when the feature is unsupported. New `data-disabled` and `data-hidden` data attribute mappings ride along. - `toggle` short-circuits on `state.disabled` and otherwise awaits the underlying media call directly, propagating errors to the caller instead of swallowing them. - `MediaButtonElement` and `createMediaButton` now wrap the activation in try/catch with a `__DEV__` console.error and rethrow so callers see the original failure. - React buttons pass `isSupported: (s) => !s.hidden` so unsupported features render `null` rather than a hidden `<button>`. Aligns with the WAI-ARIA APG toolbar pattern (focusable disabled controls) documented in `internal/design/ui/disabled-hidden.md`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace `data-[availability=...]:hidden` with `data-[disabled]` styling classes to match the new disabled/hidden button state model. Hidden buttons use the native HTML hidden attribute; disabled buttons get reduced opacity and grayscale via data-disabled. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PiP is unsupported on WebKit so the button receives the `hidden` attribute and is removed from the layout. Only assert `data-availability` when the pip button is visible. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tons Update the fullscreen and pip button reference pages plus the features concept page to describe the new `disabled`/`hidden` state model: HTML `hidden` for unsupported environments (or `null` in React) and `data-disabled` for non-interactive styling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Document the two pre-existing fields alongside the newly added disabled/hidden so the component reference table renders complete descriptions for every cast button state property. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The skin's `display: flex` (and `grid` on the icon variant) outranks the
user-agent `[hidden] { display: none }` rule on specificity, so feature
buttons stayed visible when the cast/fullscreen/pip cores set the native
`hidden` attribute.
Add a `&[hidden] { display: none }` rule under the high-specificity
skin selector in both default and minimal CSS, and a `[&[hidden]]:hidden`
class in the Tailwind variants so the same override works for the
Tailwind-compiled skins.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…omponent override Move the native `hidden` attribute override from the button component into the skin reset so any authored template that sets `[hidden]` stays hidden, not just media buttons. Use the doubled `[hidden][hidden]` selector under the skin root to outrank component-level `display: flex/grid` declarations. The Tailwind root composition gets the equivalent `[&_[hidden][hidden]]:hidden` class. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The captions button previously relied on the now-removed `data-availability="unavailable"` skin rule to hide itself when no caption tracks were present. Extend the same `disabled`/`hidden` model already applied to cast/fullscreen/pip buttons so it stays hidden in that state without depending on availability-specific CSS. - Add `disabled` and `hidden` to `CaptionsButtonState`, derived from the `disabled` prop and whether any caption/subtitle tracks exist. - `getAttrs` returns `aria-disabled` from state and the native HTML `hidden` attribute when no tracks are available. - `toggle` short-circuits on `state.disabled`. - `data-disabled` and `data-hidden` data attribute mappings ride along. - React captions button passes `isSupported: (s) => !s.hidden` so it renders `null` when no tracks are present. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously unavailable buttons used `display: none`, which masked the
`:active { scale: 0.98 }` rule. Now that `[data-disabled]` keeps the
button visible but non-interactive, guard the press animation with
`:not([disabled]):not([data-disabled])` (and the Tailwind equivalent
`not-disabled:not-data-disabled:active:*`) so disabled buttons no
longer give misleading press feedback.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mihar-22
force-pushed
the
refactor/button-disabled-hidden
branch
from
8bcff00 to
d5c1de0
Compare
| Promise.resolve(action(core, feature!)).catch((error) => { | ||
| if (__DEV__) console.error(`[${displayName}]`, error); | ||
| }); | ||
| }, |
There was a problem hiding this comment.
Core disabled not wired to clicks
Medium Severity
createMediaButton and MediaButtonElement pass createButton an isDisabled check that only considers the disabled prop and media presence, not *Core’s derived state.disabled. After this change, controls like cast can stay visible with aria-disabled and data-disabled while pointer and keyboard activation still run (toggle no-ops). Previously data-availability="unavailable" hid them in CSS.
Additional Locations (1)
Triggered by learned rule: New UI feature logic belongs in *Core classes — React and HTML layers delegate
Reviewed by Cursor Bugbot for commit d5c1de0. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7debc81. Configure here.
| protected abstract readonly mediaState: PlayerController<any, InferMediaState<Core> | undefined>; | ||
|
|
||
| protected abstract activate(state: InferMediaState<Core>, event?: UIEvent): void; | ||
| protected abstract activate(state: InferMediaState<Core>, event?: UIEvent): void | Promise<void>; |
There was a problem hiding this comment.
Interaction ignores core disabled state
Medium Severity
createButton / useButton still treat only the disabled prop (and missing media) as non-interactive, while cores now set aria-disabled and data-disabled from getState().disabled (including cast unavailable and explicit prop). Pointer and keyboard activation can still run even when the control is marked disabled.
Additional Locations (1)
Triggered by learned rule: New UI feature logic belongs in *Core classes — React and HTML layers delegate
Reviewed by Cursor Bugbot for commit 7debc81. Configure here.
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
2 participants
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.
Replace the
availableboolean on buttons withdisabledandhiddenstates that follow standard ARIA patterns —aria-disabledfor non-interactive controls and HTMLhiddenfor unsupported features.Note
Medium Risk
Breaking change for consumers who hid buttons via
data-availabilityCSS; toggle errors now propagate from core (UI still swallows at click). Touches player toolbar a11y and cross-browser behavior (PiP/cast).Overview
Toolbar feature buttons (captions, cast, fullscreen, PiP) now expose derived
disabledandhiddenstate alongsidedata-availability, instead of relying on skins to hide controls withdata-availability=unavailable|unsupported.disabledis set when thedisabledprop is true or availability is notavailable(including cast with no device, or captions with no tracks).hiddenhides unsupported APIs (fullscreen/PiP/cast) or captions when there are no tracks—HTML custom elements get nativehiddenplusdata-hidden/data-disabled; React buttons useisSupportedand rendernull.Accessibility and attrs:
getAttrsdrivesaria-disabledfromstate.disabled(not the prop alone) and addsdata-disabled/data-hiddenvia state attr maps. Default skins style[aria-disabled="true"]and no longerdisplay: noneon availability.toggle()re-checks state aftersetMedia, returns underlying media promises, and re-throws errors (cast/fullscreen/PiP); HTMLMediaButtonElementandcreateMediaButtoncatch async failures at the click boundary (dev log only).Docs and an internal disabled/hidden ADR document the pattern; e2e only asserts PiP
data-availabilitywhen the button is visible (e.g. WebKit).Reviewed by Cursor Bugbot for commit 7debc81. Bugbot is set up for automated code reviews on this repo. Configure here.