mirror of
https://github.com/zoriya/v10.git
synced 2026-08-16 02:45:09 +00:00
docs(claude): add Video.js component architecture patterns (#450)
This commit is contained in:
rahim
@@ -182,6 +182,7 @@ See [props.md](references/props.md) for naming conventions.
|
||||
| [polymorphism.md](references/polymorphism.md) | render vs asChild patterns |
|
||||
| [collection.md](references/collection.md) | Collections, portals, virtualization |
|
||||
| [anti-patterns.md](references/anti-patterns.md) | Common component mistakes |
|
||||
| [videojs.md](references/videojs.md) | Video.js component architecture |
|
||||
|
||||
For accessibility patterns (ARIA, keyboard, focus), load the `aria` skill.
|
||||
|
||||
|
||||
@@ -0,0 +1,246 @@
|
||||
# Video.js Component Architecture
|
||||
|
||||
Video.js components use a three-layer architecture separating framework-agnostic logic from platform implementations.
|
||||
|
||||
## Architecture Overview
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────┐
|
||||
│ @videojs/core Framework-agnostic business logic │
|
||||
│ @videojs/core/dom Shared DOM utilities │
|
||||
└─────────────────────────────────────────────────────────────┘
|
||||
│
|
||||
┌─────────────────┴─────────────────┐
|
||||
▼ ▼
|
||||
@videojs/html @videojs/react
|
||||
Web Components (Lit) React Components
|
||||
```
|
||||
|
||||
| Package | Responsibility |
|
||||
|---------|----------------|
|
||||
| `@videojs/core` | Core classes with `getState()`/`getAttrs()`/actions |
|
||||
| `@videojs/core/dom` | DOM utilities, selectors, button behavior |
|
||||
| `@videojs/html` | Web Components consuming core via controllers |
|
||||
| `@videojs/react` | React components consuming core via hooks |
|
||||
|
||||
---
|
||||
|
||||
## Core Class Pattern
|
||||
|
||||
Every UI component has a `*Core` class in `@videojs/core`.
|
||||
|
||||
### Props Interface
|
||||
|
||||
```ts
|
||||
interface PlayButtonProps {
|
||||
/** Custom label for the button. */
|
||||
label?: string | undefined;
|
||||
/** Whether the button is disabled. */
|
||||
disabled?: boolean | undefined;
|
||||
}
|
||||
```
|
||||
|
||||
- All props optional with `| undefined` (explicit optionality)
|
||||
- JSDoc for each prop
|
||||
|
||||
### State Interface
|
||||
|
||||
```ts
|
||||
interface PlayButtonState extends Pick<PlaybackState, 'paused' | 'ended' | 'started'> {}
|
||||
```
|
||||
|
||||
- Primitives only — no methods
|
||||
- Use `Pick<FeatureState, ...>` to select relevant fields
|
||||
|
||||
### Core Class
|
||||
|
||||
```ts
|
||||
class PlayButtonCore {
|
||||
static readonly defaultProps: NonNullableObject<Props>;
|
||||
|
||||
setProps(props: Props): void; // Merge with defaults
|
||||
getLabel(state: FeatureState): string; // Computed label
|
||||
getAttrs(state: FeatureState): ElementProps; // ARIA only
|
||||
getState(state: FeatureState): State; // Primitives only
|
||||
toggle(state: FeatureState): Promise<void>; // Action
|
||||
}
|
||||
|
||||
namespace PlayButtonCore {
|
||||
export type Props = PlayButtonProps;
|
||||
export type State = PlayButtonState;
|
||||
}
|
||||
```
|
||||
|
||||
**Rules:**
|
||||
|
||||
- `static readonly defaultProps` with `NonNullableObject<Props>` type
|
||||
- `getAttrs()` returns ARIA attributes only (no `data-*`)
|
||||
- `getState()` returns primitives only (no methods) — converted to `data-*` for CSS
|
||||
- Action methods receive feature state from store
|
||||
- Namespace exports `Props` and `State` types
|
||||
|
||||
---
|
||||
|
||||
## State vs Attrs Separation
|
||||
|
||||
| Method | Returns | Purpose |
|
||||
|--------|---------|---------|
|
||||
| `getAttrs()` | ARIA attributes | Accessibility (`aria-label`, `aria-disabled`) |
|
||||
| `getState()` | Primitives | CSS styling via `data-*` attributes |
|
||||
|
||||
**Why separate:**
|
||||
|
||||
- CSS targets `[data-paused]`, `[data-ended]` selectors
|
||||
- ARIA attrs remain semantically accurate
|
||||
- Can update independently
|
||||
|
||||
---
|
||||
|
||||
## Data Attribute Enums
|
||||
|
||||
Enums provide single source of truth + API reference tooling:
|
||||
|
||||
```ts
|
||||
export enum PlayButtonDataAttrs {
|
||||
/** Present when the media is paused. */
|
||||
paused = 'data-paused',
|
||||
/** Present when the media has ended. */
|
||||
ended = 'data-ended',
|
||||
}
|
||||
```
|
||||
|
||||
JSDoc comments generate API documentation.
|
||||
|
||||
---
|
||||
|
||||
## ElementProps
|
||||
|
||||
Shared interface in `core/element.ts`. Extend as components need new attributes:
|
||||
|
||||
- Add `aria-*` attributes used by any component
|
||||
- Use string literal types where ARIA spec defines allowed values
|
||||
- `undefined` removes the attribute
|
||||
|
||||
---
|
||||
|
||||
## Web Component (Lit)
|
||||
|
||||
```ts
|
||||
class PlayButtonElement extends MediaElement {
|
||||
static readonly tagName = 'media-play-button';
|
||||
static override properties = { label: { type: String }, disabled: { type: Boolean } };
|
||||
|
||||
readonly #core = new PlayButtonCore();
|
||||
readonly #state = new PlayerController(this, playerContext, selectPlayback);
|
||||
#disconnect: AbortController | null = null;
|
||||
|
||||
// Lifecycle: see flow below
|
||||
}
|
||||
```
|
||||
|
||||
**Lifecycle flow:**
|
||||
|
||||
1. `connectedCallback` — Create `AbortController`, apply button props via `applyElementProps()`
|
||||
2. `disconnectedCallback` — Abort controller for cleanup
|
||||
3. `willUpdate` — Sync component props to core via `setProps()`
|
||||
4. `update` — Apply `getAttrs()` and `getState()` to element
|
||||
|
||||
**Key utilities:**
|
||||
|
||||
- `PlayerController(host, context, selector)` — Store subscription
|
||||
- `applyElementProps(el, props, signal)` — Apply attrs + events
|
||||
- `applyStateDataAttrs(el, state)` — State → `data-*`
|
||||
- `logMissingFeature(name, feature)` — Deduped warning
|
||||
|
||||
---
|
||||
|
||||
## React Component
|
||||
|
||||
```tsx
|
||||
const PlayButton = forwardRef(function PlayButton(props, ref) {
|
||||
const playback = usePlayer(selectPlayback);
|
||||
const [core] = useState(() => new PlayButtonCore());
|
||||
const { getButtonProps, buttonRef } = useButton({ onActivate, isDisabled });
|
||||
|
||||
return renderElement('button', { render, className, style }, {
|
||||
state: core.getState(playback),
|
||||
ref: [ref, buttonRef],
|
||||
props: [core.getAttrs(playback), elementProps, getButtonProps()],
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
**Flow:**
|
||||
|
||||
1. `usePlayer(selector)` — Subscribe to store slice
|
||||
2. `useState(() => new Core())` — Lazy init core class
|
||||
3. `useButton()` — Get accessible button behavior
|
||||
4. `renderElement()` — Render with state→data-attrs, ref composition, props merge
|
||||
|
||||
**Props type:** `UIComponentProps<Tag, State>` allows `className`/`style` as functions of state.
|
||||
|
||||
---
|
||||
|
||||
## Shared Utilities
|
||||
|
||||
### @videojs/core/dom
|
||||
|
||||
| Utility | Purpose |
|
||||
|---------|---------|
|
||||
| `createButton(options)` | Accessible button (Enter/Space, click, disabled) |
|
||||
| `applyElementProps(el, props, signal?)` | Apply attrs and events to DOM |
|
||||
| `applyStateDataAttrs(el, state)` | State object → `data-*` attributes |
|
||||
| `getStateDataAttrs(state)` | State → data-attrs object (React) |
|
||||
| `logMissingFeature(name, feature)` | Deduped console.warn |
|
||||
| `selectPlayback` / `selectVolume` | Store selectors |
|
||||
|
||||
### @videojs/react/utils
|
||||
|
||||
| Utility | Purpose |
|
||||
|---------|---------|
|
||||
| `renderElement(tag, props, params)` | Render with state, refs, props merge |
|
||||
| `mergeProps(...propSets)` | Chain events, concat className, merge style |
|
||||
| `composeRefs(...refs)` | Compose refs (React 19 cleanup support) |
|
||||
|
||||
---
|
||||
|
||||
## File Organization
|
||||
|
||||
```
|
||||
packages/
|
||||
├── core/src/
|
||||
│ ├── core/
|
||||
│ │ ├── element.ts # ElementProps interface
|
||||
│ │ └── ui/{component}/
|
||||
│ │ ├── {component}-core.ts # Core class
|
||||
│ │ ├── {component}-core.test.ts # Core tests
|
||||
│ │ └── {component}-data-attrs.ts # Data attr enum
|
||||
│ └── dom/ui/ # createButton, utils
|
||||
├── html/src/
|
||||
│ ├── ui/{component}/ # Web Component
|
||||
│ ├── define/ui/ # Side-effect registration
|
||||
│ └── player/player-controller.ts # Store controller
|
||||
└── react/src/
|
||||
├── ui/{component}/ # React component
|
||||
├── ui/hooks/ # Behavior hooks
|
||||
└── utils/ # renderElement, mergeProps
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Component Registration
|
||||
|
||||
```ts
|
||||
// define/ui/play-button.ts
|
||||
customElements.define(PlayButtonElement.tagName, PlayButtonElement);
|
||||
|
||||
declare global {
|
||||
interface HTMLElementTagNameMap {
|
||||
[PlayButtonElement.tagName]: PlayButtonElement;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- Tag name: `static readonly tagName = 'media-{name}'`
|
||||
- Registration in `define/ui/` directory
|
||||
- Augment `HTMLElementTagNameMap` for TypeScript
|
||||
@@ -1,334 +1,30 @@
|
||||
# Component Review Checklist
|
||||
|
||||
Comprehensive checklist for reviewing UI components against architecture patterns and conventions.
|
||||
Checklists for reviewing UI components against architecture patterns and conventions.
|
||||
|
||||
---
|
||||
## Checklists
|
||||
|
||||
## Architecture
|
||||
| Checklist | Use For |
|
||||
|-----------|---------|
|
||||
| [general.md](checklists/general.md) | Standard headless component patterns (architecture, state, props, styling) |
|
||||
| [videojs.md](checklists/videojs.md) | Video.js-specific patterns (core class, platform adapters) |
|
||||
| [severity.md](checklists/severity.md) | Anti-patterns and severity classification |
|
||||
|
||||
### Compound Components
|
||||
## Quick Selection
|
||||
|
||||
- [ ] Component uses compound structure (Root, Trigger, Content, etc.)
|
||||
- [ ] Each part maps 1:1 to a DOM element
|
||||
- [ ] Parts can be reordered or omitted freely
|
||||
- [ ] No prop explosion (>10 props suggests need for decomposition)
|
||||
**Building a new Video.js component?**
|
||||
→ Use [general.md](checklists/general.md) + [videojs.md](checklists/videojs.md)
|
||||
|
||||
**Detection:** Single component with many configuration props
|
||||
**Reviewing component architecture?**
|
||||
→ Use [general.md](checklists/general.md)
|
||||
|
||||
```tsx
|
||||
// BAD: Prop explosion
|
||||
<Dialog title="..." description="..." closeButton overlayClassName="..." />
|
||||
|
||||
// GOOD: Compound
|
||||
<Dialog.Root>
|
||||
<Dialog.Overlay />
|
||||
<Dialog.Content>
|
||||
<Dialog.Title>...</Dialog.Title>
|
||||
<Dialog.Close />
|
||||
</Dialog.Content>
|
||||
</Dialog.Root>
|
||||
```
|
||||
|
||||
### Standard Hierarchies
|
||||
|
||||
| Type | Expected Parts |
|
||||
| ----------- | ---------------------------------------------------- |
|
||||
| Popups | Root → Trigger → Portal → Positioner → Popup → Arrow |
|
||||
| Collections | Root → List → Trigger + Panel |
|
||||
| Forms | Root → Label → Control → Description → Error |
|
||||
|
||||
### Context Scoping
|
||||
|
||||
- [ ] Each Root creates isolated context
|
||||
- [ ] Nested instances don't interfere
|
||||
|
||||
**Detection:** Nested components share unintended state
|
||||
|
||||
---
|
||||
|
||||
## State Management
|
||||
|
||||
### Controlled & Uncontrolled Support
|
||||
|
||||
- [ ] Both modes supported: `value` (controlled) and `defaultValue` (uncontrolled)
|
||||
- [ ] Works correctly in either mode
|
||||
- [ ] No state desync between modes
|
||||
|
||||
**Detection:** Only `defaultValue` or only `value` supported
|
||||
|
||||
| State | Uncontrolled | Controlled | Handler |
|
||||
| ------- | ---------------- | ---------- | ----------------- |
|
||||
| Open | `defaultOpen` | `open` | `onOpenChange` |
|
||||
| Value | `defaultValue` | `value` | `onValueChange` |
|
||||
| Checked | `defaultChecked` | `checked` | `onCheckedChange` |
|
||||
|
||||
### Change Event Details
|
||||
|
||||
- [ ] Handler receives value and details object
|
||||
- [ ] Details includes `reason` (click, keyboard, blur, escape, etc.)
|
||||
- [ ] Details includes `event` (original DOM event)
|
||||
- [ ] Details includes `cancel()` for preventing change
|
||||
|
||||
**Detection:** Handler receives only value, no context
|
||||
|
||||
```typescript
|
||||
// BAD
|
||||
onOpenChange?: (open: boolean) => void;
|
||||
|
||||
// GOOD
|
||||
onOpenChange?: (open: boolean, details: ChangeDetails) => void;
|
||||
```
|
||||
|
||||
### Imperative Actions
|
||||
|
||||
- [ ] `actionsRef` prop exposes imperative methods where needed
|
||||
- [ ] Common actions: `open()`, `close()`, `toggle()`, `focus()`
|
||||
|
||||
---
|
||||
|
||||
## Props & API
|
||||
|
||||
### Boolean Props
|
||||
|
||||
- [ ] Use positive adjectives: `disabled`, `required`, `open`
|
||||
- [ ] Avoid `is`/`has` prefixes: not `isDisabled`, `isOpen`
|
||||
|
||||
| Good | Avoid |
|
||||
| ---------- | ------------ |
|
||||
| `disabled` | `isDisabled` |
|
||||
| `open` | `isOpen` |
|
||||
| `loading` | `isLoading` |
|
||||
|
||||
### Event Handler Naming
|
||||
|
||||
- [ ] Pattern: `on` + Noun + Verb
|
||||
- [ ] Specific names over generic: `onValueChange` not `onChange`
|
||||
|
||||
| Good | Avoid |
|
||||
| --------------- | ----------------------- |
|
||||
| `onOpenChange` | `handleOpen`, `setOpen` |
|
||||
| `onValueChange` | `onChange` |
|
||||
| `onSelect` | `onItemSelected` |
|
||||
|
||||
### Standard Props by Category
|
||||
|
||||
**Interaction:**
|
||||
|
||||
- [ ] `disabled?: boolean`
|
||||
- [ ] `required?: boolean`
|
||||
- [ ] `readOnly?: boolean`
|
||||
|
||||
**Collections:**
|
||||
|
||||
- [ ] `multiple?: boolean`
|
||||
- [ ] `loopFocus?: boolean`
|
||||
- [ ] `orientation?: 'horizontal' | 'vertical'`
|
||||
|
||||
**Popups:**
|
||||
|
||||
- [ ] `modal?: boolean`
|
||||
- [ ] `closeOnEscape?: boolean`
|
||||
- [ ] `closeOnOutsideClick?: boolean`
|
||||
- [ ] `keepMounted?: boolean`
|
||||
|
||||
### Refs
|
||||
|
||||
- [ ] Ref forwarded to root DOM element
|
||||
- [ ] Parent can access DOM for focus, measurement
|
||||
|
||||
**Detection:** `forwardRef` not used
|
||||
|
||||
```tsx
|
||||
// BAD
|
||||
const Button = ({ children }) => <button>{children}</button>;
|
||||
|
||||
// GOOD
|
||||
const Button = forwardRef<HTMLButtonElement, Props>(({ children, ...props }, ref) => (
|
||||
<button ref={ref} {...props}>
|
||||
{children}
|
||||
</button>
|
||||
));
|
||||
```
|
||||
|
||||
### Polymorphism
|
||||
|
||||
- [ ] Uses `render` prop or `asChild`, not `as` prop
|
||||
- [ ] `render` function receives props and state
|
||||
- [ ] State accessible for conditional rendering
|
||||
|
||||
**Detection:** Component has `as` prop
|
||||
|
||||
```tsx
|
||||
// BAD: TypeScript performance issues
|
||||
<Button as="a" href="/">
|
||||
|
||||
// GOOD: render prop
|
||||
<Button render={<a href="/" />}>
|
||||
|
||||
// GOOD: asChild
|
||||
<Button asChild>
|
||||
<a href="/">Link</a>
|
||||
</Button>
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Data Attributes & Styling
|
||||
|
||||
### State via Data Attributes
|
||||
|
||||
- [ ] State exposed via `data-*` attributes
|
||||
- [ ] Enables CSS-only styling without JS
|
||||
|
||||
**Detection:** Inline styles for state, no data attributes
|
||||
|
||||
| Attribute | When Present |
|
||||
| ------------------ | --------------------------- |
|
||||
| `data-open` | Component is open |
|
||||
| `data-closed` | Component is closed |
|
||||
| `data-checked` | Toggle is checked |
|
||||
| `data-disabled` | Component is disabled |
|
||||
| `data-highlighted` | Item has focus within group |
|
||||
| `data-side` | Popup position side |
|
||||
| `data-align` | Popup alignment |
|
||||
|
||||
```tsx
|
||||
// BAD
|
||||
<button style={{ opacity: disabled ? 0.5 : 1 }}>
|
||||
|
||||
// GOOD
|
||||
<button data-disabled={disabled || undefined}>
|
||||
```
|
||||
|
||||
### CSS Variables
|
||||
|
||||
- [ ] CSS variables documented for customization
|
||||
- [ ] Standard variables: `--available-height`, `--anchor-width`, `--transform-origin`
|
||||
|
||||
### No Shipped CSS
|
||||
|
||||
- [ ] Component is headless — no CSS imported
|
||||
- [ ] User brings their own styles
|
||||
|
||||
**Detection:** `import 'component/styles.css'` in component
|
||||
|
||||
---
|
||||
|
||||
## Animation Support
|
||||
|
||||
### Exit Animations
|
||||
|
||||
- [ ] Exit animation possible (element not immediately unmounted)
|
||||
- [ ] `data-state="open"` / `data-state="closed"` for CSS transitions
|
||||
- [ ] `keepMounted` option for JS animation libraries
|
||||
|
||||
**Detection:** `{open && <Content />}` pattern without animation support
|
||||
|
||||
```tsx
|
||||
// BAD: No exit animation possible
|
||||
{open && <Dialog.Content>...</Dialog.Content>}
|
||||
|
||||
// GOOD: Data attributes for CSS
|
||||
<Dialog.Content data-state={open ? 'open' : 'closed'}>
|
||||
|
||||
// GOOD: keepMounted for JS animation
|
||||
<Dialog.Portal keepMounted>
|
||||
<AnimatePresence>
|
||||
{open && <Dialog.Content />}
|
||||
</AnimatePresence>
|
||||
</Dialog.Portal>
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## SSR Safety
|
||||
|
||||
- [ ] No `document` or `window` at module scope
|
||||
- [ ] Portals handle SSR (render fallback or wait for mount)
|
||||
- [ ] IDs generated safely (no Math.random at module level)
|
||||
|
||||
**Detection:** `document.body` reference outside effect
|
||||
|
||||
```tsx
|
||||
// BAD
|
||||
function Portal({ children }) {
|
||||
return createPortal(children, document.body);
|
||||
}
|
||||
|
||||
// GOOD
|
||||
function Portal({ children }) {
|
||||
const [mounted, setMounted] = useState(false);
|
||||
useEffect(() => setMounted(true), []);
|
||||
if (!mounted) return <>{children}</>;
|
||||
return createPortal(children, document.body);
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Accessibility
|
||||
|
||||
For full accessibility review, load the `aria` skill and run `review/workflow.md`.
|
||||
|
||||
Quick checks:
|
||||
|
||||
- [ ] All interactive elements have accessible names
|
||||
- [ ] Keyboard navigation works
|
||||
- [ ] Focus managed for modals (trapped, restored)
|
||||
- [ ] ARIA attributes reflect component state
|
||||
|
||||
---
|
||||
|
||||
## Anti-Pattern Summary
|
||||
|
||||
| Anti-Pattern | Detection | Fix |
|
||||
| ------------------- | ------------------------------------- | ----------------------------- |
|
||||
| Prop explosion | >10 props on one component | Use compound components |
|
||||
| Inline state styles | `style={{ opacity: disabled ? ... }}` | Use data attributes |
|
||||
| Shipped CSS | `import 'styles.css'` in component | Headless, user brings styles |
|
||||
| `as` prop | `<Button as="a">` | Use `render` or `asChild` |
|
||||
| Missing controlled | Only `defaultValue` | Add `value` + `onValueChange` |
|
||||
| Context collision | Nested instances share state | Scope contexts per Root |
|
||||
| No exit animation | `{open && ...}` without `keepMounted` | Add animation support |
|
||||
| SSR unsafe | `document.body` at module scope | Guard with mount check |
|
||||
| No ref forwarding | Missing `forwardRef` | Forward ref to DOM |
|
||||
|
||||
---
|
||||
|
||||
## Severity Guide
|
||||
|
||||
### Critical
|
||||
|
||||
| Issue | Why |
|
||||
| ---------------------------- | ----------------------------------- |
|
||||
| Missing controlled support | Can't integrate with external state |
|
||||
| Context collision in nesting | Breaks composition |
|
||||
| SSR crash | Breaks server rendering |
|
||||
| Memory leak (no cleanup) | Production issue |
|
||||
|
||||
### Major
|
||||
|
||||
| Issue | Why |
|
||||
| ------------------------- | -------------------- |
|
||||
| Prop explosion | Poor DX, inflexible |
|
||||
| Missing data attributes | Can't style with CSS |
|
||||
| No exit animation support | Poor UX |
|
||||
| Boolean trap | Confusing API |
|
||||
| Missing ref forwarding | Can't access DOM |
|
||||
|
||||
### Minor
|
||||
|
||||
| Issue | Why |
|
||||
| --------------------- | ------------------- |
|
||||
| Inconsistent naming | API inconsistency |
|
||||
| Missing CSS variables | Harder to customize |
|
||||
| Verbose handler names | Minor DX issue |
|
||||
|
||||
---
|
||||
**Classifying issues found?**
|
||||
→ Use [severity.md](checklists/severity.md)
|
||||
|
||||
## See Also
|
||||
|
||||
- [Anti-Patterns](../references/anti-patterns.md) — Full anti-pattern reference
|
||||
- [Anti-Patterns](../references/anti-patterns.md) — Full anti-pattern reference with examples
|
||||
- [Props](../references/props.md) — Prop naming conventions
|
||||
- [Styling](../references/styling.md) — Data attributes and CSS variables
|
||||
- [Video.js Architecture](../references/videojs.md) — Three-layer component architecture
|
||||
- [Accessibility Checklist](../../aria/review/checklist.md) — Full a11y checklist
|
||||
|
||||
@@ -0,0 +1,188 @@
|
||||
# General Component Checklist
|
||||
|
||||
Standard patterns for headless UI components.
|
||||
|
||||
---
|
||||
|
||||
## Architecture
|
||||
|
||||
### Compound Components
|
||||
|
||||
- [ ] Component uses compound structure (Root, Trigger, Content, etc.)
|
||||
- [ ] Each part maps 1:1 to a DOM element
|
||||
- [ ] Parts can be reordered or omitted freely
|
||||
- [ ] No prop explosion (>10 props suggests need for decomposition)
|
||||
|
||||
**Detection:** Single component with many configuration props
|
||||
|
||||
```tsx
|
||||
// BAD: Prop explosion
|
||||
<Dialog title="..." description="..." closeButton overlayClassName="..." />
|
||||
|
||||
// GOOD: Compound
|
||||
<Dialog.Root>
|
||||
<Dialog.Overlay />
|
||||
<Dialog.Content>
|
||||
<Dialog.Title>...</Dialog.Title>
|
||||
<Dialog.Close />
|
||||
</Dialog.Content>
|
||||
</Dialog.Root>
|
||||
```
|
||||
|
||||
### Standard Hierarchies
|
||||
|
||||
| Type | Expected Parts |
|
||||
| ----------- | ---------------------------------------------------- |
|
||||
| Popups | Root → Trigger → Portal → Positioner → Popup → Arrow |
|
||||
| Collections | Root → List → Trigger + Panel |
|
||||
| Forms | Root → Label → Control → Description → Error |
|
||||
|
||||
### Context Scoping
|
||||
|
||||
- [ ] Each Root creates isolated context
|
||||
- [ ] Nested instances don't interfere
|
||||
|
||||
**Detection:** Nested components share unintended state
|
||||
|
||||
---
|
||||
|
||||
## State Management
|
||||
|
||||
### Controlled & Uncontrolled Support
|
||||
|
||||
- [ ] Both modes supported: `value` (controlled) and `defaultValue` (uncontrolled)
|
||||
- [ ] Works correctly in either mode
|
||||
- [ ] No state desync between modes
|
||||
|
||||
**Detection:** Only `defaultValue` or only `value` supported
|
||||
|
||||
| State | Uncontrolled | Controlled | Handler |
|
||||
| ------- | ---------------- | ---------- | ----------------- |
|
||||
| Open | `defaultOpen` | `open` | `onOpenChange` |
|
||||
| Value | `defaultValue` | `value` | `onValueChange` |
|
||||
| Checked | `defaultChecked` | `checked` | `onCheckedChange` |
|
||||
|
||||
### Change Event Details
|
||||
|
||||
- [ ] Handler receives value and details object
|
||||
- [ ] Details includes `reason` (click, keyboard, blur, escape, etc.)
|
||||
- [ ] Details includes `event` (original DOM event)
|
||||
- [ ] Details includes `cancel()` for preventing change
|
||||
|
||||
**Detection:** Handler receives only value, no context
|
||||
|
||||
```typescript
|
||||
// BAD
|
||||
onOpenChange?: (open: boolean) => void;
|
||||
|
||||
// GOOD
|
||||
onOpenChange?: (open: boolean, details: ChangeDetails) => void;
|
||||
```
|
||||
|
||||
### Imperative Actions
|
||||
|
||||
- [ ] `actionsRef` prop exposes imperative methods where needed
|
||||
- [ ] Common actions: `open()`, `close()`, `toggle()`, `focus()`
|
||||
|
||||
---
|
||||
|
||||
## Props & API
|
||||
|
||||
### Boolean Props
|
||||
|
||||
- [ ] Use positive adjectives: `disabled`, `required`, `open`
|
||||
- [ ] Avoid `is`/`has` prefixes: not `isDisabled`, `isOpen`
|
||||
|
||||
### Event Handler Naming
|
||||
|
||||
- [ ] Pattern: `on` + Noun + Verb
|
||||
- [ ] Specific names over generic: `onValueChange` not `onChange`
|
||||
|
||||
### Standard Props by Category
|
||||
|
||||
**Interaction:** `disabled`, `required`, `readOnly`
|
||||
|
||||
**Collections:** `multiple`, `loopFocus`, `orientation`
|
||||
|
||||
**Popups:** `modal`, `closeOnEscape`, `closeOnOutsideClick`, `keepMounted`
|
||||
|
||||
### Refs
|
||||
|
||||
- [ ] Ref forwarded to root DOM element
|
||||
- [ ] Parent can access DOM for focus, measurement
|
||||
|
||||
**Detection:** `forwardRef` not used
|
||||
|
||||
### Polymorphism
|
||||
|
||||
- [ ] Uses `render` prop or `asChild`, not `as` prop
|
||||
- [ ] `render` function receives props and state
|
||||
- [ ] State accessible for conditional rendering
|
||||
|
||||
**Detection:** Component has `as` prop
|
||||
|
||||
---
|
||||
|
||||
## Data Attributes & Styling
|
||||
|
||||
### State via Data Attributes
|
||||
|
||||
- [ ] State exposed via `data-*` attributes
|
||||
- [ ] Enables CSS-only styling without JS
|
||||
|
||||
**Detection:** Inline styles for state, no data attributes
|
||||
|
||||
| Attribute | When Present |
|
||||
| ------------------ | --------------------------- |
|
||||
| `data-open` | Component is open |
|
||||
| `data-closed` | Component is closed |
|
||||
| `data-checked` | Toggle is checked |
|
||||
| `data-disabled` | Component is disabled |
|
||||
| `data-highlighted` | Item has focus within group |
|
||||
| `data-side` | Popup position side |
|
||||
| `data-align` | Popup alignment |
|
||||
|
||||
### CSS Variables
|
||||
|
||||
- [ ] CSS variables documented for customization
|
||||
- [ ] Standard variables: `--available-height`, `--anchor-width`, `--transform-origin`
|
||||
|
||||
### No Shipped CSS
|
||||
|
||||
- [ ] Component is headless — no CSS imported
|
||||
- [ ] User brings their own styles
|
||||
|
||||
**Detection:** `import 'component/styles.css'` in component
|
||||
|
||||
---
|
||||
|
||||
## Animation Support
|
||||
|
||||
- [ ] Exit animation possible (element not immediately unmounted)
|
||||
- [ ] `data-state="open"` / `data-state="closed"` for CSS transitions
|
||||
- [ ] `keepMounted` option for JS animation libraries
|
||||
|
||||
**Detection:** `{open && <Content />}` pattern without animation support
|
||||
|
||||
---
|
||||
|
||||
## SSR Safety
|
||||
|
||||
- [ ] No `document` or `window` at module scope
|
||||
- [ ] Portals handle SSR (render fallback or wait for mount)
|
||||
- [ ] IDs generated safely (no Math.random at module level)
|
||||
|
||||
**Detection:** `document.body` reference outside effect
|
||||
|
||||
---
|
||||
|
||||
## Accessibility
|
||||
|
||||
For full accessibility review, load the `aria` skill.
|
||||
|
||||
Quick checks:
|
||||
|
||||
- [ ] All interactive elements have accessible names
|
||||
- [ ] Keyboard navigation works
|
||||
- [ ] Focus managed for modals (trapped, restored)
|
||||
- [ ] ARIA attributes reflect component state
|
||||
@@ -0,0 +1,50 @@
|
||||
# Anti-Patterns & Severity
|
||||
|
||||
Quick reference for common issues and their severity.
|
||||
|
||||
---
|
||||
|
||||
## Anti-Pattern Summary
|
||||
|
||||
| Anti-Pattern | Detection | Fix |
|
||||
| ------------------- | ------------------------------------- | ----------------------------- |
|
||||
| Prop explosion | >10 props on one component | Use compound components |
|
||||
| Inline state styles | `style={{ opacity: disabled ? ... }}` | Use data attributes |
|
||||
| Shipped CSS | `import 'styles.css'` in component | Headless, user brings styles |
|
||||
| `as` prop | `<Button as="a">` | Use `render` or `asChild` |
|
||||
| Missing controlled | Only `defaultValue` | Add `value` + `onValueChange` |
|
||||
| Context collision | Nested instances share state | Scope contexts per Root |
|
||||
| No exit animation | `{open && ...}` without `keepMounted` | Add animation support |
|
||||
| SSR unsafe | `document.body` at module scope | Guard with mount check |
|
||||
| No ref forwarding | Missing `forwardRef` | Forward ref to DOM |
|
||||
|
||||
---
|
||||
|
||||
## Severity Guide
|
||||
|
||||
### Critical
|
||||
|
||||
| Issue | Why |
|
||||
| ---------------------------- | ----------------------------------- |
|
||||
| Missing controlled support | Can't integrate with external state |
|
||||
| Context collision in nesting | Breaks composition |
|
||||
| SSR crash | Breaks server rendering |
|
||||
| Memory leak (no cleanup) | Production issue |
|
||||
|
||||
### Major
|
||||
|
||||
| Issue | Why |
|
||||
| ------------------------- | -------------------- |
|
||||
| Prop explosion | Poor DX, inflexible |
|
||||
| Missing data attributes | Can't style with CSS |
|
||||
| No exit animation support | Poor UX |
|
||||
| Boolean trap | Confusing API |
|
||||
| Missing ref forwarding | Can't access DOM |
|
||||
|
||||
### Minor
|
||||
|
||||
| Issue | Why |
|
||||
| --------------------- | ------------------- |
|
||||
| Inconsistent naming | API inconsistency |
|
||||
| Missing CSS variables | Harder to customize |
|
||||
| Verbose handler names | Minor DX issue |
|
||||
@@ -0,0 +1,42 @@
|
||||
# Video.js Component Checklist
|
||||
|
||||
Video.js-specific patterns. See [videojs.md](../../references/videojs.md) for architecture details.
|
||||
|
||||
---
|
||||
|
||||
## Core Class
|
||||
|
||||
- [ ] Core class in `@videojs/core` (not in platform packages)
|
||||
- [ ] Props interface with `optional | undefined` pattern
|
||||
- [ ] State interface uses `Pick<FeatureState, ...>` for primitives only
|
||||
- [ ] `static readonly defaultProps` with `NonNullableObject<Props>` type
|
||||
- [ ] `setProps()` merges with defaults via `defaults()` utility
|
||||
- [ ] Namespace exports `Props` and `State` types
|
||||
|
||||
## State vs Attrs Separation
|
||||
|
||||
- [ ] `getState()` returns primitives only (no methods)
|
||||
- [ ] `getAttrs()` returns ARIA only (no `data-*`)
|
||||
- [ ] Data attribute enum with JSDoc for API tooling
|
||||
|
||||
## Web Component (Lit)
|
||||
|
||||
- [ ] Extends `MediaElement`
|
||||
- [ ] Uses `PlayerController` with selector for store subscription
|
||||
- [ ] Uses `AbortController` for cleanup in `disconnectedCallback`
|
||||
- [ ] `willUpdate`: syncs props to core via `setProps()`
|
||||
- [ ] `update`: applies attrs and state data attrs
|
||||
|
||||
## React Component
|
||||
|
||||
- [ ] Uses `useState(() => new Core())` for lazy initialization
|
||||
- [ ] Uses `usePlayer(selector)` for store subscription
|
||||
- [ ] Uses `renderElement()` for consistent rendering
|
||||
- [ ] Props type extends `UIComponentProps<Tag, State>`
|
||||
- [ ] Namespace exports `Props` and `State` types
|
||||
|
||||
## Common
|
||||
|
||||
- [ ] Missing feature handled with `logMissingFeature()`
|
||||
- [ ] Web Component registered in `define/ui/` with `HTMLElementTagNameMap`
|
||||
- [ ] `static readonly tagName = 'media-{name}'`
|
||||
Reference in New Issue
Block a user