Conversation
|
@shinokada is attempting to deploy a commit to the Themesberg Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a SplitPane feature: new Svelte components (SplitPane, Pane, Divider), context API and resizing logic, types and theme tokens, barrel re-exports, docs and examples page, an e2e test, and updates to theme exports. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Docs Page
participant SP as SplitPane
participant Ctx as SplitPaneContext
participant Pane as Pane
participant Div as Divider
App->>SP: mount SplitPane(props)
SP->>Ctx: setSplitPaneContext(ctx)
SP->>SP: init registry, sizes, direction
rect rgb(230,248,255)
Note over Pane,SP: Pane registers and requests style
Pane->>Ctx: getSplitPaneContext()
Pane->>SP: registerPane()
SP-->>Pane: provide index & computed style
end
rect rgb(255,240,240)
Note over Div,SP: User interactions (mouse/keyboard)
Div->>SP: onMouseDown(e,index) / onKeyDown(e,index)
SP->>SP: compute/clamp sizes, update state
SP-->>Pane: update styles
SP->>App: emit onResize(sizes)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/lib/index.ts(1 hunks)src/lib/splitpane/Divider.svelte(1 hunks)src/lib/splitpane/Pane.svelte(1 hunks)src/lib/splitpane/SplitPane.svelte(1 hunks)src/lib/splitpane/index.ts(1 hunks)src/lib/types.ts(1 hunks)src/routes/docs-examples/extend/split-pane/+page.svelte(1 hunks)src/routes/docs-examples/extend/split-pane/default/+page.svelte(1 hunks)src/routes/docs/extend/split-pane.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/SplitPane.svelte
📚 Learning: 2024-11-12T10:36:34.807Z
Learnt from: LahTeuto
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/Divider.svelte
🪛 ESLint
src/routes/docs-examples/extend/split-pane/+page.svelte
[error] 185-209: Found an unnecessary children snippet.
(svelte/no-useless-children-snippet)
[error] 232-256: Found an unnecessary children snippet.
(svelte/no-useless-children-snippet)
[error] 282-318: Found an unnecessary children snippet.
(svelte/no-useless-children-snippet)
[error] 341-365: Found an unnecessary children snippet.
(svelte/no-useless-children-snippet)
[error] 388-420: Found an unnecessary children snippet.
(svelte/no-useless-children-snippet)
[error] 403-417: Found an unnecessary children snippet.
(svelte/no-useless-children-snippet)
[error] 447-476: Found an unnecessary children snippet.
(svelte/no-useless-children-snippet)
[error] 499-548: Found an unnecessary children snippet.
(svelte/no-useless-children-snippet)
[error] 519-545: Found an unnecessary children snippet.
(svelte/no-useless-children-snippet)
[error] 529-529: Unexpected mustache interpolation with a string literal value.
(svelte/no-useless-mustaches)
src/routes/docs-examples/extend/split-pane/default/+page.svelte
[error] 49-73: Found an unnecessary children snippet.
(svelte/no-useless-children-snippet)
⏰ 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
| export interface DividerProps { | ||
| direction: 'horizontal' | 'vertical'; | ||
| index: number; | ||
| onMouseDown: (e: MouseEvent, index: number) => void; | ||
| onKeyDown: (e: KeyboardEvent, index: number) => void; | ||
| isDragging: boolean; | ||
| class?: string; | ||
| } No newline at end of file |
There was a problem hiding this comment.
Extend DividerProps to carry the current value.
To back the aria-valuenow requirement in Divider.svelte, DividerProps should expose a numeric field (e.g., value or percent) so the divider can announce its position. Please add the new property here and thread it through the context/Divider call sites.
🤖 Prompt for AI Agents
In src/lib/types.ts around lines 2147 to 2154, DividerProps needs a numeric
field to represent the current position for aria-valuenow; add a property (e.g.,
value: number or percent: number) to the interface and then update any context
definition, provider, and all places that construct/pass DividerProps so they
supply this numeric value, and update the Divider.svelte usage to read that
property for aria-valuenow; ensure types for context consumers and any helper
functions/signatures are adjusted to accept the new field.
| {#snippet children()} | ||
| <Pane> | ||
| <div class="p-4 bg-blue-50 h-full"> | ||
| <h3 class="font-semibold">Left Pane</h3> | ||
| <p>This is the left pane content. Drag the divider to resize!</p> | ||
| {#if sizes.length > 0 && basicContainerWidth > 0} | ||
| <div class="mt-4 p-2 bg-white rounded border text-xs font-mono"> | ||
| Width: {sizes[0].toFixed(2)}% = {getPixels(sizes[0], basicContainerWidth)}px | ||
| </div> | ||
| {/if} | ||
| </div> | ||
| </Pane> | ||
|
|
||
| <Pane> | ||
| <div class="p-4 bg-green-50 h-full"> | ||
| <h3 class="font-semibold">Right Pane</h3> | ||
| <p>This is the right pane content.</p> | ||
| {#if sizes.length > 0 && basicContainerWidth > 0} | ||
| <div class="mt-4 p-2 bg-white rounded border text-xs font-mono"> | ||
| Width: {sizes[1].toFixed(2)}% = {getPixels(sizes[1], basicContainerWidth)}px | ||
| </div> | ||
| {/if} | ||
| </div> | ||
| </Pane> | ||
| {/snippet} |
There was a problem hiding this comment.
Fix failing lint: remove unnecessary children snippet wrapper.
ESLint (svelte/no-useless-children-snippet) flags this block. SplitPane already receives the default slot as its children snippet, so the explicit {#snippet children()} wrapper is redundant and will break the lint pipeline. Drop the wrapper and render the panes directly inside <SplitPane>.
Apply this diff:
- <SplitPane responsive={false} minSize={600} onResize={handleBasicResize}>
- {#snippet children()}
- <Pane>
- <div class="p-4 bg-blue-50 h-full">
- <h3 class="font-semibold">Left Pane</h3>
- <p>This is the left pane content. Drag the divider to resize!</p>
- {#if sizes.length > 0 && basicContainerWidth > 0}
- <div class="mt-4 p-2 bg-white rounded border text-xs font-mono">
- Width: {sizes[0].toFixed(2)}% = {getPixels(sizes[0], basicContainerWidth)}px
- </div>
- {/if}
- </div>
- </Pane>
-
- <Pane>
- <div class="p-4 bg-green-50 h-full">
- <h3 class="font-semibold">Right Pane</h3>
- <p>This is the right pane content.</p>
- {#if sizes.length > 0 && basicContainerWidth > 0}
- <div class="mt-4 p-2 bg-white rounded border text-xs font-mono">
- Width: {sizes[1].toFixed(2)}% = {getPixels(sizes[1], basicContainerWidth)}px
- </div>
- {/if}
- </div>
- </Pane>
- {/snippet}
- </SplitPane>
+ <SplitPane responsive={false} minSize={600} onResize={handleBasicResize}>
+ <Pane>
+ <div class="p-4 bg-blue-50 h-full">
+ <h3 class="font-semibold">Left Pane</h3>
+ <p>This is the left pane content. Drag the divider to resize!</p>
+ {#if sizes.length > 0 && basicContainerWidth > 0}
+ <div class="mt-4 p-2 bg-white rounded border text-xs font-mono">
+ Width: {sizes[0].toFixed(2)}% = {getPixels(sizes[0], basicContainerWidth)}px
+ </div>
+ {/if}
+ </div>
+ </Pane>
+
+ <Pane>
+ <div class="p-4 bg-green-50 h-full">
+ <h3 class="font-semibold">Right Pane</h3>
+ <p>This is the right pane content.</p>
+ {#if sizes.length > 0 && basicContainerWidth > 0}
+ <div class="mt-4 p-2 bg-white rounded border text-xs font-mono">
+ Width: {sizes[1].toFixed(2)}% = {getPixels(sizes[1], basicContainerWidth)}px
+ </div>
+ {/if}
+ </div>
+ </Pane>
+ </SplitPane>🧰 Tools
🪛 ESLint
[error] 49-73: Found an unnecessary children snippet.
(svelte/no-useless-children-snippet)
🤖 Prompt for AI Agents
In src/routes/docs-examples/extend/split-pane/default/+page.svelte around lines
49 to 73, remove the unnecessary {#snippet children()} ... {/snippet} wrapper
and place the two <Pane> blocks directly inside the <SplitPane> component; this
fixes the svelte/no-useless-children-snippet lint error by letting SplitPane
receive its default slot (children) directly and avoids the redundant snippet
wrapper that caused the lint failure.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/splitpane/Divider.svelte(1 hunks)src/lib/splitpane/SplitPane.svelte(1 hunks)src/routes/docs-examples/extend/split-pane/+page.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/docs-examples/extend/split-pane/+page.svelte
- src/lib/splitpane/Divider.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/SplitPane.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
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/lib/splitpane/SplitPane.svelte (3)
92-95: Consider validating initialSizes.If
initialSizesis provided with values that don't sum to approximately 100, the panes will be sized incorrectly (e.g.,[60, 50]sums to 110, leaving overflow or odd gaps). Consider normalizing or validating the sum.Example normalization:
if (initialSizes && initialSizes.length === registeredPanes) { - sizes = [...initialSizes]; + const sum = initialSizes.reduce((a, b) => a + b, 0); + sizes = sum > 0 ? initialSizes.map(s => (s / sum) * 100) : [...initialSizes]; return; }
118-120: Edge case: Direction change during active drag.If the viewport crosses the breakpoint while a user is actively dragging (i.e.,
isDragging === true),currentDirectionflips butstartPoswas captured for the old orientation (clientX vs clientY), causing the delta calculation inresize()to jump. Consider deferring the direction switch until the drag completes.Example guard:
const handleResize = () => { - currentDirection = mq.matches ? 'vertical' : 'horizontal'; + if (!isDragging) { + currentDirection = mq.matches ? 'vertical' : 'horizontal'; + } };
159-201: Consider disabling transition during active drag.The transition defined in
getPaneStyleapplies during drag, which can make the resize feel less responsive. Consider temporarily settingtransition = falseduring the drag operation for smoother visual feedback.Example:
function startResize(e: MouseEvent, index: number) { e.preventDefault(); isDragging = true; + transition = false; startPos = currentDirection === 'horizontal' ? e.clientX : e.clientY; // ... } function stopResize(moveHandler: (e: MouseEvent) => void, upHandler: () => void) { isDragging = false; + transition = true; // ... }Note: This assumes
transitionshould be mutable. Alternatively, introduce a separateisTransitioningstate variable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/splitpane/SplitPane.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/SplitPane.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 (5)
src/lib/splitpane/SplitPane.svelte (5)
1-23: LGTM: Clean context API setup.The module-scoped context with a Symbol key follows Svelte best practices and provides a well-typed interface for child components.
25-46: LGTM: Props and state properly initialized.Default values are sensible and all state variables are appropriately typed.
48-83: LGTM: Core functions and context setup are correct.The pane registration, style computation, and context provisioning logic is clean and properly handles both horizontal and vertical orientations.
203-248: LGTM: Keyboard resize properly respects minimum constraints.The
applyClamphelper correctly enforcesminPercenton both panes, addressing the previously flagged issue. The logic handles increase, decrease, and reset operations correctly.
251-256: LGTM: Template is clean and correct.The container binding, class application, and children rendering follow Svelte 5 conventions properly.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
e2e/extend.spec.ts(1 hunks)
⏰ 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 (1)
e2e/extend.spec.ts (1)
28-31: LGTM!The test implementation follows the established pattern correctly and validates the Split Pane documentation page as expected.
…l the drag completes, disabling transition during active drag
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/splitpane/Pane.svelte (1)
16-30: SimplifypaneStylederived computation.The current pattern creates a derived function that's immediately invoked. In Svelte 5, use
$derived.by()for complex computations or compute the value directly within$derived().Apply this diff:
- const paneStyle = $derived(() => { + const paneStyle = $derived.by(() => { const styles = [style]; if (context) { const contextStyle = context.getPaneStyle(paneIndex); styles.push(contextStyle); } return styles.filter(Boolean).join('; '); }); -<div class={pane({ class: className })} style={paneStyle()}> +<div class={pane({ class: className })} style={paneStyle}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/lib/splitpane/Divider.svelte(1 hunks)src/lib/splitpane/Pane.svelte(1 hunks)src/lib/splitpane/SplitPane.svelte(1 hunks)src/lib/splitpane/index.ts(1 hunks)src/lib/splitpane/theme.ts(1 hunks)src/routes/docs-examples/extend/split-pane/+page.svelte(1 hunks)src/routes/docs/extend/split-pane.md(1 hunks)src/routes/docs/extend/virtual-list.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/routes/docs/extend/virtual-list.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/docs-examples/extend/split-pane/+page.svelte
- src/lib/splitpane/index.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-12T10:36:34.807Z
Learnt from: LahTeuto
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/Divider.svelte
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/SplitPane.svelte
🧬 Code graph analysis (1)
src/lib/splitpane/theme.ts (1)
src/lib/splitpane/index.ts (3)
splitpane(4-4)pane(4-4)divider(4-4)
⏰ 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 (4)
src/lib/splitpane/Divider.svelte (1)
17-27: LGTM! Accessibility implementation is solid.The separator role and ARIA attributes are correctly configured:
aria-orientationproperly reflects the separator's physical orientation- Keyboard and mouse event handlers are appropriately wired
- Focus management with
tabindex="0"enables keyboard navigationThe svelte-ignore directives are justified for this interactive separator pattern.
src/lib/splitpane/SplitPane.svelte (2)
105-129: Excellent SSR handling!The SSR guard on line 111-114 correctly prevents
windowaccess server-side, and the responsive behavior appropriately defers direction changes during active dragging.
206-251: Keyboard resize respects minimum constraints correctly.The
applyClamphelper (lines 221-235) properly enforcesminPercenton both panes, preventing the neighbor violation flagged in previous reviews.src/lib/splitpane/theme.ts (1)
1-53: Theme configuration is well-structured.The Tailwind variants provide:
- Appropriate focus styles for accessibility (line 25: outline with offset)
- Good UX with
select-noneto prevent text selection during drag (line 8)- Enlarged hit areas for better usability (lines 46-47: 12px area for 4px divider)
The direction-aware styling correctly handles both horizontal and vertical split orientations.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/lib/splitpane/Divider.svelte (1)
24-32: Address the outstanding accessibility issue witharia-valuenow.The past review comment correctly identified that this divider advertises slider semantics but doesn't expose the current position to assistive technologies via
aria-valuenow. This remains unresolved and blocks screen reader users from knowing the splitter position.Please implement the suggestion from the previous review to surface the current pane percentage from SplitPane and bind it here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/lib/splitpane/Divider.svelte(1 hunks)src/lib/splitpane/Pane.svelte(1 hunks)src/lib/splitpane/SplitPane.svelte(1 hunks)src/lib/splitpane/index.ts(1 hunks)src/lib/theme/themes.ts(1 hunks)src/routes/docs/extend/split-pane.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/routes/docs/extend/split-pane.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/splitpane/index.ts
- src/lib/splitpane/Pane.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/SplitPane.svelte
📚 Learning: 2024-11-12T10:36:34.807Z
Learnt from: LahTeuto
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/Divider.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 (7)
src/lib/theme/themes.ts (1)
83-83: LGTM!The theme export follows the established pattern and is properly placed in the extend section alongside other similar exports.
src/lib/splitpane/SplitPane.svelte (6)
1-23: LGTM!The module script correctly implements the Svelte context API pattern with proper TypeScript types and a unique symbol key for the context.
60-71: LGTM!The
getPaneStylefunction correctly generates dynamic styles for panes based on direction, with proper transition support.
187-227: LGTM!The resize logic correctly enforces minimum size constraints for both panes while maintaining their total size, and uses a reasonable threshold to prevent jittery updates.
229-274: LGTM!The keyboard resize handler properly enforces minimum constraints on both panes through the
applyClamphelper, and includes a convenient reset feature with Enter/Space keys.
89-125: LGTM!The size initialization effect properly validates
initialSizesfor negative, non-finite, and zero-sum values, falling back to equal distribution with helpful warnings when needed.
128-152: LGTM!The responsive direction effect properly guards against SSR by checking for
windowexistence, and smartly defers direction changes while dragging to avoid disruptive layout shifts.
src/lib/splitpane/Divider.svelte
Outdated
| class: className = '' | ||
| }: DividerProps = $props(); | ||
|
|
||
| const themePane = getTheme("pane"); |
There was a problem hiding this comment.
Fix incorrect theme key.
Line 16 fetches the "pane" theme, but this is a divider component. This should be getTheme("divider") to retrieve the correct theme for this component.
Apply this diff:
- const themePane = getTheme("pane");
+ const themeDivider = getTheme("divider");
const themeDividerHitArea = getTheme("dividerHitArea");And update line 29:
- class={divider({ direction, isDragging, class: clsx(themePane, className) })}
+ class={divider({ direction, isDragging, class: clsx(themeDivider, className) })}📝 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.
| const themePane = getTheme("pane"); | |
| const themeDivider = getTheme("divider"); |
🤖 Prompt for AI Agents
In src/lib/splitpane/Divider.svelte around line 16, change the theme key and
variable to match this divider component: replace the current const themePane =
getTheme("pane"); with a call to getTheme("divider") (e.g., const themeDivider =
getTheme("divider");) and on line 29 update any reference to themePane to
themeDivider so the component uses the correct divider theme.
…der, Fix incorrect theme key, transition to create a separate state variable
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/lib/splitpane/Divider.svelte(1 hunks)src/lib/splitpane/Pane.svelte(1 hunks)src/lib/splitpane/SplitPane.svelte(1 hunks)src/lib/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/splitpane/Divider.svelte
- src/lib/types.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-08-31T15:34:05.291Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1745
File: src/lib/forms/checkbox/Checkbox.svelte:22-39
Timestamp: 2025-08-31T15:34:05.291Z
Learning: In Svelte 5, `$state` provides deep reactivity, meaning array mutations like `push()`, `splice()`, `pop()`, etc. trigger reactivity automatically. Unlike previous Svelte versions, immutable updates (spread operator, filter) are not required for `$state` arrays to trigger reactive updates.
Applied to files:
src/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-09-04T10:33:41.720Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1751
File: src/lib/tabs/TabItem.svelte:33-33
Timestamp: 2025-09-04T10:33:41.720Z
Learning: In Svelte 5, DOM event handlers should use function props like `onclick`, `onchange`, `oninput` etc. The `on:click`, `on:change` syntax from previous Svelte versions is deprecated/obsolete in Svelte 5.
Applied to files:
src/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-08-31T14:31:57.582Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1745
File: src/lib/forms/checkbox/CheckboxButton.svelte:16-17
Timestamp: 2025-08-31T14:31:57.582Z
Learning: In Svelte 5, the `on:event` syntax for event forwarding is obsolete. Event handlers are passed as callback props (e.g., `onchange`, `oninput`) through `{...restProps}` instead of using `on:change`, `on:input` etc.
Applied to files:
src/lib/splitpane/SplitPane.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
…p value instead of unconditionally enabling
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/splitpane/SplitPane.svelte (1)
235-235: Consider making keyboard step size configurable.The hardcoded 2% step might not suit all use cases—some users may prefer finer or coarser adjustments. Consider adding a
keyboardStepprop with a default of 2.Apply this diff to add configurability:
let { direction = "horizontal", minSize = 100, responsive = true, breakpoint = 768, transition: transitionProp = true, transitionDuration = 150, + keyboardStep = 2, initialSizes, onResize, children, class: className = "" }: SplitPaneProps = $props();Then use
keyboardStepinstead of the hardcoded value:- const step = 2; // percentage + const step = keyboardStep; // percentage
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/splitpane/Pane.svelte(1 hunks)src/lib/splitpane/SplitPane.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/splitpane/Pane.svelte
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-08-31T15:34:05.291Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1745
File: src/lib/forms/checkbox/Checkbox.svelte:22-39
Timestamp: 2025-08-31T15:34:05.291Z
Learning: In Svelte 5, `$state` provides deep reactivity, meaning array mutations like `push()`, `splice()`, `pop()`, etc. trigger reactivity automatically. Unlike previous Svelte versions, immutable updates (spread operator, filter) are not required for `$state` arrays to trigger reactive updates.
Applied to files:
src/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-09-04T10:33:41.720Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1751
File: src/lib/tabs/TabItem.svelte:33-33
Timestamp: 2025-09-04T10:33:41.720Z
Learning: In Svelte 5, DOM event handlers should use function props like `onclick`, `onchange`, `oninput` etc. The `on:click`, `on:change` syntax from previous Svelte versions is deprecated/obsolete in Svelte 5.
Applied to files:
src/lib/splitpane/SplitPane.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/lib/splitpane/SplitPane.svelte (8)
1-24: LGTM! Clean context API setup.The module-scoped context definition with a Symbol key and typed interface follows Svelte 5 best practices. The exported
setSplitPaneContextandgetSplitPaneContextfunctions provide a clean API for child components.
32-53: Props and state setup correctly addresses previous feedback.The separation of
transitionProp(immutable prop) fromtransition(local mutable state) at lines 37 and 45 properly resolves the Svelte 5 props mutation issue flagged in previous reviews. All state variables correctly use$state().
92-128: Robust size initialization with comprehensive validation.The validation logic at lines 100-107 and 112-116 properly addresses previous feedback by:
- Detecting negative and non-finite values
- Falling back to equal distribution for invalid input
- Warning users about problematic configurations
This prevents invalid CSS percentages from reaching the DOM.
131-155: SSR-safe responsive behavior with proper cleanup.The SSR guard at lines 137-140 correctly addresses previous feedback, preventing
ReferenceErrorduring server-side rendering. The logic to defer direction changes during active dragging (line 146) provides good UX, and the cleanup in the return statement properly removes the event listener.
180-188: Correctly restores user's transition preference.Line 182 properly restores
transition = transitionPropinstead of unconditionally enabling transitions, addressing previous feedback and respecting the user's configuration.
190-230: LGTM! Robust resize implementation with proper constraints.The bidirectional clamping at lines 215-222 ensures both panes respect the minimum size, the delta threshold at line 196 prevents jittery updates, and the significant-change check at line 225 avoids unnecessary renders. The logic correctly maintains the total size across both panes.
232-277: Keyboard resize correctly enforces minimum constraints.The
applyClamphelper at lines 247-261 properly addresses previous feedback by ensuring neither pane drops belowminPercentduring keyboard resizing. The bidirectional clamping and change threshold prevent constraint violations.
280-282: LGTM! Clean template with proper bindings.The container binding enables resize calculations, theme integration follows the project pattern, and the children render syntax is correct for Svelte 5.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/splitpane/SplitPane.svelte (1)
63-74: Consider explicit undefined check instead of falsy check.Line 64 uses a falsy check that conflates
0(a valid collapsed pane size) withundefined(invalid index). While in practice theminPercentenforcement prevents zero values, an explicit check would be clearer and more correct:- if (!sizes[index]) return ""; + if (sizes[index] === undefined) return "";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/splitpane/SplitPane.svelte(1 hunks)src/lib/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/types.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-08-31T15:34:05.291Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1745
File: src/lib/forms/checkbox/Checkbox.svelte:22-39
Timestamp: 2025-08-31T15:34:05.291Z
Learning: In Svelte 5, `$state` provides deep reactivity, meaning array mutations like `push()`, `splice()`, `pop()`, etc. trigger reactivity automatically. Unlike previous Svelte versions, immutable updates (spread operator, filter) are not required for `$state` arrays to trigger reactive updates.
Applied to files:
src/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-09-04T10:33:41.720Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1751
File: src/lib/tabs/TabItem.svelte:33-33
Timestamp: 2025-09-04T10:33:41.720Z
Learning: In Svelte 5, DOM event handlers should use function props like `onclick`, `onchange`, `oninput` etc. The `on:click`, `on:change` syntax from previous Svelte versions is deprecated/obsolete in Svelte 5.
Applied to files:
src/lib/splitpane/SplitPane.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 (9)
src/lib/splitpane/SplitPane.svelte (9)
1-24: LGTM: Clean context API design.The module-level context API with Symbol key and typed interface follows Svelte 5 best practices. The exported functions provide a clean API for child components.
32-54: Props and state properly structured.The destructuring with
transition: transitionPropand separate$state(transitionProp)correctly addresses the Svelte 5 read-only props constraint. Note that the localtransitionstate is initialized once from the prop and won't react to subsequent prop changes—this is likely intentional since transition is typically configured at component creation.
81-90: LGTM: Context properly configured.The context object correctly implements the
SplitPaneContextinterface, providing all necessary methods for child components. The fallback value of 50 ingetPaneSizeis a reasonable default before sizes are initialized.
93-129: LGTM: Robust size initialization with comprehensive validation.The effect properly validates
initialSizesfor negative, non-finite, and zero-sum values, falling back to equal distribution with helpful warnings. This addresses the past review concerns about invalid input handling.
132-156: LGTM: Responsive handling with proper SSR guard.The effect correctly guards against SSR by checking
typeof window === 'undefined'before accessingwindow.matchMedia. The deferred direction switch during drag (lines 147-149) provides good UX by avoiding jarring mid-resize changes. Event cleanup is properly handled.
159-163: LGTM: Parent notification properly implemented.Simple effect that correctly propagates size changes to the parent via the optional
onResizecallback.
165-231: LGTM: Resize logic correctly implemented with proper constraints.The drag resize handlers properly:
- Disable transitions during drag and restore the original prop value on completion (line 183)
- Clamp both panes to respect
minPercentconstraints (lines 216-223)- Filter out sub-pixel jitter (line 197)
- Clean up document body styles and event listeners
233-278: LGTM: Keyboard navigation with proper minimum size enforcement.The keyboard resize handler correctly enforces
minPercentconstraints on both panes via theapplyClamphelper (lines 248-262), addressing the past review concern. Arrow keys adjust sizes while respecting minimums, and Enter/Space provide a convenient reset to equal distribution.
281-283: LGTM: Clean template with Svelte 5 syntax.The template correctly uses
bind:thisfor the container reference and{@render children()}for Svelte 5 snippet rendering. The theme function integration is properly structured.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/splitpane/SplitPane.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-08-31T15:34:05.291Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1745
File: src/lib/forms/checkbox/Checkbox.svelte:22-39
Timestamp: 2025-08-31T15:34:05.291Z
Learning: In Svelte 5, `$state` provides deep reactivity, meaning array mutations like `push()`, `splice()`, `pop()`, etc. trigger reactivity automatically. Unlike previous Svelte versions, immutable updates (spread operator, filter) are not required for `$state` arrays to trigger reactive updates.
Applied to files:
src/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-09-04T10:33:41.720Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1751
File: src/lib/tabs/TabItem.svelte:33-33
Timestamp: 2025-09-04T10:33:41.720Z
Learning: In Svelte 5, DOM event handlers should use function props like `onclick`, `onchange`, `oninput` etc. The `on:click`, `on:change` syntax from previous Svelte versions is deprecated/obsolete in Svelte 5.
Applied to files:
src/lib/splitpane/SplitPane.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
…against zero container size to prevent invalid percentage calculations, Guard against zero container size in keyboard handler
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/splitpane/SplitPane.svelte (1)
56-56: Consider syncing local transition state with prop changes.The local
transitionstate is initialized fromtransitionPropbut doesn't sync if the prop changes later (while not dragging). If the parent updatestransitionPropdynamically, the local state won't reflect it until the next drag operation ends.You could add a reactive sync:
let transition = $state(transitionProp); + +$effect(() => { + if (!isDragging) { + transition = transitionProp; + } +});This ensures the local state syncs with prop changes when not actively dragging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/splitpane/SplitPane.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-08-31T15:34:05.291Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1745
File: src/lib/forms/checkbox/Checkbox.svelte:22-39
Timestamp: 2025-08-31T15:34:05.291Z
Learning: In Svelte 5, `$state` provides deep reactivity, meaning array mutations like `push()`, `splice()`, `pop()`, etc. trigger reactivity automatically. Unlike previous Svelte versions, immutable updates (spread operator, filter) are not required for `$state` arrays to trigger reactive updates.
Applied to files:
src/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-09-04T10:33:41.720Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1751
File: src/lib/tabs/TabItem.svelte:33-33
Timestamp: 2025-09-04T10:33:41.720Z
Learning: In Svelte 5, DOM event handlers should use function props like `onclick`, `onchange`, `oninput` etc. The `on:click`, `on:change` syntax from previous Svelte versions is deprecated/obsolete in Svelte 5.
Applied to files:
src/lib/splitpane/SplitPane.svelte
🔇 Additional comments (7)
src/lib/splitpane/SplitPane.svelte (7)
1-24: LGTM: Clean context API design.The module-scoped context API using a Symbol key and typed interface follows Svelte best practices. The exported functions provide a clean public API for child components.
32-54: LGTM: Props validated correctly.Props follow Svelte 5 patterns with proper destructuring and defaults. The numeric validation for
minSizeandkeyboardStepprevents invalid configurations with helpful warnings.
91-139: LGTM: Context and initialization logic are solid.The context setup provides all required functions to child components. The size initialization includes comprehensive validation (negative values, non-finite, zero-sum) with appropriate fallbacks. The equal distribution logic is correct.
142-166: LGTM: Responsive direction handling is well-implemented.The SSR guard prevents server-side errors, media query cleanup is proper, and deferring direction changes during drag provides good UX. The logic correctly toggles between horizontal/vertical based on breakpoint.
169-244: LGTM: Resize handlers are robust.The onResize notification properly guards against empty arrays. The drag handlers correctly manage cursor state, prevent text selection, and clean up event listeners. The resize logic includes:
- Zero container size guards (preventing division by zero)
- Min constraint clamping for both panes
- Significant change threshold to reduce churn
All critical issues from previous reviews have been addressed.
246-294: LGTM: Keyboard resize handler properly constrains panes.The keyboard handler includes:
- Zero container size guard
applyClamphelper that ensures both panes respectminPercent- Directional arrow keys with proper orientation logic
- Enter/Space reset to equal distribution
- Selective
preventDefaultonly when key is handledAll critical min-constraint violations from previous reviews have been resolved.
297-299: LGTM: Template is clean and minimal.The template correctly binds the container reference, applies theming with the current direction, and renders child panes. No issues found.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/splitpane/SplitPane.svelte (2)
358-401: Consider adding bounds validation for the index parameter.The
resizefunction accessessizes[index]andsizes[index + 1]without validating that these indices exist. While the calling code may guarantee valid indices throughshouldRenderDivider, adding defensive checks would prevent potential runtime errors if the contract is violated.function resize(e: MouseEvent, index: number) { if (!isDragging || !container) return; + if (index < 0 || index + 1 >= sizes.length) return; const currentPos = currentDirection === "horizontal" ? e.clientX : e.clientY;
403-451: Consider adding bounds validation for the index parameter.Similar to the
resizefunction,handleKeyResizeaccessessizes[index]andsizes[index + 1]without validation. Adding a defensive check would improve robustness.function handleKeyResize(e: KeyboardEvent, index: number) { if (!container) return; + if (index < 0 || index + 1 >= sizes.length) return; const step = keyboardStep;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/splitpane/SplitPane.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-08-31T15:34:05.291Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1745
File: src/lib/forms/checkbox/Checkbox.svelte:22-39
Timestamp: 2025-08-31T15:34:05.291Z
Learning: In Svelte 5, `$state` provides deep reactivity, meaning array mutations like `push()`, `splice()`, `pop()`, etc. trigger reactivity automatically. Unlike previous Svelte versions, immutable updates (spread operator, filter) are not required for `$state` arrays to trigger reactive updates.
Applied to files:
src/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-09-04T10:33:41.720Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1751
File: src/lib/tabs/TabItem.svelte:33-33
Timestamp: 2025-09-04T10:33:41.720Z
Learning: In Svelte 5, DOM event handlers should use function props like `onclick`, `onchange`, `oninput` etc. The `on:click`, `on:change` syntax from previous Svelte versions is deprecated/obsolete in Svelte 5.
Applied to files:
src/lib/splitpane/SplitPane.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 (6)
src/lib/splitpane/SplitPane.svelte (6)
1-24: LGTM: Clean context API setup.The module-level context API is well-structured, using a Symbol key to prevent collisions and providing a clear interface for child components.
26-54: LGTM: Props properly validated.The props follow Svelte 5 conventions, with appropriate validation for numeric constraints and the
transitionprop renamed to avoid mutation issues.
102-118: LGTM: Proper ResizeObserver usage.The container size tracking correctly uses ResizeObserver with proper cleanup.
298-323: LGTM: Responsive behavior properly guarded.The responsive direction handling correctly guards against SSR window access and defers direction changes during dragging.
325-330: LGTM: Resize notification works correctly.The effect properly notifies the parent when sizes change.
454-456: LGTM: Clean template structure.The template properly binds the container reference and renders children.
…t with the actual distribution, Remove duplicate size initialization effect
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/splitpane/SplitPane.svelte (1)
101-110: Guard against division by zero ingetPaneSizefallback.Line 104's fallback
(100 / registeredPanes)returnsInfinityif called before any panes register. While unlikely in normal component lifecycle, defensive coding would improve robustness.Consider this guard:
getPaneSize: (index: number) => sizes[index] ?? (100 / registeredPanes), + getPaneSize: (index: number) => sizes[index] ?? (registeredPanes > 0 ? 100 / registeredPanes : 0),Alternatively, return a sensible default like
0or50whenregisteredPanesis zero.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/splitpane/SplitPane.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 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/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-08-31T15:34:05.291Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1745
File: src/lib/forms/checkbox/Checkbox.svelte:22-39
Timestamp: 2025-08-31T15:34:05.291Z
Learning: In Svelte 5, `$state` provides deep reactivity, meaning array mutations like `push()`, `splice()`, `pop()`, etc. trigger reactivity automatically. Unlike previous Svelte versions, immutable updates (spread operator, filter) are not required for `$state` arrays to trigger reactive updates.
Applied to files:
src/lib/splitpane/SplitPane.svelte
📚 Learning: 2025-09-04T10:33:41.720Z
Learnt from: jjagielka
Repo: themesberg/flowbite-svelte PR: 1751
File: src/lib/tabs/TabItem.svelte:33-33
Timestamp: 2025-09-04T10:33:41.720Z
Learning: In Svelte 5, DOM event handlers should use function props like `onclick`, `onchange`, `oninput` etc. The `on:click`, `on:change` syntax from previous Svelte versions is deprecated/obsolete in Svelte 5.
Applied to files:
src/lib/splitpane/SplitPane.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 (6)
src/lib/splitpane/SplitPane.svelte (6)
1-24: LGTM! Clean context API design.The module script properly defines the context key as a Symbol and exports type-safe context setters/getters. This is idiomatic Svelte 5 and provides a clean public API for child components.
32-58: LGTM! Props are properly validated and immutability is preserved.The prop validation correctly guards against non-positive values for
minSizeandkeyboardStep. Renamingtransitionprop totransitionProp(line 37) properly addresses the Svelte 5 props immutability requirement.
131-222: LGTM! Comprehensive size initialization with robust validation.The initialization effect properly handles multiple edge cases:
- Zero/invalid container dimensions (lines 138-144)
- Unachievable
minSizeconstraints (lines 149-157)- Invalid
initialSizesvalues (lines 179-195)- minSize constraint violations with clamping and renormalization (lines 199-211)
The logic is defensive and well-structured. Previous issues with duplicate effects and validation gaps have been properly addressed.
225-249: LGTM! Responsive behavior is well-implemented with proper SSR guard.The SSR check (lines 231-234) correctly prevents
window.matchMediaaccess during server-side rendering. Deferring direction changes during active drag (line 240) provides smooth UX. Cleanup is properly handled.
258-379: LGTM! Resize handlers are robust with proper constraint enforcement.Both mouse and keyboard resize handlers demonstrate excellent defensive coding:
- Zero container size guards (lines 296, 343) prevent division-by-zero errors
- Symmetric clamping logic (lines 313-320, 349-363) ensures both panes respect
minPercentconstraints- Transition state is properly restored to the prop value (line 276)
- Thresholds prevent jitter from micro-movements (lines 291, 323, 358)
All previously identified critical issues with keyboard resizing and container sizing have been properly addressed.
382-384: LGTM! Clean and minimal template.The template correctly binds the container reference, applies theme classes, and renders children using Svelte 5's
{@render}syntax.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/docs-examples/extend/split-pane/+page.svelte(1 hunks)
⏰ 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
1 participant
📑 Description
New feature: Split pane
Status
✅ Checks
api-checkdirectory as requiredpnpm check && pnpm test:e2eℹ Additional Information
Summary by CodeRabbit
New Features
Documentation
Tests