minor fixes to drawer and modal#1724
Conversation
|
@jjagielka is attempting to deploy a commit to the Themesberg Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughApplies Tailwind not-first to avatar stacked spacing, forwards Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Drawer
participant Dialog
App->>Drawer: open, form, classes, other props
Drawer->>Dialog: forward props (including classes via ...restProps)
Dialog-->>Drawer: rendered dialog UI
Drawer-->>App: drawer with form content
sequenceDiagram
participant App
participant Modal
participant Dialog
App->>Modal: props (classes, transitionParams, ...)
Modal->>Dialog: pass {classes} then ...restProps
Dialog-->>Modal: rendered modal UI
Modal-->>App: modal content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/drawer/theme.ts (1)
5-5: Type surface broadened to include dialog classes — aligns with extend(Dialog)Including Classes ensures Drawer’s classes can target Dialog’s slots (e.g., form). This matches the runtime behavior of extending Dialog. If tailwind-variants’ extend already merges slots in the resulting type, this may be redundant but is harmless and clarifies intent.
Consider adding a short comment noting why both Classes and Classes are intersected, for future maintainers.
src/routes/docs/components/drawer.md (1)
195-199: Docs: Great demo offormmode andclasses.form; small a11y tweakConsider wiring
aria-labelledbyto the heading for consistent labeling (as shown in the default example).Apply this diff:
-<Drawer form bind:open={open4} classes={{ form: "space-y-6 mb-6" }}> - <h5 class="mb-6 inline-flex items-center text-base font-semibold text-gray-500 uppercase dark:text-gray-400"> +<Drawer form bind:open={open4} aria-labelledby="drawer-form-title" classes={{ form: "space-y-6 mb-6" }}> + <h5 id="drawer-form-title" class="mb-6 inline-flex items-center text-base font-semibold text-gray-500 uppercase dark:text-gray-400">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/lib/avatar/theme.ts(1 hunks)src/lib/drawer/Drawer.svelte(1 hunks)src/lib/drawer/theme.ts(1 hunks)src/lib/modal/Modal.svelte(1 hunks)src/routes/docs/components/drawer.md(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/lib/drawer/theme.ts (2)
src/lib/theme/themeUtils.ts (1)
Classes(14-16)src/lib/dialog/theme.ts (1)
dialog(6-21)
🔇 Additional comments (9)
src/lib/avatar/theme.ts (1)
18-18: Confirm Tailwind’snot-firstVariant SupportThe CSS class (
not-first:-ms-4) is correct for skipping the first avatar and providing RTL-friendly spacing, but we still need to verify that your Tailwind setup supports thenot-firstvariant (added in v3.3+).Please confirm by checking both:
- Your Tailwind dependency version in
package.json- Existence of a
tailwind.config.js(or.ts) where variants aren’t restrictedYou can run:
# 1. Check Tailwind version in package.json rg -nP '"(?:tailwindcss)"\s*:\s*"[0-9]+\.[0-9]+\.[0-9]+"' package.json # 2. Locate your Tailwind config file fd 'tailwind.config.*' -t f -d 3If your version is ≥3.3 and your config doesn’t disable the
not-firstvariant, you’re all set. Otherwise, please upgrade Tailwind or enable the variant in your config.src/lib/drawer/Drawer.svelte (3)
10-10: Good change: stop destructuringclassesso it forwards to Dialog viarestPropsThis enables consumers to style Dialog slots (e.g.,
form) through Drawer without changing Drawer’s own API surface.
38-38: Correctly deriving styling frommodalwhile still forwarding itUsing
modal: restProps.modallets Drawer’s theme add the non-modal base classes while still forwardingmodalto Dialog viarestProps. Nicely done.
51-51: Props forwarding ensuresformandclassesreach DialogForwarding
...restPropsmeans features likeformmode andclasses={{ form: "…" }}work without Drawer needing to re-declare them.src/routes/docs/components/drawer.md (5)
185-185: Docs: Avatar import addition is consistent with the new exampleBringing in Avatar matches the new stacked avatars section and the updated stacked spacing.
199-207: Docs: Using Label + Input/Textarea reads well and matches Flowbite Svelte patternsThe grouped Label usage and required fields are clear. No issues spotted.
208-215: Docs: Inline “right” snippet in Input is a nice touchGood illustration of input adornments with a small action button.
216-220: Docs: Stacked Avatars now align visually with the newnot-first:-ms-4behaviorExample effectively demonstrates the updated stacked spacing.
221-223: Docs: Submit CTA in form mode showcases dialog-form behaviorClear and consistent with the example’s intent.
2 participants
📑 Description
Fixes:
classestoDialogforDrawerandModalAvatarstacked layoutStatus
✅ Checks
api-checkdirectory as requiredpnpm check && pnpm test:e2eℹ Additional Information
Summary by CodeRabbit