refactor: dnd overhaul#196
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the split-view system and drag-and-drop implementation to address unreliable DnD behavior (Issue #182), consolidating split UI into reusable components and migrating from @dnd-kit-svelte/svelte to @dnd-kit/svelte.
Changes:
- Migrates DnD integration to
@dnd-kit/svelteand updates drag source/target data modeling for pinned items and split panes. - Reworks split view routing/UX by removing the dedicated
/channels/splitroute and rendering split panes via shared components ($lib/components/split/*). - Simplifies/updates split restore & close behavior settings and related startup navigation logic.
Reviewed changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/settings/categories/splits.ts | Removes “single split restore” setting and keeps split-related controls focused on close behavior. |
| src/routes/(main)/channels/split/SplitView.svelte | Deletes old route-local split pane implementation. |
| src/routes/(main)/channels/split/+page.ts | Removes split route loader/redirect logic. |
| src/routes/(main)/channels/split/+page.svelte | Removes split route UI (node/view rendering + keyboard nav). |
| src/routes/(main)/channels/+layout.svelte | Updates keyboard shortcuts to operate on the new split model without “activate split route”. |
| src/routes/(main)/channels/[username]/+page.ts | Ensures the visited channel is present in the split layout. |
| src/routes/(main)/channels/[username]/+page.svelte | Renders split layout within the channel route and reintroduces split keyboard navigation. |
| src/routes/(main)/+page.svelte | Updates initial navigation/restore logic to route into a channel instead of a split route. |
| src/routes/(main)/+layout.svelte | Replaces provider/plugins and refines drag handling + overlay rendering for new DnD model. |
| src/lib/stores.ts | Simplifies layout backend-sync behavior now that single-split restore options are removed. |
| src/lib/split-layout.ts | Refactors split layout API (ensure/empty ids) and rewrites drag-end handling using typed source/target data. |
| src/lib/settings.ts | Removes splits.singleRestoreBehavior from schema/defaults. |
| src/lib/menus/channel-menu.ts | Updates split/leave behaviors for new split model; removes “open split view” route action. |
| src/lib/context.ts | Deletes sidebar context in favor of global app state. |
| src/lib/components/ui/ColorPicker.svelte | Minor CSS formatting change. |
| src/lib/components/TitleBar.svelte | Simplifies titlebar icon handling (component-only). |
| src/lib/components/StreamTooltip.svelte | Switches sidebar item navigation from <a> to imperative goto() and uses app.sidebarCollapsed. |
| src/lib/components/StreamInfo.svelte | Uses app.sidebarCollapsed instead of context. |
| src/lib/components/split/SplitView.svelte | Adds new shared split pane component using @dnd-kit/svelte. |
| src/lib/components/split/SplitNode.svelte | Adds new recursive layout renderer for split tree using paneforge panes. |
| src/lib/components/split/SplitHeader.svelte | Updates header interactions (close/preserve, drag handle) and moves guest list display here. |
| src/lib/components/Sortable.svelte | Migrates pinned sortable to @dnd-kit/svelte/sortable with new type/data fields. |
| src/lib/components/Sidebar.svelte | Replaces context-based collapse state with app.sidebarCollapsed; removes split-view button. |
| src/lib/components/Droppable.svelte | Removes old droppable wrapper component used by the previous DnD integration. |
| src/lib/components/Draggable.svelte | Migrates channel list draggable to @dnd-kit/svelte with typed data. |
| src/lib/components/ChannelListItem.svelte | Updates DnD attachment API; adds (currently unused) placeholder prop. |
| src/lib/components/ChannelList.svelte | Removes pinned droppable wrapper and swaps to app.sidebarCollapsed. |
| src/lib/app.svelte.ts | Adds global sidebarCollapsed state to the app model. |
| src/app.d.ts | Tightens TitleBar.icon type to component-only. |
| pnpm-lock.yaml | Updates lockfile for new DnD package and dependency graph changes. |
| package.json | Replaces @dnd-kit-svelte/svelte with @dnd-kit/svelte in dependencies. |
| .vscode/settings.json | Updates formatter/save actions and TypeScript SDK configuration. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
src/lib/split-layout.ts:114
insert()returns early whenthis.rootis null by settingthis.root = targetand not performing the insert. Callers likeinsertEmpty()then setfocusedto the new pane id, which can leavefocusedpointing to a pane that is not present in the tree (and no new split is actually created). Consider handling the empty-tree case by settingrootto the constructed branch (or explicitly erroring if inserts are not valid without an existing root), soinsertEmpty()always produces a 2-leaf layout.
public insert(target: string, newNode: string, branch: SplitBranch) {
if (!this.root) {
this.root = target;
return;
}
src/lib/components/split/SplitHeader.svelte:34
- Root-split close flow here now has a "preserve" path that replaces the root with an empty pane, but the non-preserve path later clears
app.splits.rootentirely. Given the channel route now renders a split layout (not a standaloneChannelview), clearing the root can leave/channels/[username]showing an empty pane after closing the last split and makessplits.goToChannelAfterClosehard to satisfy. Consider keeping a valid root pane for the current channel, or rendering the non-split channel UI whenrootis null.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #182