feat(desktop): improve changes sidebar header UI#1110
Conversation
📝 WalkthroughWalkthroughThe pull request updates state management in InfiniteScrollView by replacing local expansion state with store-provided state, adds right borders to component styling in CategoryHeader and DiffToolbar, and restructures the ChangesHeader UI from a Select-based branch selector to a DropdownMenu-based implementation with reorganized header controls. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx`:
- Around line 57-74: The base-branch Button (the icon-only branch selector
inside ChangesHeader using DropdownMenuTrigger/DropdownMenu and the Button
component) lacks an accessible label; add an aria-label (e.g., "Select base
branch" or similar descriptive text) to that Button element so screen readers
can announce its purpose, and ensure the Tooltip content still matches the
aria-label for consistency; update the JSX for the Button used in ChangesHeader
(the Button with variant="ghost" size="icon" and <LuGitBranch />) to include the
aria-label prop.
| const handleBranchSelect = (branch: string) => { | ||
| if (branch === branchData?.defaultBranch && baseBranch === null) return; | ||
| setBaseBranch(branch); | ||
| }; | ||
|
|
||
| if (isLoading || !branchData) { | ||
| return ( | ||
| <span className="px-1.5 py-0.5 rounded bg-muted/50 text-foreground text-[10px] font-medium truncate"> | ||
| {effectiveBaseBranch} | ||
| </span> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <Tooltip open={tooltipOpen}> | ||
| <Select value={effectiveBaseBranch} onValueChange={handleChange}> | ||
| <DropdownMenu> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <SelectTrigger | ||
| size="sm" | ||
| className="h-5 px-1.5 py-0 text-[10px] font-medium border-none bg-muted/50 hover:bg-muted text-foreground min-w-0 w-auto gap-0.5 rounded" | ||
| onPointerEnter={handlePointerEnter} | ||
| onPointerLeave={handlePointerLeave} | ||
| > | ||
| <SelectValue /> | ||
| </SelectTrigger> | ||
| <DropdownMenuTrigger asChild> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="size-6 p-0" | ||
| disabled={isLoading} | ||
| > | ||
| <LuGitBranch className="size-3.5" /> | ||
| </Button> |
There was a problem hiding this comment.
Add an accessible label to the icon-only base-branch button.
Icon-only controls need an aria-label for screen readers.
🔧 Suggested fix
<Button
variant="ghost"
size="icon"
className="size-6 p-0"
disabled={isLoading}
+ aria-label="Change base branch"
>📝 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 handleBranchSelect = (branch: string) => { | |
| if (branch === branchData?.defaultBranch && baseBranch === null) return; | |
| setBaseBranch(branch); | |
| }; | |
| if (isLoading || !branchData) { | |
| return ( | |
| <span className="px-1.5 py-0.5 rounded bg-muted/50 text-foreground text-[10px] font-medium truncate"> | |
| {effectiveBaseBranch} | |
| </span> | |
| ); | |
| } | |
| return ( | |
| <Tooltip open={tooltipOpen}> | |
| <Select value={effectiveBaseBranch} onValueChange={handleChange}> | |
| <DropdownMenu> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <SelectTrigger | |
| size="sm" | |
| className="h-5 px-1.5 py-0 text-[10px] font-medium border-none bg-muted/50 hover:bg-muted text-foreground min-w-0 w-auto gap-0.5 rounded" | |
| onPointerEnter={handlePointerEnter} | |
| onPointerLeave={handlePointerLeave} | |
| > | |
| <SelectValue /> | |
| </SelectTrigger> | |
| <DropdownMenuTrigger asChild> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| className="size-6 p-0" | |
| disabled={isLoading} | |
| > | |
| <LuGitBranch className="size-3.5" /> | |
| </Button> | |
| const handleBranchSelect = (branch: string) => { | |
| if (branch === branchData?.defaultBranch && baseBranch === null) return; | |
| setBaseBranch(branch); | |
| }; | |
| return ( | |
| <DropdownMenu> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| <DropdownMenuTrigger asChild> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| className="size-6 p-0" | |
| disabled={isLoading} | |
| aria-label="Change base branch" | |
| > | |
| <LuGitBranch className="size-3.5" /> | |
| </Button> |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx`
around lines 57 - 74, The base-branch Button (the icon-only branch selector
inside ChangesHeader using DropdownMenuTrigger/DropdownMenu and the Button
component) lacks an accessible label; add an aria-label (e.g., "Select base
branch" or similar descriptive text) to that Button element so screen readers
can announce its purpose, and ensure the Tooltip content still matches the
aria-label for consistency; update the JSX for the Button used in ChangesHeader
(the Button with variant="ghost" size="icon" and <LuGitBranch />) to include the
aria-label prop.
Summary
Test plan
Summary by CodeRabbit
Style
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.