Ability to drag changes to start a commit #12176
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
cc @PavelLaptev |
There was a problem hiding this comment.
Pull request overview
This PR adds the ability to drag uncommitted changes (files, folders, or hunks) to a branch card to initiate the commit process. The dropped changes are automatically staged when the commit UI opens.
Changes:
- Added
StartCommitDzHandlerclass to handle drop events for starting commits - Integrated the new drop handler into
BranchList.svelteby creating an instance for each branch - Refactored
BranchCard.svelteto use a function for determining overlay labels instead of inline ternary expressions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/desktop/src/lib/branches/dropHandler.ts | Added StartCommitDzHandler class that accepts uncommitted file/folder/hunk drops and initiates commit mode while staging the dropped changes |
| apps/desktop/src/components/BranchList.svelte | Instantiated StartCommitDzHandler for each branch and added it to the dropzones array |
| apps/desktop/src/components/BranchCard.svelte | Refactored overlay label logic into getCardOverlayLabel() function and added support for StartCommitDzHandler label |
| this.startCommitting(); | ||
| await this.checkDropData(data); |
There was a problem hiding this comment.
The order of operations may cause a race condition. The method calls startCommitting() to set the UI state to commit mode, then immediately calls checkDropData() to stage the changes. However, if the UI responds to the state change before the changes are checked/staged, users might see an empty commit form briefly. Consider awaiting checkDropData() before calling startCommitting() to ensure the changes are staged before the commit UI appears.
| this.startCommitting(); | |
| await this.checkDropData(data); | |
| await this.checkDropData(data); | |
| this.startCommitting(); |
|
|
||
| accepts(data: unknown): boolean { | ||
| if (data instanceof FileChangeDropData || data instanceof FolderChangeDropData) { | ||
| // Only accept uncomitted files/folders |
There was a problem hiding this comment.
Spelling error: "uncomitted" should be "uncommitted"
| // Only accept uncomitted files/folders | |
| // Only accept uncommitted files/folders |
| if (data instanceof FileChangeDropData || data instanceof FolderChangeDropData) { | ||
| // Only accept uncomitted files/folders | ||
| if (data.isCommitted) return false; | ||
| // Only accept unassinged files/folders or those assigned to the same stack |
There was a problem hiding this comment.
Spelling error: "unassinged" should be "unassigned"
| // Only accept unassinged files/folders or those assigned to the same stack | |
| // Only accept unassigned files/folders or those assigned to the same stack |
| ): Promise<true> { | ||
| if (data instanceof FileChangeDropData || data instanceof FolderChangeDropData) { | ||
| const changes = await data.treeChanges(); | ||
| const paths = changes.map((c) => c.path); | ||
| if (paths.length === 0) return true; | ||
| this.uncommittedService.checkFiles(data.stackId ?? null, paths); | ||
| return true; | ||
| } | ||
|
|
||
| // Handle hunk data | ||
| this.uncommittedService.checkHunk(data.stackId ?? null, data.change.path, data.hunk); | ||
| return true; |
There was a problem hiding this comment.
The return type Promise<true> is overly specific. Since this method always returns true and doesn't seem to need a return value, consider changing the return type to Promise<void> and removing the return true statements. This would make the intent clearer that this method is called for its side effects rather than its return value.
| ): Promise<true> { | |
| if (data instanceof FileChangeDropData || data instanceof FolderChangeDropData) { | |
| const changes = await data.treeChanges(); | |
| const paths = changes.map((c) => c.path); | |
| if (paths.length === 0) return true; | |
| this.uncommittedService.checkFiles(data.stackId ?? null, paths); | |
| return true; | |
| } | |
| // Handle hunk data | |
| this.uncommittedService.checkHunk(data.stackId ?? null, data.change.path, data.hunk); | |
| return true; | |
| ): Promise<void> { | |
| if (data instanceof FileChangeDropData || data instanceof FolderChangeDropData) { | |
| const changes = await data.treeChanges(); | |
| const paths = changes.map((c) => c.path); | |
| if (paths.length === 0) return; | |
| this.uncommittedService.checkFiles(data.stackId ?? null, paths); | |
| return; | |
| } | |
| // Handle hunk data | |
| this.uncommittedService.checkHunk(data.stackId ?? null, data.change.path, data.hunk); | |
| return; |
estib-vega
force-pushed
the
drop-to-stage
branch
from
627d946 to
167c163
Compare
Add a drop handler that initiates the commit process, staging the dragged changes.
estib-vega
force-pushed
the
drop-to-stage
branch
from
167c163 to
75532c8
Compare
| // Only accept uncomitted files/folders | ||
| if (data.isCommitted) return false; | ||
| // Only accept unassinged files/folders or those assigned to the same stack |
There was a problem hiding this comment.
There are two spelling errors in the comments: "uncomitted" should be "uncommitted" and "unassinged" should be "unassigned".
| // Only accept uncomitted files/folders | |
| if (data.isCommitted) return false; | |
| // Only accept unassinged files/folders or those assigned to the same stack | |
| // Only accept uncommitted files/folders | |
| if (data.isCommitted) return false; | |
| // Only accept unassigned files/folders or those assigned to the same stack |
PavelLaptev
left a comment
PavelLaptev
left a comment
There was a problem hiding this comment.
Look good, thank you! :-)
Add a drop handler that initiates the commit process, staging the
dragged changes.
Screen.Recording.2026-02-03.at.14.58.01.mov
This is part 1 of 2 in a stack made with GitButler: