feat: add all changed files panel for applied branch view #11753
Conversation
|
@nshcr is attempting to deploy a commit to the GitButler Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR adds an "All changed files" panel to applied branches in the branch tab, bringing consistency with unapplied branches which already have this feature. The change enhances user experience by providing a consistent interface across different branch states.
Key changes:
- Added a
showChangedFilesprop toBranchViewcomponent to control the visibility of the ChangedFiles panel - Integrated the ChangedFiles component rendering within BranchView when the prop is enabled
- Updated BranchesView to pass
showChangedFilesprop when displaying applied branches
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/desktop/src/components/BranchesView.svelte | Passes showChangedFiles prop to BranchView for applied branches in workspace |
| apps/desktop/src/components/BranchView.svelte | Adds optional showChangedFiles prop (defaults to false), imports necessary dependencies, creates derived state for selection and branch references, and conditionally renders ChangedFiles component |
| persistId={`unapplied-branch-${branchName}`} | ||
| changes={changes.changes} | ||
| stats={changes.stats} | ||
| allowUnselect={false} | ||
| /> | ||
| {/snippet} | ||
| </ReduxResult> |
There was a problem hiding this comment.
The persistId uses the prefix "unapplied-branch-" but this component is for applied branches. This should be changed to "applied-branch-" to accurately reflect the context and avoid confusion. The persistId is used to store UI state like scroll position and expansion states, so having accurate naming is important for debugging and maintenance.
| persistId={`unapplied-branch-${branchName}`} | |
| changes={changes.changes} | |
| stats={changes.stats} | |
| allowUnselect={false} | |
| /> | |
| {/snippet} | |
| </ReduxResult> | |
| persistId={`applied-branch-${branchName}`} | |
| changes={changes.changes} | |
| stats={changes.stats} | |
| allowUnselect={false} | |
| /> | |
| {/snippet} | |
| </ReduxResult} |
There was a problem hiding this comment.
Haha, I knew Copilot would catch this mistake :P.
I believe that when a branch transitions from unapplied to applied, it should not lose its persisted list mode state. For that reason, I reused the same persistId prefix as in the UnappliedBranchView component.
Changing the persistId prefix there would introduce backward-compatibility concerns, and since persistId are not user-visible, I think reusing the unapplied prefix for applied branches is acceptable for now.
|
Thanks so much for contributing, I love the energy! Not having looked at more than the screenshot, let me invite our HoD @PavelLaptev right away for opinions :). |
|
Thanks for the PR, @nshcr. I’ll merge it. :-) There’s one issue, though—the selected branch lacks proper styles, so it appears unselected. I can address this in the next PR. 
|
3 participants
This PR adds an all changed files preview to applied branches in the branch tab. Currently, only unapplied branches have a preview of all changed files. I'm not sure what considerations led to this behavioral difference, but in my opinion, keeping the behavior consistent here would provide a better user experience.
By the way, I noticed that the same component is also used in the stacked branch view, so I added a prop to ensure the ChangedFiles component is rendered only in the appropriate places.