Skip to content

Add 'auto' and 'full' in sizing array #18

Merged
josephdburdick merged 4 commits into
mainfrom
feat/add-auto-sizing
Apr 30, 2026
Merged

Add 'auto' and 'full' in sizing array #18
josephdburdick merged 4 commits into
mainfrom
feat/add-auto-sizing

Conversation

@josephdburdick
Copy link
Copy Markdown
Contributor

@josephdburdick josephdburdick commented Apr 30, 2026

Summary by CodeRabbit

  • New Features

    • Added 'auto' and 'full' snap values for drawer sizing; mixed snap arrays supported.
    • Playground demo showing mixed snaps and a loading→taller-content transition.
  • Bug Fixes / Stability

    • Reduced AUTO jitter and redundant resnap animations; snap selection preserved across reorderings.
    • Keyboard snap restore now supports tokenized snap targets.
  • Tests

    • Expanded tests for 'auto'/'full' sizing and clamping.
  • Chores

    • Updated playground scripts to new config filename extension.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5a00506d-f372-4cd7-a981-9076a1900053

📥 Commits

Reviewing files that changed from the base of the PR and between d1b4ccb and 86306fd.

📒 Files selected for processing (1)
  • src/components/Drawer.tsx

📝 Walkthrough

Walkthrough

Adds token-based snap points ('auto', 'full') with live content-height measurement and hysteresis; remaps snap indices when raw snap ordering changes; stabilizes resnap animations to avoid tiny repeated restarts; refactors tailwind-merge lazy init and cn fallback; extends types and tests; adds playground demo and updates Vite script references.

Changes

Cohort / File(s) Summary
Type System & Constants
src/types.ts, src/constants.ts
Expanded SnapPointValue to `number
Core Snap Resolution & Tests
src/hooks/useDrawerSnap.ts, src/hooks/useDrawerSnap.test.ts
Added measuredAutoHeight parameter; 'auto' resolves to clamped measured height, 'full' to available height. Improved AUTO measurement (ResizeObserver, scrollHeight vs child offsets) with ≤1px hysteresis. Tests updated for token values and mixed sizing arrays.
Resnap & Snap Index Stability
src/components/Drawer.tsx
Consume rawSnapValues to preserve logical snap identity; remap snapIndex when raw array re-sorts; track last resnap target and skip animate() when retarget Δ ≲ ~2px; clear tracker on exit.
Keyboard Integration
src/hooks/useDrawerKeyboardSnapMobile.ts
Widened snapBeforeKeyboardRef type to `SnapPointValue
Utilities (tailwind-merge)
src/utils.ts
Refactored lazy init with ensureTwMergeInit() and promise deduplication; cn() now triggers async init and falls back to clsx instead of throwing when tailwind-merge is absent.
Playground Demo
playground/src/PlaygroundApp.tsx
Added snapsLoadingTallerThanContent scenario/demo using mixed sizing (AUTO + 480 + FULL) that starts with a skeleton then replaces with taller content; wires event logging and default snap.
Build Scripts
package.json
Updated playground Vite config references from playground/vite.config.tsplayground/vite.config.mts for playground, playground:build, and playground:preview.

Sequence Diagram(s)

sequenceDiagram
    participant App as App
    participant RO as ResizeObserver
    participant Snap as useDrawerSnap
    participant Drawer as Drawer
    participant Anim as AnimationEngine

    App->>Drawer: open drawer with 'auto' snap
    Drawer->>Snap: resolveSnapValueToPx('auto', availableH, measuredH)
    Snap-->>Drawer: targetHeight
    Drawer->>Anim: animate(currentH, targetHeight)
    Anim-->>Drawer: animationComplete
    Drawer->>Drawer: store lastRawSnap & lastTarget

    App->>RO: content changes (loading → taller)
    RO->>Snap: report new measuredHeight
    Snap->>Snap: apply ≤1px hysteresis
    alt height change > threshold
        Snap->>Drawer: new targetHeight
        Drawer->>Anim: animate(currentH, newTargetHeight)
        Anim-->>Drawer: onAnimationComplete
    else height change ≲ threshold
        Note over Drawer: skip redundant animation
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • Make Tailwind optional #16 — Refactors src/utils.ts to use lazy tailwind-merge initialization with promise deduplication and graceful fallback; directly related to the utility changes here.

Poem

🐇 I measured the meadow, small bumps and all,

'Auto' finds the height, 'Full' answers the call,
Snaps now settle soft, no jittering tune,
I hop with delight beneath the bright moon,
Tiny paws clap — the drawer sleeps soon.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding support for 'auto' and 'full' token values in sizing arrays, which is the primary feature across multiple files including constants, types, and hook logic.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/add-auto-sizing

