Conversation
|
@jjagielka is attempting to deploy a commit to the Themesberg Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a new Dialog component and theme, centralizes dialog-related props/types, refactors Modal and Drawer to delegate rendering/behavior to Dialog, updates drawer/modal themes to extend dialog, and migrates examples/docs from bind:hidden/backdrop/activateClickOutside → bind:open/modal/outsideclose and transitionParams. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Drawer
participant Dialog
participant HTMLDialog
User->>Drawer: set open = true
Drawer->>Dialog: render(bind:open, transition, transitionParams, outsideclose, modal)
Dialog->>HTMLDialog: show() / showModal()
User-->>Dialog: interact (click outside / Esc / CloseButton / submit)
Dialog->>Dialog: evaluate autoclose/outsideclose/permanent/onaction/onsubmit/oncancel
Dialog->>HTMLDialog: close()
Dialog-->>Drawer: bind updates open = false
sequenceDiagram
actor User
participant Modal
participant Dialog
participant HTMLDialog
User->>Modal: set open = true
Modal->>Dialog: render(bind:open, transitionParams)
Dialog->>HTMLDialog: showModal()
User-->>Dialog: submit / cancel / toggle
Dialog->>Dialog: call onsubmit / oncancel / onaction and manage returnValue
Dialog->>HTMLDialog: close()
Dialog-->>Modal: bind updates open = false
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ 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: 31
🔭 Outside diff range comments (7)
src/routes/blocks/application/crud-create-drawers.md (1)
50-52: Fix label association and brand field typo
- “Bland” appears to be a typo for “Brand.”
- The Description label’s
fordoesn’t match the textareaid, breaking label association.Apply this diff:
- <div class="mb-6"> - <Label for="bland" class="mb-2 block">Bland</Label> - <Input id="bland" name="bland" required placeholder="Product brand" /> - </div> + <div class="mb-6"> + <Label for="brand" class="mb-2 block">Brand</Label> + <Input id="brand" name="brand" required placeholder="Product brand" /> + </div> @@ - <div class="mb-6"> - <Label for="brand" class="mb-2">Description</Label> - <Textarea id="message" placeholder="Enter event description here" rows={4} name="message" /> - </div> + <div class="mb-6"> + <Label for="message" class="mb-2">Description</Label> + <Textarea id="message" placeholder="Enter event description here" rows={4} name="message" /> + </div>Also applies to: 64-66
src/routes/builder/modal/+page.svelte (2)
63-80: Update the generator to emit transitionParams instead of paramsThe generated snippet is out of date with the new API.
- props.push(`params={{${paramsString}}}`); + props.push(`transitionParams={{${paramsString}}}`);
82-85: Update generated markup to use bind:open and drop legacy {closeModal}Keep using modalStatus as the bound variable to minimize changes elsewhere.
- return `<Modal title="Terms of Service" {modalStatus} {closeModal}${propsString}> + return `<Modal title="Terms of Service"${propsString} bind:open={modalStatus}> Modal content </Modal>`;src/routes/builder/drawer/+page.svelte (1)
84-118: Builder code generator still emits legacy Drawer API and contains invalid markup.The generated snippet still uses legacy props and structure (activateClickOutside, transitionType, params, drawerStatus/closeDrawer, drawer.toggle) and has a malformed class attribute on Drawerhead. This will produce incorrect and broken code for users copying from the builder.
Update to emit the new API: open/bind:open, outsideclose, transition, transitionParams, and correct handlers.
Apply this diff to fix the generator:
- if (!outsideclickStatus) props.push(" activateClickOutside={false}"); + if (!outsideclickStatus) props.push(" outsideclose={false}"); @@ - if (currentTransition !== transitions[0]) { - props.push(` transitionType={${currentTransition.transition.name}}`); + if (currentTransition !== transitions[0]) { + props.push(` transition={${currentTransition.transition.name}}`); @@ - props.push(` params={{${paramsString}}}`); + props.push(` transitionParams={{${paramsString}}}`); @@ - return `<Button onclick={drawer.toggle}>Drawer</Button> -<Drawer drawerStatus={drawerStatus} closeDrawer={closeDrawer}${propsString}> - <Drawerhead onclick={closeDrawer} class="mb-4> + return `<Button onclick={() => (open = true)}>Drawer</Button> +<Drawer bind:open${propsString}> + <Drawerhead onclick={() => (open = false)} class="mb-4"> <h5 id="drawer-label" class="inline-flex items-center text-xl font-semibold text-gray-500 dark:text-gray-400"> <InfoCircleSolid class="me-2.5 h-5 w-5" />${selectedTransition} drawer </h5> </Drawerhead> My Drawer </Drawer>`;src/lib/modal/Modal.svelte (1)
46-80: Update the in-file props documentation block to match the new API.The block still lists removed props (onaction, oncancel, onsubmit, ontoggle, form, modal, autoclose, focustrap, params). Replace with the current public API (e.g., open, permanent, dismissable, outsideclose, size, placement, classes, transition, transitionParams, fullscreen, etc.).
I can draft a revised doc block if you want the exact text.
src/lib/drawer/Drawer.svelte (2)
21-28: Avoid binding window size for transition computation.Binding innerWidth/innerHeight and translating by viewport dimensions causes unnecessary reactivity and excessive off-screen motion. The simpler fixed-distance defaults above are adequate and perform better.
Also applies to: 30-30
43-55: Update the inline props doc to reflect the new API.The comment still lists deprecated props (hidden, activateClickOutside, position, backdrop, transitionType). Replace with: open, width, placement, class, classes, transition, transitionParams, outsideclose (passed through), modal (if supported via Dialog), bodyScrolling (only if implemented), ...restProps.
I can propose an updated block if desired.
🧹 Nitpick comments (23)
src/routes/blocks/application/crud-update-drawers.md (1)
35-36: Add aria-labelledby for accessible dialog namingTie the Drawer to the title element to provide an accessible name for assistive tech.
Apply this diff:
- <Drawer bind:open id="sidebar4"> + <Drawer bind:open id="sidebar4" aria-labelledby="drawer-label">src/routes/blocks/application/crud-read-drawers.md (2)
32-36: Add aria-labelledby to improve dialog accessibilityAssociate the dialog with its visible title.
Apply this diff:
- <Drawer bind:open id="sidebar1"> + <Drawer bind:open id="sidebar1" aria-labelledby="drawer-label">
32-36: Consider restoring a visible close affordanceRelying solely on outside-click/Escape may reduce discoverability. Adding a CloseButton improves UX, especially on touch devices.
Apply this diff to include a close control without heavy layout changes:
- <Drawer bind:open id="sidebar1"> + <Drawer bind:open id="sidebar1" aria-labelledby="drawer-label"> @@ - <div> - <h4 id="drawer-label" class="mb-1.5 text-xl leading-none font-semibold text-gray-900 dark:text-white">Apple iMac 25"</h4> - <p class="mb-5 text-xl font-bold text-gray-900 dark:text-white">$2999</p> - </div> + <div class="mb-2 flex items-start justify-between"> + <div> + <h4 id="drawer-label" class="mb-1.5 text-xl leading-none font-semibold text-gray-900 dark:text-white">Apple iMac 25"</h4> + <p class="text-xl font-bold text-gray-900 dark:text-white">$2999</p> + </div> + <CloseButton class="ml-2" onclick={() => (open = false)} aria-label="Close product details drawer" /> + </div>If the intent is to rely on outside-close globally, verify that
outsideclosedefaults to true for Drawer. If not, consider explicitly setting it in examples.src/routes/blocks/application/crud-create-drawers.md (1)
42-44: Add aria-labelledby for accessible dialog namingConnect the Drawer to its title for screen readers.
Apply this diff:
- <Drawer bind:open id="sidebar4"> + <Drawer bind:open id="sidebar4" aria-labelledby="drawer-label">src/lib/index.ts (1)
85-86: Remove duplicate exports to avoid confusion
videoandutilsare exported earlier (Lines 50–51 and 49). Duplicate exports are harmless at build but add noise.Apply this diff:
-export * from "./video"; -export * from "./utils";src/routes/api-check/components/drawer/examples/Default.svelte (1)
9-9: Prefer Svelte’son:clickfor event bindingUse the idiomatic
on:clickto attach handlers; this avoids relying on prop forwarding and is consistent with Svelte’s event system.Apply this diff:
- <Button onclick={() => (open1 = true)}>Show drawer</Button> + <Button on:click={() => (open1 = true)}>Show drawer</Button>src/routes/api-check/components/drawer/examples/Right.svelte (1)
15-15: Prefer Svelte’son:clickfor event bindingUse
on:clickto attach handlers instead ofonclickfor idiomatic Svelte and to avoid relying on prop forwarding.Apply this diff:
- <Button onclick={() => (open6 = true)}>Show drawer</Button> + <Button on:click={() => (open6 = true)}>Show drawer</Button>src/routes/api-check/components/drawer/examples/Left.svelte (1)
9-9: Prefer Svelte’son:clickfor event bindingUse
on:clickto attach handlers instead ofonclickfor idiomatic Svelte and to avoid relying on prop forwarding.Apply this diff:
- <Button onclick={() => (open5 = true)}>Show drawer</Button> + <Button on:click={() => (open5 = true)}>Show drawer</Button>src/routes/api-check/components/drawer/examples/Bottom.svelte (1)
7-11: Minor: verify easing choice for UX consistency.You added
easing: sineIn. Many UIs prefersineOutorsineInOutfor more natural settle-in/out motion. Not critical; just ensure this matches the rest of the Drawer examples for a consistent feel.src/routes/api-check/forms/timepicker.svelte (1)
211-215: Close-on-save behavior wired to state; consider Svelte preventDefault modifier.
e.preventDefault()then settingopen1 = falseworks. For brevity and clarity, you could use the Svelte modifier on the form and simplify the handler to avoid dealing with the event.For example:
- Change form:
on:submit|preventDefault={saveAll}- Then define
function saveAll(): void { /* save... */ open1 = false; }src/routes/api-check/components/drawer/examples/DisablingOutside.svelte (1)
5-5: Inline constantoutsidecloseto reduce noiseSince it’s never changed, you can inline it at the usage site.
-let outsideclose = false; -<Drawer {outsideclose} bind:open={open10} id="sidebar10" aria-labelledby="drawer-label"> +<Drawer outsideclose={false} bind:open={open10} id="sidebar10" aria-labelledby="drawer-label">Also applies to: 12-12
src/routes/api-check/components/drawer/examples/Disabled.svelte (1)
6-6: Rename state for clarityConsider a clearer name that reflects non-modal usage. This is an example file, but readability helps.
- let openBackdropFalse = $state(false); + let openNonModal = $state(false);And update usages:
- <Button on:click={() => (openBackdropFalse = true)}>Show drawer</Button> + <Button on:click={() => (openNonModal = true)}>Show drawer</Button>-<Drawer modal={false} bind:open={openBackdropFalse} aria-labelledby="drawer-label"> +<Drawer modal={false} bind:open={openNonModal} aria-labelledby="drawer-label">Also applies to: 10-10, 13-13
src/routes/api-check/components/drawer/examples/Navigation.svelte (1)
4-4: Optional: renameopen2to something meaningfulMinor readability tweak in the example.
- let open2 = $state(false); + let openNavigation = $state(false);- <Button on:click={() => (open2 = true)}>Show navigation</Button> + <Button on:click={() => (openNavigation = true)}>Show navigation</Button>-<Drawer bind:open={open2} aria-labelledby="drawer-nav-label" class="w-64 bg-gray-50 p-0 dark:bg-gray-800"> +<Drawer bind:open={openNavigation} aria-labelledby="drawer-nav-label" class="w-64 bg-gray-50 p-0 dark:bg-gray-800">Also applies to: 9-9, 11-11
src/routes/api-check/components/drawer/examples/OnlyOutside.svelte (1)
5-5: Optional: shorten the state variable nameImproves readability without changing behavior.
- let openDisablingOnlyOutsideClick = $state(false); + let openOutsideDisabled = $state(false);- <Button on:click={() => (openDisablingOnlyOutsideClick = true)}>Show drawer</Button> + <Button on:click={() => (openOutsideDisabled = true)}>Show drawer</Button>-<Drawer outsideclose={false} bind:open={openDisablingOnlyOutsideClick} aria-labelledby="drawer-label"> +<Drawer outsideclose={false} bind:open={openOutsideDisabled} aria-labelledby="drawer-label">Also applies to: 9-9, 12-12
src/lib/dialog/theme.ts (1)
12-20: Remove commented variant scaffolding or implement itThe commented
variants.positionanddefaultVariantsblocks add noise and can mislead future readers. Either implement them or remove the commented code to keep the theme lean.Apply this cleanup:
- variants: { - // position: { - // fixed: { base: "fixed" }, - // absolute: { base: "absolute" } - // }, - }, - defaultVariants: { - // position: "fixed" - } + variants: { + }, + defaultVariants: { + }src/routes/api-check/components/drawer/examples/Scrolling.svelte (1)
16-17: Ensure header CloseButton actually closes the DrawerWith Dialog handling dismissal, this header
CloseButtonmay not close unless it’s wired into the dismissable context or bound to state. If the context isn’t in use here, explicitly toggleopenHiddento false for clarity.Apply this change:
- <CloseButton class="mb-4 dark:text-white" /> + <CloseButton class="mb-4 dark:text-white" onclick={() => (openHidden = false)} />Alternatively, remove it and rely on Dialog’s built-in dismiss button (
dismissable={true}).src/lib/types.ts (1)
915-916: Typo/minor formatting in type literal foronselectpayloadThere’s no space before the index signature; not harmful but reduces readability.
- onselect?: (data: { time: string; endTime: string;[key: string]: string }) => void; + onselect?: (data: { time: string; endTime: string; [key: string]: string }) => void;src/lib/modal/theme.ts (1)
11-13: Slot overrides are minimal and consistent with dialog baseThe simplified
baseandformwork with the extended dialog theme. Note thatbg-white dark:bg-gray-800exists both here and in the dialog base; duplication is harmless but could be trimmed later.If desired, you can rely on dialog’s bg colors and remove them here:
- base: "w-full rounded-lg divide-y text-gray-500 dark:text-gray-400 border-gray-300 dark:border-gray-700 divide-gray-300 dark:divide-gray-700 bg-white dark:bg-gray-800 pointer-events-auto", + base: "w-full rounded-lg divide-y text-gray-500 dark:text-gray-400 border-gray-300 dark:border-gray-700 divide-gray-300 dark:divide-gray-700 pointer-events-auto",src/lib/dialog/Dialog.svelte (1)
94-99: CloseButton renders with type="submit" even when no form is presentWhen
form === false,type="submit"has no effect. Dismissal currently relies on the dismissable context viacreateDismissableContext(close_handler), which is okay, but the type attribute is misleading. Consider settingtype="button"when not in form mode.- <CloseButton type="submit" formnovalidate class={closeCls({ class: clsx(classes?.close) })} /> + <CloseButton + type={form ? "submit" : "button"} + formnovalidate + class={closeCls({ class: clsx(classes?.close) })} + />src/lib/modal/Modal.svelte (1)
29-29: Use type="button" for the header CloseButton to avoid accidental form submissions.This avoids submitting a nested form if present in modal content.
Apply this diff:
- <CloseButton type="submit" formnovalidate class={clsx(styling.close)} /> + <CloseButton type="button" class={clsx(styling.close)} />src/routes/docs/components/drawer.md (3)
389-389: Grammar: “its” not “it’s”.[Suggest fix] “Use the drawer’s backdrop:xxx class to manage its look.”
Apply this diff:
-Use the drawer's `backdrop:xxx` class to manage it's look. +Use the drawer's `backdrop:xxx` class to manage its look.
392-392: Grammar and clarity in Non-modal note.Correct typos and phrasing.
Apply this diff:
-Drawer by default is `modal` (see `dialog`). You can set prop `modal={false}` to open `Drawer` in non-modal mode, that means with no backdrop. Note howerver, that you will need managed the `Drawer` position, z-index and `Esc` button manually. +Drawer is `modal` by default (see `Dialog`). Set `modal={false}` to open the `Drawer` in non-modal mode (no backdrop). Note, however, that you will need to manage the Drawer’s position, z-index, and Esc key handling manually.
20-24: Minor: imported items in examples not used.CloseButton and sineIn are imported but not used in several blocks. Consider removing them from the examples to reduce noise.
Also applies to: 33-37
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
src/lib/dialog/Dialog.svelte(1 hunks)src/lib/dialog/index.ts(1 hunks)src/lib/dialog/theme.ts(1 hunks)src/lib/drawer/Drawer.svelte(2 hunks)src/lib/drawer/theme.ts(1 hunks)src/lib/index.ts(1 hunks)src/lib/modal/Modal.svelte(3 hunks)src/lib/modal/theme.ts(2 hunks)src/lib/types.ts(9 hunks)src/routes/admin-dashboard/(sidebar)/crud/products/+page.svelte(2 hunks)src/routes/api-check/components/drawer/examples/Bottom.svelte(2 hunks)src/routes/api-check/components/drawer/examples/Contact.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Default.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Disabled.svelte(1 hunks)src/routes/api-check/components/drawer/examples/DisablingOutside.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Enabled.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Form.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Left.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Navigation.svelte(1 hunks)src/routes/api-check/components/drawer/examples/OnlyOutside.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Position.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Right.svelte(2 hunks)src/routes/api-check/components/drawer/examples/Scrolling.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Top.svelte(2 hunks)src/routes/api-check/forms/timepicker.svelte(4 hunks)src/routes/blocks/application/crud-create-drawers.md(2 hunks)src/routes/blocks/application/crud-read-drawers.md(1 hunks)src/routes/blocks/application/crud-update-drawers.md(1 hunks)src/routes/blocks/application/faceted-search-drawers.md(1 hunks)src/routes/builder/drawer/+page.svelte(3 hunks)src/routes/builder/modal/+page.svelte(1 hunks)src/routes/docs/components/drawer.md(15 hunks)src/routes/docs/forms/timepicker.md(4 hunks)src/routes/examples/drawer/classesCheck.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-18T09:38:03.879Z
Learnt from: Chizaruu
PR: themesberg/flowbite-svelte#1465
File: src/lib/forms/Timepicker.svelte:105-116
Timestamp: 2024-10-18T09:38:03.879Z
Learning: In `Timepicker.svelte`, the `Dropdown` opens as intended and is used exclusively with the 'dropdown' and 'timerange-dropdown' Timepicker types.
Applied to files:
src/routes/docs/forms/timepicker.mdsrc/routes/api-check/forms/timepicker.svelte
🧬 Code Graph Analysis (3)
src/lib/modal/theme.ts (4)
src/lib/modal/index.ts (1)
modal(2-2)src/lib/theme/themeUtils.ts (1)
Classes(14-16)src/lib/dialog/index.ts (1)
dialog(2-2)src/lib/dialog/theme.ts (1)
dialog(6-21)
src/lib/types.ts (6)
src/lib/drawer/theme.ts (1)
DrawerVariants(5-5)src/lib/forms/helper/theme.ts (1)
HelperVariants(4-4)src/lib/modal/theme.ts (1)
ModalVariants(6-6)src/lib/dialog/theme.ts (1)
DialogVariants(4-4)src/lib/toolbar/theme.ts (1)
ToolbarGroupVariants(6-6)src/lib/skeleton/theme.ts (2)
TestimonialPlaceholderVariants(9-9)WidgetPlaceholderVariants(12-12)
src/lib/drawer/theme.ts (3)
src/lib/theme/themeUtils.ts (1)
Classes(14-16)src/lib/dialog/index.ts (1)
dialog(2-2)src/lib/dialog/theme.ts (1)
dialog(6-21)
🪛 ESLint
src/lib/dialog/Dialog.svelte
[error] 74-74: Expected an assignment or function call and instead saw an expression.
(@typescript-eslint/no-unused-expressions)
🪛 LanguageTool
src/routes/docs/components/drawer.md
[grammar] ~392-~392: Ensure spelling is correct
Context: ...mode, that means with no backdrop. Note howerver, that you will need managed the `Drawer...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.17.2)
src/routes/docs/components/drawer.md
458-458: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
🔇 Additional comments (44)
src/routes/blocks/application/crud-update-drawers.md (1)
24-24: Switch to bind:open is consistent and clearUsing an
openboolean andbind:openaligns with the new Dialog-backed Drawer API and simplifies state handling.Also applies to: 27-27, 33-36
src/routes/docs/forms/timepicker.md (1)
419-419: bind:open migration and control flow look goodThe Drawer visibility is driven by
openand properly toggled on open, cancel, and save. This matches the new Dialog model.Also applies to: 494-496, 501-501, 552-553
src/routes/blocks/application/crud-read-drawers.md (1)
24-24: bind:open migration LGTMVisibility is now controlled by
openand usage in the trigger and Drawer matches the new API.Also applies to: 29-29, 32-36
src/routes/blocks/application/crud-create-drawers.md (1)
24-24: bind:open migration looks correct
openstate, trigger, and Drawer binding reflect the new API and behavior.Also applies to: 34-35, 40-41, 42-44
src/lib/index.ts (1)
22-22: Publicly exporting Dialog is appropriateExposing
./dialogat the root aligns the API surface with Drawer/Modal’s new foundation.src/routes/api-check/components/drawer/examples/Default.svelte (2)
5-5: State rename toopen1aligns with new Drawer APIUsing
$state(false)andopen1matches the newbind:opencontract for Drawer.
12-12: Update remaining bind:hidden occurrences in documentationA repo-wide search uncovered several
bind:hiddenusages in the static documentation that need to be updated tobind:opento stay in sync with the new Drawer API:• static/llm/forms/timepicker.md:490
• static/llm/components/drawer.md:31, 64, 142, 188, 250, 292, 332, 369, 407, 441, 475, 510, 538, 573Please update each example to use:
- <Drawer bind:hidden={…} …> + <Drawer bind:open={…} …>and adjust any associated variable names if necessary.
Likely an incorrect or invalid review comment.
src/lib/dialog/index.ts (1)
1-2: Public exports for Dialog look goodNamed re-export of the Svelte component and theme is clear and tree-shakable.
src/routes/blocks/application/faceted-search-drawers.md (2)
18-18: State rename toopenaligns with new Drawer APIThe boolean
openstate reflects the newbind:opencontract and is initialized correctly.
23-23: LGTM on trigger updateSwitching the trigger to set
open = truematches the new state semantics.src/routes/api-check/components/drawer/examples/Right.svelte (1)
6-6: State rename toopen6aligns with new Drawer APIUsing
$state(false)andopen6matches thebind:opencontract.src/routes/api-check/components/drawer/examples/Left.svelte (1)
5-5: State rename toopen5aligns with new Drawer APIUsing
$state(false)andopen5matches thebind:opencontract.src/routes/admin-dashboard/(sidebar)/crud/products/+page.svelte (2)
11-11: Correct migration to open-based state.Switching to
let open: boolean = $state(false)aligns with the Dialog-based Drawer API and sets a clear closed-by-default state.
98-100: bind:open usage is correct; consider explicit modal intent.The shorthand
bind:opencorrectly binds to the localopenvariable, and passing it toDrawerComponentenables children to close the Drawer. If this Drawer is intended to be modal (backdrop, focus-trap), consider setting the explicit prop if the new API doesn’t default to modal in your theme, to avoid ambiguity in examples.Do you want me to scan the new Drawer API to confirm the default modal behavior and update this instance to be explicit for docs consistency?
src/routes/api-check/components/drawer/examples/Bottom.svelte (3)
6-6: State migration to open looks good.
open8defaults tofalseand is used consistently withbind:open.
15-15: Open handler is correct.Directly setting
open8 = trueis clear and avoids ambiguous toggling.
18-18: bind:open and transitionParams wiring is correct.
placement="bottom"withtransitionParams={transitionParamsBottom}andbind:open={open8}conforms to the new API and keeps the example focused.src/routes/api-check/components/drawer/examples/Contact.svelte (2)
5-5: Good migration to open-based state.
open3 = $state(false)follows the new Drawer/Dialog model.
9-12: Event and binding changes look correct.Opening with
open3 = trueand switching tobind:open={open3}reflect the new API without altering behavior.src/routes/api-check/forms/timepicker.svelte (4)
148-148: State initialization for Drawer is correct.
open1 = $state(false)matches the new open-binding convention.
423-423: Open handler aligns with the new API.Using
open1 = trueavoids toggle edge cases and keeps the example deterministic.
429-429: bind:open is correctly applied to Drawer.Binding
open1ensures close buttons and outside interactions can update the parent state through the Dialog-based Drawer.
481-481: Cancel closes the Drawer as expected.Explicitly closing with
open1 = falseis clear. UX-wise, this aligns with the Save handler’s close behavior.src/routes/api-check/components/drawer/examples/Enabled.svelte (2)
5-5: State variable naming reflects the example intent.
openBackdropTruereads clearly in the “Enabled” example context.
9-12: Open binding migrated correctly; consider making modal intent explicit.Since this example likely demonstrates “backdrop enabled,” consider setting the explicit prop (e.g.,
modal={true}) if that is now the public surface, to avoid relying on defaults in documentation examples.Would you like me to sweep the new Drawer API and examples to ensure all “backdrop-enabled” demos explicitly use
modal={true}for clarity?src/routes/examples/drawer/classesCheck.svelte (1)
4-9: Migration to bind:open is correct and consistentState initialization and the trigger button correctly reflect the new open-based API.
src/routes/api-check/components/drawer/examples/Form.svelte (1)
5-11: Migration to bind:open looks goodState and binding updates are correct and match the new Dialog-based API.
src/routes/api-check/components/drawer/examples/Top.svelte (1)
6-6: Open-binding migration is correctThe state, trigger, and Drawer binding are updated correctly and align with the new API.
Also applies to: 15-15, 18-18
src/routes/builder/modal/+page.svelte (1)
112-112: Prop rename to transitionParams is correctThis aligns the live example with the new Dialog-based API.
Note: The code generator below still emits params= and {modalStatus} {closeModal}. See next comments for fixes.
src/lib/drawer/theme.ts (1)
8-11: LGTM: theme extends dialog and simplifies base/stylesExtending the dialog theme and simplifying the base slot looks consistent with the new Dialog-based architecture.
src/lib/dialog/theme.ts (1)
6-11: Solid initial theme token for Dialog; slots and base classes look consistentThe base/form/close slots cover the essentials for dialog and interop with Modal/Drawer. The
::backdropandopen:variants align with native semantics. No issues here.src/lib/types.ts (7)
90-91: Correct import path and type usage for DialogVariantsImporting
DialogVariantsfrom./dialog/themekeeps types local and consistent across Modal/Drawer. Looks good.
556-558: Drawer inherits DialogProps cleanly;bodyScrollingisolated here is appropriateExtending
DialogPropsensures Drawer stays in sync with Dialog capabilities while preserving Drawer-specific props viabodyScrolling. Good separation.
1032-1040: Avoid classes duplication in Modal by omitting from DialogPropsUsing
Omit<DialogProps, "classes">is the right move sinceModalVariantsalready includesClasses<typeof dialog>. Prevents structural type conflicts and makesclasses.closeavailable through the extended dialog theme.
721-721: Style-only change is fineWhitespace-only change; no impact.
1117-1117: Style-only change is fineWhitespace-only change; no impact.
1426-1426: Style-only change is fineWhitespace-only change; no impact.
1436-1436: Style-only change is fineWhitespace-only change; no impact.
src/lib/modal/theme.ts (2)
3-3: Extending modal from dialog centralizes shared concerns
extend: dialogis the right direction; it de-duplicates styling and keeps thecloseslot available viaClasses<typeof dialog>. Good call.
24-32: Placement classes applied correctly, no overrides detectedWe’ve confirmed that in
src/lib/modal/Modal.sveltetheplacementprop feeds directly intomodalStyle, andsrc/lib/modal/theme.tsonly uses the fixed inset container plus the auto-margin utilities for each of the nine placements. No other positioning rules (absolute,inset-*, overriding margins) were found in the Modal files.•
Modal.sveltelines 9 & 20:placementis passed intomodalStyle
•theme.tslines 24–32: onlyfixed inset-0plusmt|mb|mx|mymargin classesPlease proceed with a visual smoke test of all nine placement options at various breakpoints to catch any cross-browser quirks.
src/routes/builder/drawer/+page.svelte (3)
137-141: Migration to bind:open is correct and consistent.The button now opens via open = true and Drawer binds to open. Drawerhead closes via open = false. This aligns with the new Dialog-based API.
147-147: outsideclose status display looks good.The label reflects the current outsideclose setting, which matches the new API semantics.
163-163: Good UX control for outsideclose.Toggling outsideclose via this button is a clear demonstration of the new prop’s effect.
src/lib/drawer/Drawer.svelte (1)
32-34: LGTM on Dialog integration and class theming.Forwarding rest props to Dialog while resolving theme classes keeps Drawer lean and consistent with the new architecture.
| placement: { | ||
| left: { base: "inset-y-0 start-0" }, | ||
| right: { base: "inset-y-0 end-0" }, | ||
| top: { base: "inset-x-0 top-0" }, | ||
| bottom: { base: "inset-x-0 bottom-0" } | ||
| left: { base: "me-auto h-full" }, | ||
| right: { base: "ms-auto h-full" }, | ||
| top: { base: "mb-auto w-full" }, | ||
| bottom: { base: "mt-auto w-full" } | ||
| }, | ||
| width: { | ||
| default: { base: "w-80" }, | ||
| full: { base: "w-full" }, | ||
| half: { base: "w-1/2" } | ||
| }, |
There was a problem hiding this comment.
Width vs. placement conflict for top/bottom drawers
For placement="top"/"bottom" you add w-full, but the width variant defaults to w-80. Depending on class order, w-80 can override w-full, producing incorrect widths for top/bottom drawers.
Consider enforcing w-full for top/bottom via compoundVariants so it reliably wins:
export const drawer = tv({
extend: dialog,
slots: {
base: "p-4 max-h-screen max-w-screen",
},
variants: {
- placement: {
+ placement: {
left: { base: "me-auto h-full" },
right: { base: "ms-auto h-full" },
top: { base: "mb-auto w-full" },
bottom: { base: "mt-auto w-full" }
},
width: {
default: { base: "w-80" },
full: { base: "w-full" },
half: { base: "w-1/2" }
},
},
+ compoundVariants: [
+ // Ensure top/bottom drawers are always full width regardless of width variant
+ { placement: ["top", "bottom"], class: { base: "!w-full" } },
+ ],
defaultVariants: {
placement: "left",
width: "default"
}
});Alternatively, reorder the variants so placement classes are appended after width, but compoundVariants is more robust.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| placement: { | |
| left: { base: "inset-y-0 start-0" }, | |
| right: { base: "inset-y-0 end-0" }, | |
| top: { base: "inset-x-0 top-0" }, | |
| bottom: { base: "inset-x-0 bottom-0" } | |
| left: { base: "me-auto h-full" }, | |
| right: { base: "ms-auto h-full" }, | |
| top: { base: "mb-auto w-full" }, | |
| bottom: { base: "mt-auto w-full" } | |
| }, | |
| width: { | |
| default: { base: "w-80" }, | |
| full: { base: "w-full" }, | |
| half: { base: "w-1/2" } | |
| }, | |
| export const drawer = tv({ | |
| extend: dialog, | |
| slots: { | |
| base: "p-4 max-h-screen max-w-screen", | |
| }, | |
| variants: { | |
| placement: { | |
| left: { base: "me-auto h-full" }, | |
| right: { base: "ms-auto h-full" }, | |
| top: { base: "mb-auto w-full" }, | |
| bottom: { base: "mt-auto w-full" } | |
| }, | |
| width: { | |
| default: { base: "w-80" }, | |
| full: { base: "w-full" }, | |
| half: { base: "w-1/2" } | |
| }, | |
| }, | |
| compoundVariants: [ | |
| // Ensure top/bottom drawers are always full width regardless of width variant | |
| { placement: ["top", "bottom"], class: { base: "!w-full" } }, | |
| ], | |
| defaultVariants: { | |
| placement: "left", | |
| width: "default" | |
| } | |
| }); |
🤖 Prompt for AI Agents
In src/lib/drawer/theme.ts around lines 13 to 23, the placement variant sets
w-full for top/bottom but the width variant can later append w-80 and override
it; add compoundVariants that match placement: "top" and placement: "bottom"
(for any width variant, or explicitly for default/half/full) and set the
computed class to enforce w-full for those placements so the top/bottom drawers
always use full width; alternatively (if you prefer) reorder the variants so
placement classes are appended after width, but the recommended fix is to add
explicit compoundVariants forcing w-full for placement "top" and "bottom".
src/routes/docs/forms/timepicker.md
Outdated
| </div> | ||
|
|
||
| <Drawer bind:hidden class="w-96 bg-gray-50 p-6 dark:bg-gray-800" id="drawer-timepicker"> | ||
| <Drawer bind:open class="w-96 bg-gray-50 p-6 dark:bg-gray-800" id="drawer-timepicker"> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Wire CloseButton to actually close the Drawer and add accessible naming
CloseButton currently doesn’t modify open; wire it to close. Also set aria-labelledby to connect the dialog to its title.
Apply this diff:
-<Drawer bind:open class="w-96 bg-gray-50 p-6 dark:bg-gray-800" id="drawer-timepicker">
+<Drawer bind:open class="w-96 bg-gray-50 p-6 dark:bg-gray-800" id="drawer-timepicker" aria-labelledby="drawer-label">
@@
- <CloseButton class="mb-4 dark:text-white" />
+ <CloseButton class="mb-4 dark:text-white" onclick={() => (open = false)} aria-label="Close time schedule drawer" />To ensure consistent accessibility across docs, you can scan for Drawers missing aria-labelledby:
Also applies to: 503-507
🏁 Script executed:
#!/bin/bash
# List Drawer usages that bind `open` but lack an aria-labelledby attribute
rg -nP '<Drawer(?![^>]*aria-labelledby)[^>]*bind:open'Length of output: 4047
Ensure Drawer components include accessible naming and a working close button
Wire each CloseButton to set its open binding to false and add aria-labelledby on the corresponding <Drawer>.
In src/routes/docs/forms/timepicker.md around line 501, apply:
-<Drawer bind:open class="w-96 bg-gray-50 p-6 dark:bg-gray-800" id="drawer-timepicker">
+<Drawer
+ bind:open
+ class="w-96 bg-gray-50 p-6 dark:bg-gray-800"
+ id="drawer-timepicker"
+ aria-labelledby="drawer-label"
+>
@@
- <CloseButton class="mb-4 dark:text-white" />
+ <CloseButton
+ class="mb-4 dark:text-white"
+ on:click={() => (open = false)}
+ aria-label="Close time schedule drawer"
+ />Other instances missing aria-labelledby and a working close handler:
- src/routes/admin-dashboard/(sidebar)/crud/products/+page.svelte:98–99
- src/routes/builder/drawer/+page.svelte:139
- src/routes/api-check/forms/timepicker.svelte:429
- src/routes/api-check/components/drawer/examples/Bottom.svelte:18
- src/routes/api-check/components/drawer/examples/Contact.svelte:11
- src/routes/api-check/components/drawer/examples/Navigation.svelte:11
- src/routes/api-check/components/drawer/examples/Scrolling.svelte:11
- src/routes/api-check/components/drawer/examples/Top.svelte:18
- src/routes/api-check/components/drawer/examples/Right.svelte:18
- src/routes/api-check/components/drawer/examples/OnlyOutside.svelte:12
- src/routes/api-check/components/drawer/examples/Enabled.svelte:12
- src/routes/api-check/components/drawer/examples/Form.svelte:11
- src/routes/api-check/components/drawer/examples/Left.svelte:12
- src/routes/api-check/components/drawer/examples/Position.svelte:12
- src/routes/api-check/components/drawer/examples/Disabled.svelte:13
- src/routes/api-check/components/drawer/examples/DisablingOutside.svelte:12
- src/routes/docs/components/drawer.md:44, 75, 151, 195, 258, 298, 336, 371, 408, 440, 476
- src/routes/blocks/application/crud-read-drawers.md:32
- src/routes/blocks/application/faceted-search-drawers.md:25
- src/routes/blocks/application/crud-update-drawers.md:35
- src/routes/blocks/application/crud-create-drawers.md:42
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Drawer bind:open class="w-96 bg-gray-50 p-6 dark:bg-gray-800" id="drawer-timepicker"> | |
| <Drawer | |
| bind:open | |
| class="w-96 bg-gray-50 p-6 dark:bg-gray-800" | |
| id="drawer-timepicker" | |
| aria-labelledby="drawer-label" | |
| > | |
| <CloseButton | |
| class="mb-4 dark:text-white" | |
| on:click={() => (open = false)} | |
| aria-label="Close time schedule drawer" | |
| /> | |
| <!-- …rest of drawer content… --> |
🤖 Prompt for AI Agents
In src/routes/docs/forms/timepicker.md around line 501, the <Drawer
id="drawer-timepicker" ...> lacks an accessible label and the CloseButton isn't
wired to close it; add aria-labelledby="drawer-timepicker-title" on the Drawer,
add a heading or element inside the drawer with id="drawer-timepicker-title" to
serve as the label, and wire the CloseButton to set the drawer's open binding to
false (e.g., call the handler or set the bound variable to false) so clicking
CloseButton closes the drawer; apply the same pattern to the other listed
files/locations.
|
|
||
| <div class="text-center"> | ||
| <Button onclick={() => (hidden1 = false)}>Show drawer</Button> | ||
| <Button onclick={() => (open1 = true)}>Show drawer</Button> |
There was a problem hiding this comment.
Fix ARIA: aria-labelledby should reference the heading; aria-controls belongs on the trigger (not the dialog)
The dialog should be labelled by the header (id="drawer-label"). aria-controls is typically used on the control element (button), not on the dialog itself.
Apply this diff:
- <Button onclick={() => (open1 = true)}>Show drawer</Button>
+ <Button aria-controls="sidebar1" onclick={() => (open1 = true)}>Show drawer</Button>-<Drawer bind:open={open1} id="sidebar1" aria-controls="sidebar1" aria-labelledby="sidebar1" classes={{ backdrop: "bg-red-700 opacity-50" }}>
+<Drawer bind:open={open1} id="sidebar1" aria-labelledby="drawer-label" classes={{ backdrop: "bg-red-700 opacity-50" }}>Also applies to: 11-11
🤖 Prompt for AI Agents
In src/routes/examples/drawer/classesCheck.svelte around lines 8 and 11, the
dialog currently uses aria-controls incorrectly and the heading isn't referenced
by aria-labelledby; update the markup so the dialog element has
aria-labelledby="drawer-label" (ensure the dialog has an id, e.g., id="drawer")
and the heading element has id="drawer-label", and move aria-controls to the
trigger Button (e.g., aria-controls="drawer") instead of placing it on the
dialog; remove any aria-controls on the dialog itself.
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/routes/examples/drawer/classesCheck.svelte (1)
16-16: Wire CloseButton to actually close the Drawer and add an accessible labelCurrently it doesn't change open.
- <CloseButton class="mb-4 dark:text-white" /> + <CloseButton class="mb-4 dark:text-white" on:click={() => (open1 = false)} aria-label="Close drawer" />
♻️ Duplicate comments (4)
src/routes/examples/drawer/classesCheck.svelte (1)
8-8: Add aria-controls on the trigger (Button), not on the dialogThis links the trigger to the Drawer element and improves a11y.
- <Button onclick={() => (open1 = true)}>Show drawer</Button> + <Button aria-controls="sidebar1" onclick={() => (open1 = true)}>Show drawer</Button>src/lib/types.ts (1)
557-568: title prop is missing in DialogProps; it gets swallowed if destructured in Dialog.svelteEither stop destructuring title in Dialog.svelte (preferred), or add it to DialogProps to forward to the underlying element.
Preferred fix (no API expansion): remove title from the destructuring list in Dialog.svelte so it passes through via {...restProps}.
Alternative (if you want to support it explicitly), apply:
export interface DialogProps extends DialogVariants, HTMLDialogAttributes { onaction?: ({ action, data }: { action: string; data: FormData }) => any; form?: boolean; modal?: boolean; autoclose?: boolean; focustrap?: boolean; permanent?: boolean; dismissable?: boolean; outsideclose?: boolean; transition?: TransitionFunc; transitionParams?: ParamsType; + title?: string; }To verify whether Dialog.svelte currently destructures title, search:
#!/bin/bash rg -nA2 -i $'component|script|let\\s*{[^}]*title' src/lib/dialog/Dialog.sveltesrc/routes/docs/components/drawer.md (2)
371-381: Remove non-existent Drawer prop “form”Drawer doesn’t accept a form prop; the attribute is inert and misleading.
-<Drawer form placement="bottom" class="w-full" transitionParams={transitionParamsBottom} bind:open={open8}> +<Drawer placement="bottom" class="w-full" transitionParams={transitionParamsBottom} bind:open={open8}>
455-497: Remove the “Body scrolling” section (outdated and includes an internal note)This documents a prop that isn’t wired and includes an internal comment. Recommend removing the entire section until behavior is decided and implemented.
-## Body scrolling - -### This section is useless. We don't need it. Confirm. - -By default, body scrolling is disabled when the drawer is visible, however, you can choose to enable it using the `bodyScrolling={true|false}` prop. - -**Limitation:** When bodyScrolling={true}, clicking the backdrop to close the drawer is disabled. Use the Escape key or provide a close button inside the drawer content instead. - -```svelte example -<script> - import { Drawer, CardPlaceholder, Button, CloseButton } from "flowbite-svelte"; - import { InfoCircleSolid, ArrowRightOutline } from "flowbite-svelte-icons"; - let drawerOpen = $state(false); -</script> - -<div class="text-center"> - <Button onclick={() => (drawerOpen = true)}>Show drawer</Button> - <CardPlaceholder size="2xl" class="mt-6"/> -</div> - -<Drawer bind:open={drawerOpen} bodyScrolling={true}> - <h5 class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400"> - <InfoCircleSolid class="me-2.5 h-5 w-5" />Info - </h5> - <p class="mb-6 text-sm text-gray-500 dark:text-gray-400"> - Supercharge your hiring by taking advantage of our <a href="/" class="text-primary-600 dark:text-primary-500 underline hover:no-underline">limited-time sale</a> - for Flowbite Docs + Job Board. Unlimited access to over 190K top-ranked candidates and the #1 design job board. Lorem ipsum dolor sit, amet consectetur adipisicing elit. At vel quidem sed numquam repellendus. Nihil earum veritatis quas non quisquam nisi hic ad, quo ullam iusto aliquid architecto saepe asperiores. Lorem, ipsum dolor sit amet consectetur adipisicing elit. Amet ratione vero repudiandae ex? Possimus doloremque culpa excepturi, corporis natus hic perferendis dolorem dignissimos fugit nostrum ipsam sapiente voluptate eos ea! Lorem ipsum dolor sit, amet - consectetur adipisicing elit. At vel quidem sed numquam repellendus. Nihil earum veritatis quas non quisquam nisi hic ad, quo ullam iusto aliquid architecto saepe asperiores. Lorem, ipsum dolor sit amet consectetur adipisicing elit. Amet ratione vero repudiandae ex? Possimus doloremque culpa excepturi, corporis natus hic perferendis dolorem dignissimos fugit nostrum ipsam sapiente voluptate eos ea! Lorem ipsum dolor sit, amet consectetur adipisicing elit. At vel quidem sed numquam repellendus. Nihil earum veritatis quas non quisquam nisi hic ad, quo ullam iusto - aliquid architecto saepe asperiores. Lorem, ipsum dolor sit amet consectetur adipisicing elit. Amet ratione vero repudiandae ex? Possimus doloremque culpa excepturi, corporis natus hic perferendis dolorem dignissimos fugit nostrum ipsam sapiente voluptate eos ea! - </p> - <p class="mb-6 text-sm text-gray-500 dark:text-gray-400"> - Supercharge your hiring by taking advantage of our <a href="/" class="text-primary-600 dark:text-primary-500 underline hover:no-underline">limited-time sale</a> - for Flowbite Docs + Job Board. Unlimited access to over 190K top-ranked candidates and the #1 design job board. Lorem ipsum dolor sit, amet consectetur adipisicing elit. At vel quidem sed numquam repellendus. Nihil earum veritatis quas non quisquam nisi hic ad, quo ullam iusto aliquid architecto saepe asperiores. Lorem, ipsum dolor sit amet consectetur adipisicing elit. Amet ratione vero repudiandae ex? Possimus doloremque culpa excepturi, corporis natus hic perferendis dolorem dignissimos fugit nostrum ipsam sapiente voluptate eos ea! Lorem ipsum dolor sit, amet consectetur adipisicing elit. At vel quidem sed numquam repellendus. Nihil earum veritatis quas non quisquam nisi hic ad, quo ullam iusto aliquid architecto saepe asperiores. Lorem, ipsum dolor sit amet consectetur adipisicing elit. Amet ratione vero repudiandae ex? Possimus doloremque culpa excepturi, corporis natus hic perferendis dolorem dignissimos fugit nostrum ipsam sapiente voluptate eos ea! Lorem ipsum dolor sit, amet consectetur adipisicing elit. At vel quidem sed numquam repellendus. Nihil earum veritatis quas non quisquam nisi hic ad, quo ullam iusto - aliquid architecto saepe asperiores. Lorem, ipsum dolor sit amet consectetur adipisicing elit. Amet ratione vero repudiandae ex? Possimus doloremque culpa excepturi, corporis natus hic perferendis dolorem dignissimos fugit nostrum ipsam sapiente voluptate eos ea! - </p> - <div class="grid grid-cols-2 gap-4"> - <Button color="light" href="/">Learn more</Button> - <Button href="/" class="px-4">Get access <ArrowRightOutline class="ms-2 h-5 w-5" /></Button> - </div> -</Drawer> -```Optionally, once behavior is decided and implemented, re-introduce a corrected section.
🧹 Nitpick comments (4)
src/routes/docs/components/drawer.md (2)
388-390: Grammar: “its” (possessive), not “it's”Minor doc nit.
-Use the drawer's `backdrop:xxx` class to manage it's look. +Use the drawer’s `backdrop:xxx` class to manage its look.
392-392: Clarify and fix typos in the Non-modal descriptionCorrects “howerver” and improves phrasing.
-Drawer by default is `modal` (see `dialog`). You can set prop `modal={false}` to open `Drawer` in non-modal mode, that means with no backdrop. Note howerver, that you will need managed the `Drawer` position, z-index and `Esc` button manually. +Drawer is modal by default (see Dialog). You can set `modal={false}` to use a non-modal Drawer (no backdrop). However, you will need to manage the Drawer’s position, stacking context (z-index), and Escape key handling manually.src/lib/drawer/Drawer.svelte (2)
36-56: Update the component JSDoc to reflect the new APIComment still lists legacy props (hidden, backdrop, etc.). Update to the open-based API and Dialog-facing props.
-@prop hidden = $bindable() -@prop activateClickOutside = true -@prop position +@prop open = $bindable(false) @prop width -@prop backdrop = true -@prop backdropClass @prop placement = "left" @prop class: className @prop classes -@prop transitionParams -@prop transitionType = fly -@prop bodyScrolling = false +@prop transition = fly +@prop transitionParams @prop ...restProps
21-26: Optional: simplify transition offsets to consistent constantsUsing innerWidth/innerHeight yields very large offsets on some screens. Consider symmetric constants for a smoother animation, matching the docs’ 320px examples.
- let x = $derived(placement === "left" ? -320 : placement === "right" ? innerWidth + 320 : undefined); - let y = $derived(placement === "top" ? -100 : placement === "bottom" ? innerHeight + 100 : undefined); + let x = $derived(placement === "left" ? -320 : placement === "right" ? 320 : undefined); + let y = $derived(placement === "top" ? -320 : placement === "bottom" ? 320 : undefined);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/lib/dialog/Dialog.svelte(1 hunks)src/lib/drawer/Drawer.svelte(2 hunks)src/lib/types.ts(8 hunks)src/routes/docs/components/drawer.md(15 hunks)src/routes/docs/forms/timepicker.md(4 hunks)src/routes/examples/drawer/classesCheck.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/dialog/Dialog.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-18T09:38:03.879Z
Learnt from: Chizaruu
PR: themesberg/flowbite-svelte#1465
File: src/lib/forms/Timepicker.svelte:105-116
Timestamp: 2024-10-18T09:38:03.879Z
Learning: In `Timepicker.svelte`, the `Dropdown` opens as intended and is used exclusively with the 'dropdown' and 'timerange-dropdown' Timepicker types.
Applied to files:
src/routes/docs/forms/timepicker.md
📚 Learning: 2024-11-12T10:36:34.807Z
Learnt from: LahTeuto
PR: themesberg/flowbite-svelte#1484
File: src/lib/navbar/NavLi.svelte:34-34
Timestamp: 2024-11-12T10:36:34.807Z
Learning: When enhancing ARIA roles in Svelte components, avoid setting `role="presentation"` on elements that should retain the `link` role, as it can diminish ARIA role coverage and negatively affect accessibility.
Applied to files:
src/routes/examples/drawer/classesCheck.svelte
🧬 Code Graph Analysis (1)
src/lib/types.ts (6)
src/lib/dialog/theme.ts (1)
DialogVariants(4-4)src/lib/drawer/theme.ts (1)
DrawerVariants(5-5)src/lib/forms/helper/theme.ts (1)
HelperVariants(4-4)src/lib/modal/theme.ts (1)
ModalVariants(6-6)src/lib/toolbar/theme.ts (1)
ToolbarGroupVariants(6-6)src/lib/skeleton/theme.ts (2)
TestimonialPlaceholderVariants(9-9)WidgetPlaceholderVariants(12-12)
🪛 LanguageTool
src/routes/docs/components/drawer.md
[grammar] ~392-~392: Ensure spelling is correct
Context: ...mode, that means with no backdrop. Note howerver, that you will need managed the `Drawer...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (2)
src/routes/docs/forms/timepicker.md (1)
494-499: Visibility flow LGTMTrigger opens the Drawer and Cancel button closes it using the new bind:open API. Clean and consistent.
Also applies to: 549-551
src/lib/drawer/Drawer.svelte (1)
27-28: Honoring transitionParams: LGTMThe component now prefers consumer-supplied transitionParams and falls back to sensible defaults.
src/lib/drawer/Drawer.svelte
Outdated
|
|
||
| warnThemeDeprecation("Drawer", { backdropClass }, { backdropClass: "backdrop" }); | ||
| const styling = $derived(classes ?? { backdrop: backdropClass }); | ||
| let { children, open = $bindable(false), width, placement = "left", class: className, classes, transitionParams, transition = fly, bodyScrolling = false, ...restProps }: DrawerProps = $props(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Remove unused bodyScrolling prop (or implement it), and align docs
bodyScrolling is destructured but never used; this is confusing and the docs’ “Body scrolling” section is slated for removal. Either implement the behavior or remove the prop from Drawer and types.
Proposed removal in this component:
-let { children, open = $bindable(false), width, placement = "left", class: className, classes, transitionParams, transition = fly, bodyScrolling = false, ...restProps }: DrawerProps = $props();
+let { children, open = $bindable(false), width, placement = "left", class: className, classes, transitionParams, transition = fly, ...restProps }: DrawerProps = $props();And in src/lib/types.ts (outside this range), remove the unused prop:
export interface DrawerProps extends DrawerVariants, DialogProps {
- bodyScrolling?: boolean;
}To verify there are no remaining references:
🏁 Script executed:
#!/bin/bash
rg -n "bodyScrolling"Length of output: 1662
Remove unused bodyScrolling prop (or implement it) and sync docs/examples
The bodyScrolling prop is currently destructured in Drawer.svelte and annotated with @prop, but its value is never used. You should either implement the intended “disable body scroll when open” behavior or remove the prop entirely—including all type definitions, docs, component metadata, and examples—to avoid confusion.
If you opt for removal, please update the following:
• src/lib/drawer/Drawer.svelte
– Remove bodyScrolling = false from the prop destructuring on line 10.
– Remove the @prop bodyScrolling = false annotation on line 54.
• src/lib/types.ts
– Delete bodyScrolling?: boolean; from the DrawerProps interface.
• src/routes/component-data/Drawer.json
– Remove the ["bodyScrolling", "false"] entry.
• Documentation (src/routes/docs/components/drawer.md)
– Delete the entire “Body scrolling” section (lines ~460–470) and any <Drawer bodyScrolling={…}> examples.
– Remove the limitation note about backdrop clicks being disabled.
• Static docs (static/llm/components/drawer.md)
– Mirror the removals made in the live docs.
• Examples (src/routes/api-check/components/drawer/examples/Scrolling.svelte)
– Remove bodyScrolling={true} from the <Drawer> usage.
Once all references are cleaned up (or the feature is fully implemented), rerun:
rg -n bodyScrollingto confirm no stray mentions remain.
🤖 Prompt for AI Agents
In src/lib/drawer/Drawer.svelte around line 10 the prop bodyScrolling is
destructured but unused; remove bodyScrolling = false from the prop list and
delete the @prop bodyScrolling = false annotation (line ~54); then remove
bodyScrolling?: boolean; from src/lib/types.ts, delete the
["bodyScrolling","false"] entry from src/routes/component-data/Drawer.json,
remove the “Body scrolling” section and any <Drawer bodyScrolling={…}> examples
from src/routes/docs/components/drawer.md and mirror those removals in
static/llm/components/drawer.md, and remove bodyScrolling={true} from
src/routes/api-check/components/drawer/examples/Scrolling.svelte; after edits
run rg -n bodyScrolling to ensure no remaining references.
src/routes/docs/components/drawer.md
Outdated
| <Drawer bind:open> | ||
| <h5 class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400"> | ||
| <InfoCircleSolid class="me-2.5 h-5 w-5" />Info | ||
| </h5> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix Svelte binding and add accessible naming in the default Drawer example
bind:open must bind to the variable. Also connect aria-labelledby to the heading.
-<Drawer bind:open>
- <h5 class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400">
+<Drawer bind:open={open} aria-labelledby="drawer-label">
+ <h5 id="drawer-label" class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400">
<InfoCircleSolid class="me-2.5 h-5 w-5" />Info
</h5>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Drawer bind:open> | |
| <h5 class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400"> | |
| <InfoCircleSolid class="me-2.5 h-5 w-5" />Info | |
| </h5> | |
| <Drawer bind:open={open} aria-labelledby="drawer-label"> | |
| <h5 id="drawer-label" class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400"> | |
| <InfoCircleSolid class="me-2.5 h-5 w-5" />Info | |
| </h5> |
🤖 Prompt for AI Agents
In src/routes/docs/components/drawer.md around lines 44 to 47, the Drawer
example uses bind:open without referencing a variable and the heading isn't
referenced for accessibility; change the binding to bind:open={open} (ensure an
`open` variable exists/initialized in the component) and add an id on the
heading (e.g., id="drawer-title") then add aria-labelledby="drawer-title" to the
Drawer component so the dialog is programmatically associated with its heading.
| ```svelte example | ||
| <script> | ||
| import { Drawer, Button, CloseButton, Label, Textarea } from "flowbite-svelte"; | ||
| import { Drawer, CardPlaceholder, Button, CloseButton, Label, Textarea } from "flowbite-svelte"; | ||
| import { InfoCircleSolid, ArrowRightOutline } from "flowbite-svelte-icons"; | ||
|
|
||
| let hidden5 = $state(true); | ||
| let open5 = $state(false); | ||
| </script> | ||
|
|
||
| <div class="text-center"> | ||
| <Button onclick={() => (hidden5 = false)}>Show drawer</Button> | ||
| <Button onclick={() => (open5 = true)}>Show drawer</Button> | ||
| <CardPlaceholder size="2xl" class="mt-6"/> | ||
| </div> | ||
|
|
||
| <Drawer placement="left" bind:hidden={hidden5}> | ||
| <div class="flex items-center justify-between"> | ||
| <h5 id="drawer-label" class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400"> | ||
| <InfoCircleSolid class="me-2.5 h-5 w-5" />Info | ||
| </h5> | ||
| <CloseButton class="mb-4 dark:text-white" /> | ||
| </div> | ||
| <Drawer placement="left" bind:open={open5}> | ||
| <h5 class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400"> | ||
| <InfoCircleSolid class="me-2.5 h-5 w-5" />Info | ||
| </h5> | ||
| <p class="mb-6 text-sm text-gray-500 dark:text-gray-400"> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure Drawer examples are labelled by their headings
Several examples are missing aria-labelledby and matching heading ids. Apply the same pattern used in the default example across these snippets.
Example pattern:
-<Drawer placement="left" bind:open={open5}>
- <h5 class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400">
+<Drawer placement="left" bind:open={open5} aria-labelledby="drawer-label-left">
+ <h5 id="drawer-label-left" class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400">You can list occurrences missing aria-labelledby with:
Also applies to: 298-301, 336-345, 371-374, 440-443, 409-411
🏁 Script executed:
#!/bin/bash
# Drawers without aria-labelledby in this file
rg -n '^\s*<Drawer(?![^>]*aria-labelledby)[^>]*>' src/routes/docs/components/drawer.mdLength of output: 344
🏁 Script executed:
#!/bin/bash
# List Drawer tags missing aria-labelledby in the docs file
rg -n '<Drawer' src/routes/docs/components/drawer.md | grep -v 'aria-labelledby'Length of output: 794
Add aria-labelledby to all Drawer examples
Several <Drawer> instances in src/routes/docs/components/drawer.md are missing aria-labelledby attributes and corresponding heading ids. Please apply the same pattern used in the default example to each of the following locations:
• Line 44: <Drawer bind:open>
• Line 75: <Drawer bind:open={open2} class="…">
• Line 151: <Drawer bind:open={open3}>
• Line 195: <Drawer bind:open={open4}>
• Line 258: <Drawer placement="left" bind:open={open5}>
• Line 298: <Drawer placement="right" transitionParams={transitionParamsRight} bind:open={open6}>
• Line 336: <Drawer placement="top" class="w-full" transitionParams={transitionParamsTop} bind:open={open7}>
• Line 371: <Drawer form placement="bottom" class="w-full" transitionParams={transitionParamsBottom} bind:open={open8}>
• Line 408: <Drawer modal={false} bind:open={openNonModal} class="fixed z-50">
• Line 440: <Drawer outsideclose={false} bind:open={openDisablingOnlyOutsideClick}>
• Line 476: <Drawer bind:open={drawerOpen} bodyScrolling={true}>
Example pattern to follow:
-<Drawer placement="left" bind:open={open5}>
- <h5 class="mb-4 inline-flex items-center …">
+<Drawer placement="left" bind:open={open5} aria-labelledby="drawer-label-left">
+ <h5 id="drawer-label-left" class="mb-4 inline-flex items-center …">Ensure each Drawer’s heading has a unique id matching its aria-labelledby.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/routes/docs/components/drawer.md around the listed lines (44, 75, 151,
195, 258, 298, 336, 371, 408, 440, 476), several <Drawer> examples lack
aria-labelledby and matching heading ids; update each <Drawer> tag to include
aria-labelledby="unique-id-X" and set the corresponding heading element inside
that Drawer to id="unique-id-X" (use a distinct id per example, following the
pattern used in the default example) so every Drawer has an aria-labelledby
value that points to its heading id.
| <Drawer bind:open class="w-96 bg-gray-50 p-6 dark:bg-gray-800"> | ||
| <Heading tag="h5" id="drawer-label" class="mb-8 inline-flex items-center text-base font-semibold text-gray-800 uppercase dark:text-white"> | ||
| <ClockSolid class="h-6 w-6" /> | ||
| Time schedule | ||
| </Heading> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add aria-labelledby on Drawer to provide an accessible name
The heading already has id="drawer-label"; wire the Drawer to it.
-<Drawer bind:open class="w-96 bg-gray-50 p-6 dark:bg-gray-800">
+<Drawer bind:open class="w-96 bg-gray-50 p-6 dark:bg-gray-800" aria-labelledby="drawer-label">
<Heading tag="h5" id="drawer-label" class="mb-8 inline-flex items-center text-base font-semibold text-gray-800 uppercase dark:text-white">
<ClockSolid class="h-6 w-6" />
Time schedule
</Heading>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Drawer bind:open class="w-96 bg-gray-50 p-6 dark:bg-gray-800"> | |
| <Heading tag="h5" id="drawer-label" class="mb-8 inline-flex items-center text-base font-semibold text-gray-800 uppercase dark:text-white"> | |
| <ClockSolid class="h-6 w-6" /> | |
| Time schedule | |
| </Heading> | |
| <Drawer bind:open class="w-96 bg-gray-50 p-6 dark:bg-gray-800" aria-labelledby="drawer-label"> | |
| <Heading | |
| tag="h5" | |
| id="drawer-label" | |
| class="mb-8 inline-flex items-center text-base font-semibold text-gray-800 uppercase dark:text-white" | |
| > | |
| <ClockSolid class="h-6 w-6" /> | |
| Time schedule | |
| </Heading> | |
| … | |
| </Drawer> |
🤖 Prompt for AI Agents
In src/routes/docs/forms/timepicker.md around lines 501 to 505, the Drawer
element lacks an accessible name even though the Heading has id="drawer-label";
add aria-labelledby="drawer-label" to the Drawer component (e.g., <Drawer
bind:open aria-labelledby="drawer-label" ...>) so the Drawer is properly
associated with the heading for screen readers.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/lib/drawer/Drawer.svelte (1)
37-55: Update the component documentation to reflect current propsThe component documentation still references outdated props like
hidden,activateClickOutside,backdrop, andbackdropClass. Update it to match the current API:
- Remove references to deprecated props
- Add
openprop documentation- Update the type reference to the correct line number
Apply this diff:
-@prop hidden = $bindable() -@prop activateClickOutside = true -@prop position +@prop open = $bindable(false) @prop width -@prop backdrop = true -@prop backdropClass @prop placement = "left" @prop class: className @prop classes @prop transitionParams -@prop transitionType = fly +@prop transition = fly
♻️ Duplicate comments (4)
src/lib/types.ts (1)
557-568: Title property is destructured in Dialog.svelte but not part of DialogPropsThe
titleproperty is destructured from props in Dialog.svelte (line 38) but it's not defined in the DialogProps interface. This causes the title to be swallowed and not forwarded to the dialog element.src/lib/drawer/Drawer.svelte (1)
10-10: Remove unused bodyScrolling propThe
bodyScrollingprop is destructured but never used in the component. This creates confusion as it suggests functionality that doesn't exist.src/routes/docs/components/drawer.md (2)
44-47: Fix incomplete binding in default exampleThe
bind:opendirective is missing its binding variable. Based on the script declaration on line 36, this should bind to theopenvariable.
371-371: Remove non-existent form prop from DrawerThe
formprop is not supported by Drawer. This attribute has no effect and should be removed.
🧹 Nitpick comments (2)
src/lib/types.ts (1)
734-734: Consider extracting the empty interface pattern to reduce boilerplateMultiple interfaces (HelperProps, ToolbarGroupProps, TestimonialPlaceholderProps, WidgetPlaceholderProps) follow the same empty extension pattern. Consider extracting a type alias pattern to reduce redundancy.
Add a utility type alias:
type EmptyExtension<V, E> = V & E;Then simplify:
-export interface HelperProps extends HelperVariants, Omit<HTMLAttributes<HTMLParagraphElement>, "color"> { } +export type HelperProps = EmptyExtension<HelperVariants, Omit<HTMLAttributes<HTMLParagraphElement>, "color">>;Also applies to: 1117-1117, 1426-1426, 1436-1436
src/routes/docs/components/drawer.md (1)
392-392: Fix typo: "howerver" should be "however"There's a spelling error in the documentation.
Apply this diff:
-Drawer by default is `modal` (see `dialog`). You can set prop `modal={false}` to open `Drawer` in non-modal mode, that means with no backdrop. Note howerver, that you will need managed the `Drawer` position, z-index and `Esc` button manually. +Drawer by default is `modal` (see `dialog`). You can set prop `modal={false}` to open `Drawer` in non-modal mode, that means with no backdrop. Note however, that you will need to manage the `Drawer` position, z-index and `Esc` button manually.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/drawer/Drawer.svelte(2 hunks)src/lib/types.ts(8 hunks)src/routes/docs/components/drawer.md(14 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/lib/types.ts (6)
src/lib/dialog/theme.ts (1)
DialogVariants(4-4)src/lib/drawer/theme.ts (1)
DrawerVariants(5-5)src/lib/forms/helper/theme.ts (1)
HelperVariants(4-4)src/lib/modal/theme.ts (1)
ModalVariants(6-6)src/lib/toolbar/theme.ts (1)
ToolbarGroupVariants(6-6)src/lib/skeleton/theme.ts (2)
TestimonialPlaceholderVariants(9-9)WidgetPlaceholderVariants(12-12)
🪛 LanguageTool
src/routes/docs/components/drawer.md
[grammar] ~392-~392: Ensure spelling is correct
Context: ...mode, that means with no backdrop. Note howerver, that you will need managed the `Drawer...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_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)
- GitHub Check: Playwright and Lint
🔇 Additional comments (7)
src/lib/types.ts (2)
571-571: LGTM!The DrawerProps interface correctly extends both DrawerVariants and DialogProps, properly implementing the centralized dialog behavior.
1045-1053: LGTM!The ModalProps interface correctly extends ModalVariants and DialogProps (excluding "classes"), and appropriately adds modal-specific styling properties like header, footer, and class properties.
src/lib/drawer/Drawer.svelte (1)
32-34: LGTM! Clean dialog delegationThe implementation correctly delegates all drawer behavior to the Dialog component, properly forwarding transition parameters and maintaining theme styling.
src/routes/docs/components/drawer.md (4)
155-155: Valid use of method="dialog" for dialog-based formsThe
method="dialog"attribute is appropriate here as Drawer now uses a Dialog component internally, which supports native dialog form submission behavior.
388-388: LGTM! Clear documentation of backdrop stylingGood update to document the CSS-based backdrop management approach using
backdrop:xxxclasses.
390-421: LGTM! Well-documented non-modal drawer behaviorThe non-modal section clearly explains the behavior and requirements when using
modal={false}, including proper positioning with fixed classes and z-index management.
423-453: LGTM! Clear documentation of outsideclose propThe section effectively documents the
outsidecloseprop behavior with a working example.
|
Can you run |
|
I did and it pass. Does it fail for you? |
|
pnpm check && pnpm test:e2e flowbite-svelte@1.11.8 check /Users/shinichiokada/Flowbite/flowbite-svelte Loading svelte-check in workspace: /Users/shinichiokada/Flowbite/flowbite-svelte /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Bottom.svelte:18:90 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Bottom.svelte:18:85 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Contact.svelte:11:14 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Contact.svelte:11:9 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Default.svelte:12:14 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Default.svelte:12:9 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Disabled.svelte:13:9 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Disabled.svelte:13:23 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/DisablingOutside.svelte:12:10 <Drawer {outsideclose} bind:open={open10} id="sidebar10"> /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/DisablingOutside.svelte:12:24 <Drawer {outsideclose} bind:open={open10} id="sidebar10"> /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Enabled.svelte:12:14 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Enabled.svelte:12:9 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Form.svelte:11:14 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Form.svelte:11:9 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Left.svelte:12:31 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Left.svelte:12:26 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Navigation.svelte:11:14 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Navigation.svelte:11:9 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/OnlyOutside.svelte:12:9 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/OnlyOutside.svelte:12:30 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Position.svelte:12:49 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Position.svelte:12:44 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Right.svelte:18:73 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Right.svelte:18:68 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Scrolling.svelte:11:14 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Scrolling.svelte:11:9 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Top.svelte:18:84 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/api-check/components/drawer/examples/Top.svelte:18:79 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/admin-dashboard/(sidebar)/crud/products/+page.svelte:98:32 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/admin-dashboard/(sidebar)/crud/products/+page.svelte:98:27 /Users/shinichiokada/Flowbite/flowbite-svelte/src/routes/testdir/drawer/+page.svelte:11:9 ==================================== |
|
You must have something wrong with you conf. All those errors claims property Or add an alias to your kit: {
adapter: adapter(),
alias: { "flowbite-svelte": "src/lib" },
} |
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (3)
src/routes/api-check/components/drawer/examples/Form.svelte (3)
21-22: Fix label-control association (for/id mismatch breaks accessibility)The Label’s for value doesn’t match the Textarea id, so assistive tech won’t announce the description correctly.
Apply this diff:
- <Label for="description" class="mb-2">Description</Label> - <Textarea id="message" placeholder="Write event description..." rows={4} name="message" class="w-full" /> + <Label for="description" class="mb-2">Description</Label> + <Textarea id="description" placeholder="Write event description..." rows={4} name="description" class="w-full" />
24-26: Add a label for the date inputInputs should have an associated label for accessibility and better click target.
Apply this diff:
<div class="mb-6"> + <Label for="date" class="mb-2 block">Date</Label> <Input id="date" name="date" required type="date" /> </div>
28-32: Prevent unintended form submit and improve email input semantics
- The “Add” button likely shouldn’t submit the entire form; set it to type="button".
- Use type="email" for the guest email field and provide an accessible label (sr-only is fine if you want a clean UI).
Apply this diff:
<div class="relative"> - <Input id="search" placeholder="Add guest email" class="p-3" /> - <Button class="bg-primary-700 hover:bg-primary-800 focus:ring-primary-300 dark:bg-primary-600 dark:hover:bg-primary-700 dark:focus:ring-primary-800 absolute end-2 bottom-2 inline-flex items-center rounded-lg px-3 py-1 text-sm font-medium text-white focus:ring-4 focus:outline-hidden" type="submit"> + <Label for="search" class="sr-only">Guest email</Label> + <Input id="search" type="email" name="guest" autocomplete="email" placeholder="Add guest email" class="p-3" /> + <Button class="bg-primary-700 hover:bg-primary-800 focus:ring-primary-300 dark:bg-primary-600 dark:hover:bg-primary-700 dark:focus:ring-primary-800 absolute end-2 bottom-2 inline-flex items-center rounded-lg px-3 py-1 text-sm font-medium text-white focus:ring-4 focus:outline-hidden" type="button"> <UserAddOutline class="me-1.5 h-3 w-3 text-white" />Add </Button> </div>
♻️ Duplicate comments (8)
src/routes/api-check/components/drawer/examples/Left.svelte (1)
12-15: Add aria-labelledby to provide an accessible name (and ensure a unique id)Builds on the previous review: the Drawer still lacks aria-labelledby. Also, consider making the heading id unique to avoid collisions across multiple examples on the same page.
Apply this diff:
-<Drawer placement="left" bind:open={open5}> - <h5 id="drawer-label" class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400"> +<Drawer placement="left" bind:open={open5} aria-labelledby="drawer-left-label"> + <h5 id="drawer-left-label" class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400">Optional: add aria-describedby referencing the paragraph below for a richer accessible description (assign an id to the paragraph and set aria-describedby on Drawer accordingly).
src/routes/api-check/components/drawer/examples/Form.svelte (1)
11-11: Add aria-labelledby to ensure the drawer has an accessible nameLink the dialog to the existing header id.
Apply this diff:
-<Drawer bind:open={open4}> +<Drawer aria-labelledby="drawer-label" bind:open={open4}>src/routes/api-check/components/drawer/examples/Top.svelte (1)
18-21: Add aria-labelledby to provide an accessible name for the dialogThe dialog currently lacks an accessible name. You already have id="drawer-label" on the header; reference it from the Drawer.
Apply this diff:
-<Drawer placement="top" class="w-full" transitionParams={transitionParamsTop} bind:open={open7}> +<Drawer placement="top" class="w-full" transitionParams={transitionParamsTop} bind:open={open7} aria-labelledby="drawer-label">Optional (nice to have): also add aria-describedby pointing to the descriptive paragraph to improve screen reader context.
For example:
<!-- Drawer --> <Drawer ... aria-labelledby="drawer-label" aria-describedby="drawer-desc"> <!-- Description --> <p id="drawer-desc" class="mb-6 max-w-lg text-sm text-gray-500 dark:text-gray-400"> ... </p>src/routes/api-check/components/drawer/examples/Default.svelte (1)
12-12: Fix dialog labelling and remove incorrect aria-controls on the dialogThe dialog’s
aria-labelledbyshould reference the heading id (drawer-label).aria-controlsis not appropriate on the dialog element; it belongs on the trigger.-<Drawer bind:open={open1} id="sidebar1" aria-controls="sidebar1" aria-labelledby="sidebar1"> +<Drawer bind:open={open1} id="sidebar1" aria-labelledby="drawer-label">src/routes/api-check/components/drawer/examples/Navigation.svelte (2)
9-9: Use Svelte’s on:click; onclick won’t attach an event listenerSvelte won’t register the handler via a plain onclick attribute on a component. Use on:click to ensure the Drawer opens.
- <Button onclick={() => (open2 = true)}>Show navigation</Button> + <Button on:click={() => (open2 = true)}>Show navigation</Button>
11-12: Add accessible labeling via aria-labelledby and a matching header idEnsure assistive tech announces the dialog title.
-<Drawer bind:open={open2} class="w-64 bg-gray-50 p-0 dark:bg-gray-800"> - <h5 class="px-4 text-base font-semibold text-gray-500 uppercase dark:text-gray-400">Menu</h5> +<Drawer bind:open={open2} aria-labelledby="drawer-nav-label" class="w-64 bg-gray-50 p-0 dark:bg-gray-800"> + <h5 id="drawer-nav-label" class="px-4 text-base font-semibold text-gray-500 uppercase dark:text-gray-400">Menu</h5>src/routes/api-check/components/drawer/examples/Disabled.svelte (2)
10-10: Use Svelte’s on:click instead of onclickonclick won’t attach a Svelte event listener.
Apply this diff:
- <Button onclick={() => (openBackdropFalse = true)}>Show drawer</Button> + <Button on:click={() => (openBackdropFalse = true)}>Show drawer</Button>
13-13: Label the dialog via aria-labelledbyTie the Drawer to the existing heading for better a11y.
Apply this diff:
-<Drawer modal={false} bind:open={openBackdropFalse} class="fixed inset-y-0 z-50"> +<Drawer modal={false} bind:open={openBackdropFalse} aria-labelledby="drawer-label" class="fixed inset-y-0 z-50">
🧹 Nitpick comments (4)
src/routes/api-check/components/drawer/examples/Bottom.svelte (1)
18-21: Connect the heading to the dialog with aria-labelledby for better a11y.Since Drawer now uses dialog semantics, exposing aria-labelledby improves screen reader labeling.
-<Drawer placement="bottom" class="w-full" transitionParams={transitionParamsBottom} bind:open={open8}> +<Drawer placement="bottom" class="w-full" transitionParams={transitionParamsBottom} bind:open={open8} aria-labelledby="drawer-label"> <h5 id="drawer-label" class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400"> <InfoCircleSolid class="me-2.5 h-5 w-5" />Info </h5>src/routes/api-check/components/drawer/examples/Form.svelte (1)
11-11: Verify bind:open against the local Drawer build (svelte-check errors suggest alias issue)The PR intentionally switched Drawer to bind:open. If svelte-check complains that open is non-bindable/unknown, your examples are likely resolving to the published package instead of the local src/lib. Add an alias and rebuild:
svelte.config.js:
kit: { adapter: adapter(), alias: { "flowbite-svelte": "src/lib" } }Then:
pnpm build
pnpm checkI can help generate a script to assert the alias is set if needed.
src/routes/api-check/components/drawer/examples/Disabled.svelte (2)
13-16: Add an explicit close affordance inside the drawerEven with Esc support, a visible close affordance improves discoverability and accessibility.
Apply this diff to add a CloseButton next to the title:
-<Drawer modal={false} bind:open={openBackdropFalse} aria-labelledby="drawer-label" class="fixed inset-y-0 z-50"> - <h5 id="drawer-label" class="mb-4 inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400"> - <InfoCircleSolid class="me-2.5 h-5 w-5" />Info - </h5> +<Drawer modal={false} bind:open={openBackdropFalse} aria-labelledby="drawer-label" class="fixed inset-y-0 z-50"> + <div class="mb-4 flex items-center justify-between"> + <h5 id="drawer-label" class="inline-flex items-center text-base font-semibold text-gray-500 dark:text-gray-400"> + <InfoCircleSolid class="me-2.5 h-5 w-5" />Info + </h5> + <CloseButton aria-label="Close drawer" on:click={() => (openBackdropFalse = false)} /> + </div>
2-2: Remove testing-library import from this runtime example (if unneeded)Prevents pulling a test-only dependency into the example bundle.
Apply this diff:
-import { findAllByTestId } from "@testing-library/svelte";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/routes/api-check/components/drawer/examples/Bottom.svelte(2 hunks)src/routes/api-check/components/drawer/examples/Contact.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Default.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Disabled.svelte(1 hunks)src/routes/api-check/components/drawer/examples/DisablingOutside.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Enabled.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Form.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Left.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Navigation.svelte(1 hunks)src/routes/api-check/components/drawer/examples/OnlyOutside.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Position.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Right.svelte(2 hunks)src/routes/api-check/components/drawer/examples/Scrolling.svelte(1 hunks)src/routes/api-check/components/drawer/examples/Top.svelte(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/routes/api-check/components/drawer/examples/Right.svelte
- src/routes/api-check/components/drawer/examples/OnlyOutside.svelte
- src/routes/api-check/components/drawer/examples/DisablingOutside.svelte
- src/routes/api-check/components/drawer/examples/Contact.svelte
- src/routes/api-check/components/drawer/examples/Position.svelte
- src/routes/api-check/components/drawer/examples/Enabled.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
PR: themesberg/flowbite-svelte#1442
File: src/lib/utils/Popper.svelte:144-145
Timestamp: 2024-11-22T08:37:27.996Z
Learning: In `src/lib/utils/Popper.svelte`, inconsistencies in event handling (e.g., using both `mouseenter`/`mouseleave` and `pointerenter`/`pointerleave`) may be intentional to minimize modifications and reduce the risk of new problems.
Applied to files:
src/routes/api-check/components/drawer/examples/Navigation.sveltesrc/routes/api-check/components/drawer/examples/Disabled.svelte
📚 Learning: 2024-11-12T10:36:34.807Z
Learnt from: LahTeuto
PR: themesberg/flowbite-svelte#1484
File: src/lib/navbar/NavLi.svelte:34-34
Timestamp: 2024-11-12T10:36:34.807Z
Learning: When enhancing ARIA roles in Svelte components, avoid setting `role="presentation"` on elements that should retain the `link` role, as it can diminish ARIA role coverage and negatively affect accessibility.
Applied to files:
src/routes/api-check/components/drawer/examples/Disabled.sveltesrc/routes/api-check/components/drawer/examples/Default.svelte
⏰ 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)
- GitHub Check: Playwright and Lint
🔇 Additional comments (8)
src/routes/api-check/components/drawer/examples/Left.svelte (1)
5-5: Migration to bind:open looks goodInitialization with $state(false) and using bind:open aligns with the new Dialog-based API.
src/routes/api-check/components/drawer/examples/Form.svelte (1)
5-5: LGTM: correct migration to bindable open stateUsing a local state var for bind:open aligns with the new Dialog-based API.
src/routes/api-check/components/drawer/examples/Top.svelte (1)
6-6: State rename and default closed state align with new Dialog APIUsing open7 with default false matches bind:open semantics. Looks good.
src/routes/api-check/components/drawer/examples/Default.svelte (1)
5-5: LGTM on migrating tobind:openstateThe state rename to align with the new Dialog-based Drawer API looks correct.
src/routes/api-check/components/drawer/examples/Scrolling.svelte (1)
11-14: Good migration to bind:open and simplified header contentThis aligns with the Dialog-based API and removes the deprecated bodyScrolling usage from this example. Looks good.
src/routes/api-check/components/drawer/examples/Navigation.svelte (1)
4-4: State migration looks correctDefaulting to closed with let open2 = $state(false) matches the new open-based API.
src/routes/api-check/components/drawer/examples/Disabled.svelte (2)
13-13: Verify Drawer doesn’t already apply fixed/inset stylesDialog/Drawer typically manages positioning and stacking. Adding class="fixed inset-y-0 z-50" at the usage site can double-create stacking contexts or conflict with internal styles.
Apply this diff if positioning is already handled internally:
-<Drawer modal={false} bind:open={openBackdropFalse} aria-labelledby="drawer-label" class="fixed inset-y-0 z-50"> +<Drawer modal={false} bind:open={openBackdropFalse} aria-labelledby="drawer-label">
6-6: LGTM: bindable state via $state aligns with Svelte 5 and bind:openThis pairs correctly with bind:open usage.
| import { sineIn } from "svelte/easing"; | ||
|
|
||
| let hidden8 = $state(true); | ||
| let open8 = $state(false); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm Svelte 5 runes and local alias setup to avoid svelte-check “open is non-bindable” errors.
This example uses $state and bind:open (new Drawer API). If svelte-check still reports DrawerProps lacks open/non-bindable, your local tooling might resolve to the published package instead of src/lib or target an older Drawer. Verify alias and component props.
🏁 Script executed:
#!/bin/bash
# Verify Svelte version, alias config, and Drawer prop exposure.
set -e
echo "Svelte version (dev/prod deps):"
jq -r '.dependencies.svelte // empty' package.json 2>/dev/null || true
jq -r '.devDependencies.svelte // empty' package.json 2>/dev/null || true
echo -e "\nLook for alias mapping 'flowbite-svelte' -> 'src/lib' in svelte.config.*:"
fd -a 'svelte.config.*' | while read -r f; do
echo -e "\n--- $f ---"
sed -n '1,200p' "$f"
done
echo -e "\nAlias occurrences:"
rg -n 'alias\s*:\s*{[^}]*"flowbite-svelte"\s*:\s*"src/lib"' -A 3 || true
echo -e "\nFind Drawer.svelte implementations and check for 'open' prop:"
fd -a 'Drawer.svelte' | while read -r d; do
echo -e "\n--- $d ---"
rg -n 'export let (open|hidden)|bindable|bind:' "$d" -n || sed -n '1,80p' "$d"
done
echo -e "\nConfirm example uses bind:open in routes:"
rg -n 'bind:open=' src/routes || true
echo -e "\nTip: if alias is missing, set in svelte.config.js kit.alias:"
cat <<'ALIAS_EXAMPLE'
kit: {
adapter: adapter(),
alias: { "flowbite-svelte": "src/lib" },
}
ALIAS_EXAMPLELength of output: 11217
Enable Svelte 5 runes & map flowbite-svelte to src/lib in your config
Your project is on Svelte ^5.38.0, but:
runes: trueis commented out undercompilerOptions- there’s no
kit.aliasentry pointingflowbite-svelte→src/lib
Without these, VS Code/svelte-check will fall back to the published package and flag open as non-bindable. Please update svelte.config.js:
/** @type {import('@sveltejs/kit').Config} */
const config = {
extensions: [".svelte", ...mdsvexConfig.extensions],
compilerOptions: {
- // runes: true
+ runes: true
},
preprocess: [
mdsvex(mdsvexConfig),
vitePreprocess({ typeScript: true })
],
vitePlugin: {
inspector: { holdMode: true }
},
kit: {
adapter: adapter(),
+ alias: {
+ "flowbite-svelte": "src/lib"
+ }
}
};Once that’s in place, svelte-check will correctly resolve your local Drawer and recognize bind:open.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let open8 = $state(false); | |
| /** @type {import('@sveltejs/kit').Config} */ | |
| const config = { | |
| extensions: [".svelte", ...mdsvexConfig.extensions], | |
| compilerOptions: { | |
| runes: true | |
| }, | |
| preprocess: [ | |
| mdsvex(mdsvexConfig), | |
| vitePreprocess({ typeScript: true }) | |
| ], | |
| vitePlugin: { | |
| inspector: { holdMode: true } | |
| }, | |
| kit: { | |
| adapter: adapter(), | |
| alias: { | |
| "flowbite-svelte": "src/lib" | |
| } | |
| } | |
| }; | |
| export default config; |
🤖 Prompt for AI Agents
In src/routes/api-check/components/drawer/examples/Bottom.svelte around line 6,
Svelte-check flags bind:open because the project Svelte config doesn't enable
runes and doesn't map flowbite-svelte to the local src/lib; open is created with
let open8 = $state(false) but the compiler isn't resolving your local Drawer.
Fix svelte.config.js: uncomment/enable compilerOptions.runes = true and add a
kit.alias (or top-level alias) mapping "flowbite-svelte" -> "src/lib" (or the
correct relative path to your local library), then restart the language server
so Svelte 5 runes and the alias allow bind:open to be recognized.
|
|
||
| <div class="text-center"> | ||
| <Button onclick={() => (hidden8 = false)}>Show drawer</Button> | ||
| <Button onclick={() => (open8 = true)}>Show drawer</Button> |
There was a problem hiding this comment.
Use Svelte’s on:click instead of onclick (current handler won’t fire).
onclick on a Svelte component is forwarded as a prop/attribute and won’t attach a DOM listener. Use on:click to actually open the drawer.
- <Button onclick={() => (open8 = true)}>Show drawer</Button>
+ <Button on:click={() => (open8 = true)}>Show drawer</Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Button onclick={() => (open8 = true)}>Show drawer</Button> | |
| <Button on:click={() => (open8 = true)}>Show drawer</Button> |
🤖 Prompt for AI Agents
In src/routes/api-check/components/drawer/examples/Bottom.svelte around line 15,
the Button uses an onclick prop which is forwarded as an attribute and does not
attach a DOM listener, so the handler won't fire; change it to use Svelte's
event directive (on:click) and bind the same inline handler to set open8 = true
so the drawer actually opens when the button is clicked.
| import { InfoCircleSolid, ArrowRightOutline } from "flowbite-svelte-icons"; | ||
| import { sineIn } from "svelte/easing"; | ||
| let hidden1 = $state(true); | ||
| let open1 = $state(false); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure svelte-check resolves the new Drawer typings
The reported “open is non-bindable/unknown prop” errors typically occur when examples import Drawer from the published package instead of the local refactored source. Verify that the dev config aliases "flowbite-svelte" to "src/lib" so svelte-check uses the local Drawer with the new open prop.
Run this script to confirm the alias is present:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find svelte config files
files=$(fd -a 'svelte\.config\.(js|ts|cjs|mjs)$' || true)
if [ -z "$files" ]; then
echo "No svelte.config.* found at repo root"
exit 1
fi
echo "Checking for kit.alias {'flowbite-svelte': 'src/lib'} in:"
echo "$files"
# Look for the alias mapping to local source
if rg -n --pretty 'kit\s*:\s*{[^}]*alias\s*:\s*{[^}]*["'\''`]flowbite-svelte["'\''`]\s*:\s*["'\''`]src/lib["'\''`]' $files; then
echo "Alias found. svelte-check should resolve local Drawer with open prop."
else
echo "Alias not found. Add under kit.alias:"
echo " alias: { 'flowbite-svelte': 'src/lib' }"
exit 1
fiLength of output: 899
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== svelte.config.js (first 200 lines) ==="
sed -n '1,200p' svelte.config.js
echo
echo "=== Searching for 'flowbite-svelte' in svelte.config.js ==="
rg -n "flowbite-svelte" svelte.config.js || echo "No occurrences of 'flowbite-svelte' found"Length of output: 1255
Add missing flowbite-svelte alias so svelte-check uses local Drawer
The dev config currently lacks an alias mapping for flowbite-svelte, so your examples are still resolving the published package instead of your local src/lib source. Please update svelte.config.js to include:
• File: svelte.config.js
• Location: inside the existing kit block
Suggested diff:
kit: {
adapter: adapter(),
+ // Ensure svelte-check resolves local source for Drawer and other components
+ alias: {
+ 'flowbite-svelte': 'src/lib'
+ }
}This will route all imports of flowbite-svelte to your local refactored code and eliminate the “open is non-bindable/unknown prop” errors in your examples.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In svelte.config.js (inside the existing kit block), add a Vite resolve alias
mapping 'flowbite-svelte' to the local source (./src/lib) so imports resolve to
your local refactored code; update kit.vite.resolve.alias (or create it if
missing) to point 'flowbite-svelte' to the project src/lib path so svelte-check
and the examples use the local Drawer component instead of the published
package.
|
|
||
| <div class="text-center"> | ||
| <Button onclick={() => (hidden1 = false)}>Show drawer</Button> | ||
| <Button onclick={() => (open1 = true)}>Show drawer</Button> |
There was a problem hiding this comment.
Use Svelte’s on:click (not onclick) and move control ARIA to the trigger
onclick won’t correctly attach a listener on the Svelte component. Use on:click. Also, move aria-controls to the trigger and add aria-haspopup="dialog" and a reflective aria-expanded for better accessibility.
- <Button onclick={() => (open1 = true)}>Show drawer</Button>
+ <Button
+ on:click={() => (open1 = true)}
+ aria-controls="sidebar1"
+ aria-haspopup="dialog"
+ aria-expanded={open1}
+ >
+ Show drawer
+ </Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Button onclick={() => (open1 = true)}>Show drawer</Button> | |
| <Button | |
| on:click={() => (open1 = true)} | |
| aria-controls="sidebar1" | |
| aria-haspopup="dialog" | |
| aria-expanded={open1} | |
| > | |
| Show drawer | |
| </Button> |
🤖 Prompt for AI Agents
In src/routes/api-check/components/drawer/examples/Default.svelte around line 9,
the button uses the DOM prop "onclick" and the ARIA control is on the wrong
element; replace "onclick" with Svelte's "on:click" on the trigger, move the
aria-controls attribute to that trigger with its value set to the drawer's id,
add aria-haspopup="dialog" and add aria-expanded={open1} on the trigger so it
reflects the open state, and ensure the drawer element has the same id and
role="dialog" (or appropriate dialog semantics) so the attributes correctly
reference the controlled element.
| </h5> | ||
| <CloseButton class="mb-4 dark:text-white" /> | ||
| </div> | ||
| <Drawer modal={false} bind:open={openBackdropFalse} class="fixed inset-y-0 z-50"> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Resolve svelte-check errors for bind:open (type/bindability)
Given the PR’s API shift, ensure Drawer’s open is bindable in the installed source used by these examples. If examples import from "flowbite-svelte" (package), but your local type/impl still has hidden and/or non-bindable open, svelte-check will fail.
Options:
- Ensure Drawer exports a bindable open (Svelte 5) and that types reflect it.
- Or point "flowbite-svelte" to "src/lib" during local dev (as suggested) so examples type-check against the PR’s updated code.
Run this script to verify that open is present and bindable in the local Drawer implementation and that examples resolve to it:
If you prefer the alias approach, add this to svelte.config.js:
// svelte.config.js (excerpt)
kit: {
adapter: adapter(),
alias: {
'flowbite-svelte': 'src/lib'
}
}🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Locate Drawer implementation"
fd -a 'Drawer.svelte' | sed -n '1,120p'
echo
echo "2) Inspect Drawer for bindable 'open' (look for $state or @bindable)"
# Expect to see either: export let open = $state(false) OR a JSDoc @bindable above 'export let open'
rg -n --sortr path $'export\\s+let\\s+open' -A 3
rg -n --sortr path $'\\$state\\(|@bindable' src | sed -n '1,200p'
echo
echo "3) Verify examples import resolves to local src/lib (if using alias)"
# Expect alias in svelte.config.js: alias: { "flowbite-svelte": "src/lib" }
rg -n $'alias\\s*:\\s*\\{[^}]*"flowbite-svelte"\\s*:\\s*"src/lib"' svelte.config.* jsconfig.* tsconfig.*
echo
echo "4) Grep DrawerProps typing to ensure 'open' exists and is bindable"
rg -n 'interface\\s+DrawerProps|type\\s+DrawerProps' -A 6 src
rg -n 'open\\s*:\\s*boolean' src
echo
echo "5) Re-run checks"
echo "pnpm build && pnpm check && pnpm test:e2e"Length of output: 281
To properly verify bindable open support in your local Drawer implementation, let’s rerun a focused script that avoids unescaped $ in echoes and pinpoints the definitions and alias configuration:
This will confirm whether:
openis declared and bindable in your localDrawer.svelte.- Your TypeScript typings include
open. - You’ve configured the
flowbite-sveltealias to point atsrc/libfor local examples.
Once we have the output, we can determine if fixes or config updates are needed.
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "1) Locate Drawer.svelte"
drawer_file=$(fd -a Drawer.svelte | head -n 1)
echo "→ $drawer_file"
echo
echo "2) Search for 'export let open' in Drawer.svelte"
rg -n 'export\s+let\s+open' "$drawer_file" -A 3 || echo " No 'export let open' found"
echo
echo "3) Search for \$state usage in Drawer.svelte"
rg -n '\$state\(' "$drawer_file" -A 3 || echo " No \$state found"
echo
echo "4) Search for @bindable JSDoc in Drawer.svelte"
rg -n '@bindable' "$drawer_file" -A 3 || echo " No @bindable found"
echo
echo "5) Inspect DrawerProps typings"
rg -n 'interface\s+DrawerProps' -A 6 src/lib/drawer || echo " No DrawerProps interface"
rg -n 'type\s+DrawerProps' -A 6 src/lib/drawer || echo " No DrawerProps type"
echo
echo "6) Verify 'open' in DrawerProps"
rg -n 'open\s*:' src/lib/drawer || echo " No open property in types"
echo
echo "7) Check for flowbite-svelte → src/lib alias"
rg -n 'alias' svelte.config.js tsconfig.json jsconfig.json || echo " No alias key found"
rg -n '"flowbite-svelte"\s*:\s*"src/lib"' svelte.config.js tsconfig.json jsconfig.json || echo " No flowbite-svelte → src/lib alias"Length of output: 2858
Enable bindable open on Drawer and configure local alias
Findings:
src/lib/drawer/Drawer.sveltedoes not export anopenprop or use the Svelte 5 bindable API.- The
DrawerPropstype (imported from$lib) has noopenfield. - No
flowbite-svelte → src/libalias is set insvelte.config.jsor TypeScript config, so examples import the published package instead of your local source.
Action items:
- In
src/lib/drawer/Drawer.svelte, add a bindableopenprop. For Svelte 5:Or for Svelte 4:/** @bindable */ export let open = $state(false);
export let open = false;
- Update your
DrawerPropstype to include:open?: boolean;
- Add a local‐dev alias so examples point at your in-repo code. In
svelte.config.js:kit: { adapter: adapter(), alias: { 'flowbite-svelte': 'src/lib' } }
After applying these changes, re-run:
pnpm build && pnpm check && pnpm test:e2e
to confirm that bind:open errors are resolved.
🤖 Prompt for AI Agents
In src/routes/api-check/components/drawer/examples/Disabled.svelte around line
13, the example uses bind:open but the Drawer component and types don't expose a
bindable open prop and the example imports the published package instead of
local source; fix by adding a bindable open prop to src/lib/drawer/Drawer.svelte
(use the Svelte 5 @bindable export or export let open = false for Svelte 4),
update the DrawerProps type in your library to include open?: boolean, and add a
local dev alias in svelte.config.js mapping 'flowbite-svelte' to 'src/lib' so
examples import the in-repo component; after changes run pnpm build && pnpm
check && pnpm test:e2e to verify bind:open works.
|
|
||
| <div class="text-center"> | ||
| <Button onclick={() => (hidden5 = false)}>Show drawer</Button> | ||
| <Button onclick={() => (open5 = true)}>Show drawer</Button> |
There was a problem hiding this comment.
Fix Svelte event directive (use on:click, not onclick)
onclick won’t register as an event listener on a Svelte component. Use on:click so the button actually opens the drawer.
Apply this diff:
- <Button onclick={() => (open5 = true)}>Show drawer</Button>
+ <Button on:click={() => (open5 = true)}>Show drawer</Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Button onclick={() => (open5 = true)}>Show drawer</Button> | |
| <Button on:click={() => (open5 = true)}>Show drawer</Button> |
🤖 Prompt for AI Agents
In src/routes/api-check/components/drawer/examples/Left.svelte around line 9,
the Button uses the native attribute onclick which Svelte components do not
bind; replace onclick={() => (open5 = true)} with the Svelte event directive
on:click={() => (open5 = true)} so the click handler properly registers and
opens the drawer.
| <h5 class="px-4 text-base font-semibold text-gray-500 uppercase dark:text-gray-400">Menu</h5> | ||
| <CloseButton class="dark:text-white" /> | ||
| </div> | ||
| <Drawer bind:open={open2} class="w-64 bg-gray-50 p-0 dark:bg-gray-800"> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify svelte-check setup resolves the local Drawer with bind:open
The usage of bind:open aligns with this PR, but the reported svelte-check errors suggest the examples might be resolving Drawer from the published package (which still expects bind:hidden) instead of src/lib. Confirm the alias is configured so imports from "flowbite-svelte" point to src/lib.
Run this script to verify the alias and the Drawer prop in the PR branch:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Check svelte.config.* for alias mapping flowbite-svelte -> src/lib"
fd -a 'svelte.config.*' | while read -r cfg; do
echo "---- $cfg ----"
rg -n "alias|kit|flowbite-svelte" "$cfg" || true
done
echo
echo "2) Check Drawer implementation exposes an 'open' prop (and not 'hidden')"
fd -a Drawer.svelte | rg -n 'src/lib' | while read -r file; do
echo "---- $file ----"
rg -n "export let open|let open =|bindable|hidden" "$file" || true
done
echo
echo "3) Grep example usage of bind:open (should be present)"
rg -n "bind:open" src/routes || true
echo
echo "If alias is missing, add this to svelte.config.* under kit:"
cat <<'CFG'
kit: {
// ...
alias: {
"flowbite-svelte": "src/lib"
}
}
CFGLength of output: 12520
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Verify Drawer.svelte exports 'open' and not 'hidden'"
rg -n "export let open" src/lib/drawer/Drawer.svelte
rg -n "export let hidden" src/lib/drawer/Drawer.svelte || echo "No 'hidden' export found"Length of output: 207
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Verifying Drawer.svelte exports in src/lib/drawer/Drawer.svelte..."
echo "1) Checking for 'export let open':"
rg -n "export let open" src/lib/drawer/Drawer.svelte || echo " → No 'export let open' found"
echo
echo "2) Checking for 'export let hidden':"
rg -n "export let hidden" src/lib/drawer/Drawer.svelte || echo " → No 'export let hidden' found"Length of output: 614
#!/usr/bin/env bash
set -euo pipefail
echo "=== svelte.config.js (first 200 lines) ==="
sed -n '1,200p' svelte.config.js || true
echo
echo "=== Locating Drawer.svelte ==="
fd -t f Drawer.svelte || true
FILE=$(fd -t f Drawer.svelte | head -n1 || true)
if [[ -n "$FILE" ]]; then
echo
echo "=== Preview of $FILE (first 50 lines) ==="
head -n50 "$FILE"
else
echo "Drawer.svelte not found in src/lib/drawer or elsewhere."
fi#!/usr/bin/env bash
set -euo pipefail
echo "=== Checking alias mapping in svelte.config.js ==="
rg -n "alias" svelte.config.js || echo "→ No alias section found"
rg -n "flowbite-svelte" svelte.config.js || echo "→ No flowbite-svelte reference found"
echo
echo "=== Locating all Drawer.svelte files ==="
DRAWER_FILES=$(fd -t f Drawer.svelte || true)
if [[ -z "$DRAWER_FILES" ]]; then
echo "→ No Drawer.svelte files found"
exit 0
fi
echo "$DRAWER_FILES"
for file in $DRAWER_FILES; do
echo
echo "---- $file ----"
rg -n "export let open" "$file" || echo " → 'export let open' not declared"
rg -n "export let hidden" "$file" || echo " → 'export let hidden' not declared"
doneAdd SvelteKit alias so examples resolve local Drawer component
It looks like your svelte.config.js does not define an alias for flowbite-svelte, so svelte-check may be picking up the published package (which still expects a bind:hidden prop) instead of your local src/lib. Please add the following under the kit section in svelte.config.js:
kit: {
// …
+ alias: {
+ 'flowbite-svelte': 'src/lib'
+ }
}This will ensure that imports from "flowbite-svelte" in your examples (including the Navigation.svelte snippet) resolve to your PR’s source code where Drawer properly exports open rather than hidden.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Drawer bind:open={open2} class="w-64 bg-gray-50 p-0 dark:bg-gray-800"> | |
| // In svelte.config.js, under the `kit` section, add the alias: | |
| kit: { | |
| // … | |
| alias: { | |
| 'flowbite-svelte': 'src/lib' | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/routes/api-check/components/drawer/examples/Navigation.svelte around line
11, the example imports resolve to the published flowbite-svelte package (which
expects bind:hidden) instead of the local Drawer component; update
svelte.config.js by adding a kit.alias (mapping "flowbite-svelte" to your local
src/lib path) so SvelteKit and svelte-check resolve imports to the PR source
code where Drawer exports bind:open; save and restart the dev server / re-run
svelte-check to verify examples now use the local component.
| import { Drawer, Button, CloseButton } from "flowbite-svelte"; | ||
| import { InfoCircleSolid, ArrowRightOutline } from "flowbite-svelte-icons"; | ||
| let drawerHidden = $state(true); | ||
| let openHidden = $state(false); |
There was a problem hiding this comment.
Fix event binding (use on:click) and align state name with new API
- Svelte requires on:click (not onclick). The current handler won’t fire.
- Rename openHidden to open for clarity and consistency with bind:open.
- let openHidden = $state(false);
+ let open = $state(false);- <Button onclick={() => (openHidden = true)}>Show drawer</Button>
+ <Button on:click={() => (open = true)}>Show drawer</Button>-<Drawer bind:open={openHidden}>
+<Drawer bind:open={open}>Also applies to: 8-8, 11-11
🤖 Prompt for AI Agents
In src/routes/api-check/components/drawer/examples/Scrolling.svelte around lines
4, 8 and 11, rename the local state variable from openHidden to open (change the
let declaration using $state to let open = $state(false)) and update the click
handlers to use Svelte's on:click syntax instead of onclick on the relevant
elements; also update any bind:open usages to reference the new open variable so
binding and events work with the new API.
| </h5> | ||
| <CloseButton class="mb-4 dark:text-white" /> | ||
| </div> | ||
| <Drawer bind:open={openHidden}> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Optional: provide an accessible name for the dialog
Consider wiring the header to the dialog for screen readers. If Drawer/Dialog forwards ARIA attributes to the dialog element, add aria-labelledby referencing the heading id.
Example change (if supported by Drawer/Dialog):
-<Drawer bind:open={open}>
+<Drawer bind:open={open} aria-labelledby="drawer-label">To verify support, run:
🏁 Script executed:
#!/bin/bash
# Check if Drawer/Dialog forwards aria-labelledby or exposes a specific prop (e.g., labelledby/ariaLabel)
rg -n --no-heading $'aria-labelledby|labelledby|ariaLabel' src/lib/dialog src/lib/drawer src/lib/typesLength of output: 157
🏁 Script executed:
#!/bin/bash
# 1. Locate the Drawer component file
DRAWER_FILE=$(fd -t f Drawer.svelte src | head -n1)
if [[ -z "$DRAWER_FILE" ]]; then
echo "ERROR: Could not find Drawer.svelte in src/"
exit 1
fi
echo "Found Drawer component at: $DRAWER_FILE"
echo
# 2. Check for rest‐props forwarding
echo "Checking for $$restProps forwarding:"
grep -R '{\$\$restProps}' -n "$DRAWER_FILE" || echo " → No $$restProps forwarding found"
echo
# 3. Inspect top of the file for dialog or ARIA handling
echo "Inspecting first 200 lines of the component:"
sed -n '1,200p' "$DRAWER_FILE"
echo
# 4. Search for any aria- attributes in the component
echo "Searching for aria- attributes in the component:"
grep -R 'aria-' -n "$DRAWER_FILE" || echo " → No aria- attributes referenced"
echo
# 5. Look for a <dialog> element or role="dialog"
echo "Searching for <dialog> tag or role=\"dialog\":"
grep -R -n "<dialog" "$DRAWER_FILE" && exit 0
grep -R -n 'role=.*dialog' "$DRAWER_FILE" || echo " → No native <dialog> or role=\"dialog\" usage found"Length of output: 2959
🏁 Script executed:
#!/bin/bash
# Locate Dialog component file
DIALOG_FILE=$(fd -t f Dialog.svelte src/lib/dialog | head -n1)
if [[ -z "$DIALOG_FILE" ]]; then
echo "ERROR: Could not find Dialog.svelte in src/lib/dialog"
exit 1
fi
echo "Found Dialog component at: $DIALOG_FILE"
echo
# 1. Check for restProps in props destructuring
echo "Looking for restProps in props destructuring:"
grep -n '...restProps' -n "$DIALOG_FILE" || echo " → No restProps destructuring found"
echo
# 2. Search for restProps spread in markup
echo "Searching for {...restProps} in markup:"
grep -n '{\.\.\.restProps}' "$DIALOG_FILE" || echo " → No restProps spread in markup found"
echo
# 3. Inspect the top 200 lines of the component for context
echo "Inspecting first 200 lines of Dialog.svelte:"
sed -n '1,200p' "$DIALOG_FILE"Length of output: 6432
Add an accessible name to the drawer
The Drawer component forwards all extra props down to the underlying <dialog>, so you can expose your header to screen readers by wiring up aria-labelledby.
– Give your header element an id, e.g.
<h2 id="drawer-label">Scrolling Content</h2>– Update the Drawer invocation:
- <Drawer bind:open={openHidden}>
+ <Drawer bind:open={openHidden} aria-labelledby="drawer-label">Since Drawer spreads {...restProps} into Dialog.svelte, and Dialog.svelte in turn spreads {...restProps} onto its <dialog> element, this will correctly expose the label to assistive technologies.
🤖 Prompt for AI Agents
In src/routes/api-check/components/drawer/examples/Scrolling.svelte around line
11, the Drawer lacks an accessible name for screen readers; add an id to the
header (e.g. assign an id like "drawer-label" to the <h2> element) and pass
aria-labelledby="drawer-label" to the <Drawer> component so the prop is
forwarded to the underlying <dialog>, exposing the header as the dialog label
for assistive technologies.
|
|
||
| <div class="text-center"> | ||
| <Button onclick={() => (hidden7 = false)}>Show drawer</Button> | ||
| <Button onclick={() => (open7 = true)}>Show drawer</Button> |
There was a problem hiding this comment.
Fix event handler: use Svelte’s on:click instead of onclick
onclick won’t attach a Svelte event listener; the button won’t open the drawer.
Apply this diff:
- <Button onclick={() => (open7 = true)}>Show drawer</Button>
+ <Button on:click={() => (open7 = true)}>Show drawer</Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Button onclick={() => (open7 = true)}>Show drawer</Button> | |
| <Button on:click={() => (open7 = true)}>Show drawer</Button> |
🤖 Prompt for AI Agents
In src/routes/api-check/components/drawer/examples/Top.svelte around line 15,
the Button uses a plain DOM onclick attribute which Svelte won't bind; replace
onclick={() => (open7 = true)} with Svelte's on:click handler (on:click={() =>
open7 = true}) so the click opens the drawer; update that single line
accordingly.
|
Double close button. 
|
|
https://flowbite-svelte.com/docs/components/drawer#non-modal Using |
|
Non-modal means not backdrop and the ability to play with the rest of the elements of the page. The See: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/dialog |
2 participants
📑 Description
Helper component created:
Dialog. It's now base for:Modalquite straight forwardDrawer- breaking changeThe biggest change for
Draweris that now it's controlled bybind:opennot bybind:hidden. This to be in sync with underlyingDialog.As
Draweris nowdialogwe don't need to manage backdrop, but rathermodal / non-modalcase.Position
fixedandabsolutenow don't have sense (unlessnon-modalcase).Body scrolling - I don't understand that case, so probably to be removed.
Lots of side effects as
Drawerwas used in many other files / examples.Status
✅ Checks
api-checkdirectory as requiredℹ Additional Information
Summary by CodeRabbit
New Features
Refactor
Documentation