feat(ui): extract shared form and overlay primitives - #617
Conversation
WalkthroughThe shared ChangesShared UI migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Storybook
participant AppAdapter
participant SharedDialog
participant RadixDialog
Storybook->>AppAdapter: Render dialog story
AppAdapter->>SharedDialog: Re-exported Dialog components
SharedDialog->>RadixDialog: Open and close dialog primitives
RadixDialog-->>Storybook: Render accessible dialog state
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/wodsmith-start/src/components/ui/dialog.stories.tsx (1)
99-108: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the sheet closes after clicking "Done".
The
EditingSheetplay function clicks the "Done" button but doesn't verify the sheet is removed from the DOM, unlikeConfirmationDialogwhich assertsdialogis removed after Cancel. Add the missing close assertion for parity.💚 Proposed fix
await userEvent.click(within(dialog).getByRole("button", { name: "Done" })) + await expect(dialog).not.toBeInTheDocument() }, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wodsmith-start/src/components/ui/dialog.stories.tsx` around lines 99 - 108, Update the EditingSheet play function to verify the dialog closes after clicking the “Done” button. Using the existing dialog reference and testing utilities, assert that the dialog is removed from the DOM, matching the close assertion used by ConfirmationDialog.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/package.json`:
- Around line 58-60: Add react-dom to the peerDependencies object alongside
react in the package metadata, using a compatible React 19 version range
consistent with the existing react declaration.
In `@packages/ui/src/components/form.tsx`:
- Around line 43-52: Update useFormField to validate the context before calling
getFieldState: check whether fieldContext.name is missing, throw the existing
“useFormField should be used within <FormField>” error, then retrieve field
state only after validation. This avoids the ineffective truthiness check caused
by FormFieldContext’s default value.
In `@packages/ui/src/components/popover.tsx`:
- Line 1: Add the "use client" directive at the top of the popover component
module, before the `@radix-ui/react-popover` import, matching the client-component
setup used by dialog.tsx, dropdown-menu.tsx, and sheet.tsx.
In `@packages/ui/src/components/select.tsx`:
- Around line 1-5: Add the "use client" directive at the very top of select.tsx,
before the React and Radix imports, so the Select components using client-only
hooks and portals are treated as client components.
In `@packages/ui/test/primitives.test.tsx`:
- Line 18: Add exactly one nearby `// `@lat`:` comment for each `it(...)` test in
`packages/ui/test/primitives.test.tsx`, including the test named “exposes an
accessible labeled checkbox with checked state” and the other test in the file;
use the existing project convention for the reference value and avoid adding
duplicates.
---
Nitpick comments:
In `@apps/wodsmith-start/src/components/ui/dialog.stories.tsx`:
- Around line 99-108: Update the EditingSheet play function to verify the dialog
closes after clicking the “Done” button. Using the existing dialog reference and
testing utilities, assert that the dialog is removed from the DOM, matching the
close assertion used by ConfirmationDialog.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7bdb43f1-582b-49ae-a5d9-5ce0b84a3eea
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (40)
apps/crew/src/components/ui/checkbox.tsxapps/crew/src/components/ui/dialog.tsxapps/crew/src/components/ui/dropdown-menu.tsxapps/crew/src/components/ui/form.tsxapps/crew/src/components/ui/popover.tsxapps/crew/src/components/ui/radio-group.tsxapps/crew/src/components/ui/select.tsxapps/crew/src/components/ui/sheet.tsxapps/crew/src/components/ui/tabs.tsxapps/crew/src/components/ui/tooltip.tsxapps/wodsmith-start/docs/ui-library-inventory.mdapps/wodsmith-start/scripts/generate-ui-library-inventory.mjsapps/wodsmith-start/src/components/ui/checkbox.tsxapps/wodsmith-start/src/components/ui/dialog.stories.tsxapps/wodsmith-start/src/components/ui/dialog.tsxapps/wodsmith-start/src/components/ui/dropdown-menu.stories.tsxapps/wodsmith-start/src/components/ui/dropdown-menu.tsxapps/wodsmith-start/src/components/ui/form.stories.tsxapps/wodsmith-start/src/components/ui/form.tsxapps/wodsmith-start/src/components/ui/popover.tsxapps/wodsmith-start/src/components/ui/radio-group.tsxapps/wodsmith-start/src/components/ui/select.tsxapps/wodsmith-start/src/components/ui/sheet.tsxapps/wodsmith-start/src/components/ui/tabs.stories.tsxapps/wodsmith-start/src/components/ui/tabs.tsxapps/wodsmith-start/src/components/ui/tooltip.tsxlat.md/ui-library.mdpackages/ui/package.jsonpackages/ui/src/components/checkbox.tsxpackages/ui/src/components/dialog.tsxpackages/ui/src/components/dropdown-menu.tsxpackages/ui/src/components/form.tsxpackages/ui/src/components/popover.tsxpackages/ui/src/components/radio-group.tsxpackages/ui/src/components/select.tsxpackages/ui/src/components/sheet.tsxpackages/ui/src/components/tabs.tsxpackages/ui/src/components/tooltip.tsxpackages/ui/test/compatibility.test.tspackages/ui/test/primitives.test.tsx
Independent stacked-diff audit: changes requestedAudited exactly Findings
CodeRabbit triage
Evidence
No files were edited, committed, or pushed. |
|
Follow-up pushed in 31ed97c.
Validation: @repo/ui build/type-check/lint and 25 tests pass; Start/Crew type-checks pass; Storybook static/dev smoke passes; Tabs axe reports 0 violations in light and dark and keyboard/click selection passes; EditingSheet reports all 5 interaction steps passing and returns focus after close; inventory, LAT, route/diff checks, and full pre-push pass. The previously documented primary-button contrast finding remains inherited stack-base debt and is intentionally unchanged for the separate reviewed layer. |
Independent re-review: LGTMRe-audited follow-up Verified:
Live CI has no failures: all build, lint, typecheck, test, both E2E shards, and Crew E2E jobs pass. Only the final E2E The previously noted primary-button contrast issue remains inherited stack-base debt from #616 and is not introduced by this PR delta. No files were edited, committed, or pushed during this re-review. |
d6316c1
into
codex/ui-package-foundation
Summary
Stack
Validation
The package build currently emits ignored JavaScript artifacts only; declaration output is not configured. Start and Crew application builds stop at the expected missing ignored .alchemy/local/wrangler.jsonc files, and this PR does not generate deployment state.
mainSummary by cubic
Extracted shared form and overlay primitives into
@repo/uisubpaths and replaced app-local copies with thin re-exports. Added Storybook coverage and tests, and applied review feedback for styling and docs/inventory updates.New Features
@repo/uiexports:checkbox,dialog,dropdown-menu,form,popover,radio-group,select,sheet,tabs,tooltip.Dependencies
@repo/uito 19.2.3.@radix-ui/react-checkbox,@radix-ui/react-dialog,@radix-ui/react-dropdown-menu,@radix-ui/react-popover,@radix-ui/react-radio-group,@radix-ui/react-select,@radix-ui/react-tooltip.Written for commit 31ed97c. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Documentation