Review rate limit: 1/3 review remaining, refill in 39 minutes and 26 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@josephdburdick josephdburdick changed the title Add auto sizing Add 'auto' and 'full' in sizing array Apr 30, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/hooks/useDrawerSnap.ts (1)

381-409: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Preserve the active raw snap when mixed arrays reorder.

These lines make mixed sizing arrays recompute and re-sort as measuredAutoHeight changes. The new test in src/hooks/useDrawerSnap.test.ts at Line 209 already shows ['auto', 480] changing from [100, 480] to [480, 600]. src/components/Drawer.tsx still keeps the active stop as snapIndex, so a drawer parked on 'auto' can silently switch to 480 once AUTO grows past it. The same problem affects controlled activeSnapPoint="auto" because the prop value does not change, so the index is never re-resolved. Please preserve the active raw snap value and remap the index after recomputing heights/rawValues.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useDrawerSnap.ts` around lines 381 - 409, When recomputing sizing
with resolveSizingToHeights (triggered by measuredAutoHeight changes) capture
the current active raw snap (e.g. prevRaw = rawValues[snapIndex] or if
controlled use activeSnapPoint) before calling resolveSizingToHeights, then
after you get the new {heights, rawValues} remap the active index by locating
the same raw value in the new rawValues (use strict equality for numbers and the
'auto' token) and update the component's snap index/state accordingly so a
drawer parked on 'auto' preserves its logical stop instead of silently switching
when measuredAutoHeight changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@playground/src/PlaygroundApp.tsx`:
- Around line 398-419: The demo's default snap is inconsistent with its text:
currently the Drawer is configured with sizing={[SNAP_POINT.AUTO, 480,
DRAWER_SIZING.FULL]} and defaultSnapPoint={SNAP_POINT.AUTO} but the copy and
helper text claim it opens at 480px; either change defaultSnapPoint to the 480
stop (e.g., use the numeric snap or SNAP_POINT value representing 480) so the
behavior matches the text, or update the inline comment/title/helper paragraph
to state that the panel opens at AUTO (content-fit) instead of 480px; locate and
edit the defaultSnapPoint prop and the nearby descriptive strings (title and
paragraph) in PlaygroundApp.tsx to make them consistent.

---

Outside diff comments:
In `@src/hooks/useDrawerSnap.ts`:
- Around line 381-409: When recomputing sizing with resolveSizingToHeights
(triggered by measuredAutoHeight changes) capture the current active raw snap
(e.g. prevRaw = rawValues[snapIndex] or if controlled use activeSnapPoint)
before calling resolveSizingToHeights, then after you get the new {heights,
rawValues} remap the active index by locating the same raw value in the new
rawValues (use strict equality for numbers and the 'auto' token) and update the
component's snap index/state accordingly so a drawer parked on 'auto' preserves
its logical stop instead of silently switching when measuredAutoHeight changes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4a79d9c9-5097-46eb-b98b-62ab79fbc87d

📥 Commits

Reviewing files that changed from the base of the PR and between 4b64ccd and 319e487.

📒 Files selected for processing (10)
  • package.json
  • playground/src/PlaygroundApp.tsx
  • playground/vite.config.mts
  • src/components/Drawer.tsx
  • src/constants.ts
  • src/hooks/useDrawerKeyboardSnapMobile.ts
  • src/hooks/useDrawerSnap.test.ts
  • src/hooks/useDrawerSnap.ts
  • src/types.ts
  • src/utils.ts

Comment thread playground/src/PlaygroundApp.tsx
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/Drawer.tsx`:
- Around line 358-379: The problem is that the "lockstep" useEffect writes
lastRawSnapRef from the current snapIndex before the remap effect can read the
previous logical raw value, so reordering can lose the prior identity; fix by
tracking the previous rawSnapValues snapshot in a ref (e.g.,
prevRawSnapValuesRef) and derive the previous raw from that snapshot (compute
prevRaw = prevRawSnapValuesRef.current?.[snapIndex] or via indexToRawValue
against the prior array) inside the remap effect so remapping uses the true
previous raw, then update prevRawSnapValuesRef (and only then update
lastRawSnapRef) so the two effects no longer clobber each other (update
references: lastRawSnapRef, rawSnapValues, snapIndex, and the two useEffect
blocks).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9f79622d-6105-4e13-8834-31f6e071375d

📥 Commits

Reviewing files that changed from the base of the PR and between 319e487 and d1b4ccb.

📒 Files selected for processing (2)
  • playground/src/PlaygroundApp.tsx
  • src/components/Drawer.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • playground/src/PlaygroundApp.tsx

Comment thread src/components/Drawer.tsx Outdated
@josephdburdick josephdburdick merged commit 0a13592 into main Apr 30, 2026
5 checks passed
This was referenced Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant