Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude/agents/code-reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Principal reviewer only — identify issues; do not implement fixes. Adapted fro

1. Git diff for the branch (`git diff main...HEAD`)
2. The GitHub issue being implemented (acceptance criteria are the spec)
3. Standards: `CLAUDE.md` (architecture invariants) and the parent PRD (issue #1, Implementation & Testing Decisions)
3. Standards: `CLAUDE.md`, `standards/slideshow.md`, and the parent PRD (issue #1, Implementation & Testing Decisions)

## Lenses

Expand Down
42 changes: 42 additions & 0 deletions .claude/skills/composition-patterns/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
---
name: composition-patterns
description: React composition patterns for the editor shell. Use when refactoring components with boolean prop proliferation, splitting large components, or designing flexible component APIs in src/editor-shell/.
metadata:
tags: react, composition, architecture, editor-shell
source: vercel-labs/agent-skills (subset)
---

## When to use

Load this skill before adding or refactoring UI in `src/editor-shell/`. Read `standards/slideshow.md` alongside these rules.

Triggers:

- A component file approaches or exceeds the 300-line cap
- Adding boolean props to customize component behavior (`isEditing`, `showFooter`, etc.)
- One component handles multiple slide/dialog/panel modes via conditionals
- Sibling components need shared state (e.g. drag preview + storyboard grid)

## Applicable rules (this project)

Only three rules from the Vercel composition-patterns skill apply at current scale. Read the full rule file before implementing:

| Priority | Rule | File |
| -------- | ---- | ---- |
| CRITICAL | Avoid boolean prop proliferation | [rules/architecture-avoid-boolean-props.md](rules/architecture-avoid-boolean-props.md) |
| MEDIUM | Explicit variant components | [rules/patterns-explicit-variants.md](rules/patterns-explicit-variants.md) |
| HIGH | Lift state into providers | [rules/state-lift-state.md](rules/state-lift-state.md) |

## Not applicable yet

Skip these until the codebase needs them:

- `architecture-compound-components` — no shared compound context yet
- `state-context-interface` / `state-decouple-implementation` — state still lives in App with callback props
- `patterns-children-over-render-props` — no render-prop APIs
- `react19-no-forwardref` — not using forwardRef

## Local examples

- `TitleSlideDialog` and `SlideSettingsDialog` are explicit variants (not one dialog with `isTitle` boolean)
- App lifts state; dialogs receive data + callbacks — provider pattern becomes relevant when siblings outside the dialog need the same state
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
---
title: Avoid Boolean Prop Proliferation
impact: CRITICAL
impactDescription: prevents unmaintainable component variants
tags: composition, props, architecture
---

## Avoid Boolean Prop Proliferation

Don't add boolean props like `isThread`, `isEditing`, `isDMThread` to customize
component behavior. Each boolean doubles possible states and creates
unmaintainable conditional logic. Use composition instead.

**Incorrect (boolean props create exponential complexity):**

```tsx
function Composer({
onSubmit,
isThread,
channelId,
isDMThread,
dmId,
isEditing,
isForwarding,
}: Props) {
return (
<form>
<Header />
<Input />
{isDMThread ? (
<AlsoSendToDMField id={dmId} />
) : isThread ? (
<AlsoSendToChannelField id={channelId} />
) : null}
{isEditing ? (
<EditActions />
) : isForwarding ? (
<ForwardActions />
) : (
<DefaultActions />
)}
<Footer onSubmit={onSubmit} />
</form>
)
}
```

**Correct (composition eliminates conditionals):**

```tsx
// Channel composer
function ChannelComposer() {
return (
<Composer.Frame>
<Composer.Header />
<Composer.Input />
<Composer.Footer>
<Composer.Attachments />
<Composer.Formatting />
<Composer.Emojis />
<Composer.Submit />
</Composer.Footer>
</Composer.Frame>
)
}

// Thread composer - adds "also send to channel" field
function ThreadComposer({ channelId }: { channelId: string }) {
return (
<Composer.Frame>
<Composer.Header />
<Composer.Input />
<AlsoSendToChannelField id={channelId} />
<Composer.Footer>
<Composer.Formatting />
<Composer.Emojis />
<Composer.Submit />
</Composer.Footer>
</Composer.Frame>
)
}

// Edit composer - different footer actions
function EditComposer() {
return (
<Composer.Frame>
<Composer.Input />
<Composer.Footer>
<Composer.Formatting />
<Composer.Emojis />
<Composer.CancelEdit />
<Composer.SaveEdit />
</Composer.Footer>
</Composer.Frame>
)
}
```

Each variant is explicit about what it renders. We can share internals without
sharing a single monolithic parent.
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
---
title: Create Explicit Component Variants
impact: MEDIUM
impactDescription: self-documenting code, no hidden conditionals
tags: composition, variants, architecture
---

## Create Explicit Component Variants

Instead of one component with many boolean props, create explicit variant
components. Each variant composes the pieces it needs. The code documents
itself.

**Incorrect (one component, many modes):**

```tsx
// What does this component actually render?
<Composer
isThread
isEditing={false}
channelId='abc'
showAttachments
showFormatting={false}
/>
```

**Correct (explicit variants):**

```tsx
// Immediately clear what this renders
<ThreadComposer channelId="abc" />

// Or
<EditMessageComposer messageId="xyz" />

// Or
<ForwardMessageComposer messageId="123" />
```

Each implementation is unique, explicit and self-contained. Yet they can each
use shared parts.

**Implementation:**

```tsx
function ThreadComposer({ channelId }: { channelId: string }) {
return (
<ThreadProvider channelId={channelId}>
<Composer.Frame>
<Composer.Input />
<AlsoSendToChannelField channelId={channelId} />
<Composer.Footer>
<Composer.Formatting />
<Composer.Emojis />
<Composer.Submit />
</Composer.Footer>
</Composer.Frame>
</ThreadProvider>
)
}

function EditMessageComposer({ messageId }: { messageId: string }) {
return (
<EditMessageProvider messageId={messageId}>
<Composer.Frame>
<Composer.Input />
<Composer.Footer>
<Composer.Formatting />
<Composer.Emojis />
<Composer.CancelEdit />
<Composer.SaveEdit />
</Composer.Footer>
</Composer.Frame>
</EditMessageProvider>
)
}

function ForwardMessageComposer({ messageId }: { messageId: string }) {
return (
<ForwardMessageProvider messageId={messageId}>
<Composer.Frame>
<Composer.Input placeholder="Add a message, if you'd like." />
<Composer.Footer>
<Composer.Formatting />
<Composer.Emojis />
<Composer.Mentions />
</Composer.Footer>
</Composer.Frame>
</ForwardMessageProvider>
)
}
```

Each variant is explicit about:

- What provider/state it uses
- What UI elements it includes
- What actions are available

No boolean prop combinations to reason about. No impossible states.
125 changes: 125 additions & 0 deletions .claude/skills/composition-patterns/rules/state-lift-state.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
---
title: Lift State into Provider Components
impact: HIGH
impactDescription: enables state sharing outside component boundaries
tags: composition, state, context, providers
---

## Lift State into Provider Components

Move state management into dedicated provider components. This allows sibling
components outside the main UI to access and modify state without prop drilling
or awkward refs.

**Incorrect (state trapped inside component):**

```tsx
function ForwardMessageComposer() {
const [state, setState] = useState(initialState)
const forwardMessage = useForwardMessage()

return (
<Composer.Frame>
<Composer.Input />
<Composer.Footer />
</Composer.Frame>
)
}

// Problem: How does this button access composer state?
function ForwardMessageDialog() {
return (
<Dialog>
<ForwardMessageComposer />
<MessagePreview /> {/* Needs composer state */}
<DialogActions>
<CancelButton />
<ForwardButton /> {/* Needs to call submit */}
</DialogActions>
</Dialog>
)
}
```

**Incorrect (useEffect to sync state up):**

```tsx
function ForwardMessageDialog() {
const [input, setInput] = useState('')
return (
<Dialog>
<ForwardMessageComposer onInputChange={setInput} />
<MessagePreview input={input} />
</Dialog>
)
}

function ForwardMessageComposer({ onInputChange }) {
const [state, setState] = useState(initialState)
useEffect(() => {
onInputChange(state.input) // Sync on every change 😬
}, [state.input])
}
```

**Incorrect (reading state from ref on submit):**

```tsx
function ForwardMessageDialog() {
const stateRef = useRef(null)
return (
<Dialog>
<ForwardMessageComposer stateRef={stateRef} />
<ForwardButton onPress={() => submit(stateRef.current)} />
</Dialog>
)
}
```

**Correct (state lifted to provider):**

```tsx
function ForwardMessageProvider({ children }: { children: React.ReactNode }) {
const [state, setState] = useState(initialState)
const forwardMessage = useForwardMessage()
const inputRef = useRef(null)

return (
<Composer.Provider
state={state}
actions={{ update: setState, submit: forwardMessage }}
meta={{ inputRef }}
>
{children}
</Composer.Provider>
)
}

function ForwardMessageDialog() {
return (
<ForwardMessageProvider>
<Dialog>
<ForwardMessageComposer />
<MessagePreview /> {/* Custom components can access state and actions */}
<DialogActions>
<CancelButton />
<ForwardButton /> {/* Custom components can access state and actions */}
</DialogActions>
</Dialog>
</ForwardMessageProvider>
)
}

function ForwardButton() {
const { actions } = use(Composer.Context)
return <Button onPress={actions.submit}>Forward</Button>
}
```

The ForwardButton lives outside the Composer.Frame but still has access to the
submit action because it's within the provider. Even though it's a one-off
component, it can still access the composer's state and actions from outside the
UI itself.

**Key insight:** Components that need shared state don't have to be visually
nested inside each other—they just need to be within the same provider.
Loading
Loading