fix(desktop): remove viewed checkboxes from v2 diff sidebar#3480
Conversation
The sidebar file list no longer shows checkboxes for marking files as viewed. Removes the Checkbox component, viewed/onSetViewed props, and the partitionByViewed utility from the changes sidebar pipeline.
📝 WalkthroughWalkthroughRemoved per-file "viewed" tracking (props, checkbox UI, and partitioning util) so change lists render in original order; added persisted sidebar state with a combined "changes" tab that has internal "diffs"/"review" subtabs, responsive compact header support, and an optional icon on tab definitions. Changes
Sequence Diagram(s)sequenceDiagram
participant Sidebar as WorkspaceSidebar
participant Store as CollectionsStore
participant Header as SidebarHeader
participant Content as ChangesTabContent
participant List as ChangesFileList
Sidebar->>Store: read workspaceLocalState (activeTab, changesSubtab)
Note right of Store: persisted UI state
Sidebar->>Header: render(compact, tabs, activeTab)
Sidebar->>Content: render(selected subtab content)
Content->>List: render(files)
List-->>Content: user selects file (onSelect)
Content-->>Sidebar: notify selection
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 docstrings
🧪 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 |
Greptile SummaryThis PR removes the "viewed" file-tracking checkbox feature from the v2 diff sidebar file list by propagating the prop deletion cleanly up the component chain ( Key changes:
Confidence Score: 5/5Safe to merge — clean, well-scoped removal of a UI feature with no logic gaps or regressions. All prop removals are consistent across the full component chain, the useViewedFiles hook is correctly retained where still needed (DiffPane), the nested-button accessibility issue is fixed as a side-effect, and the deleted partitionByViewed utility has no remaining consumers. No new code paths are introduced and no state is orphaned. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Before["Before"]
A1[useChangesTab] -->|viewedSet, setViewed| B1[ChangesTabContent]
B1 -->|viewedSet, onSetViewed| C1[ChangesFileList]
C1 -->|partitionByViewed sort| D1[files array]
D1 -->|viewed, onSetViewed| E1[FileRow]
E1 --> F1[Checkbox + button nested inside div]
end
subgraph After["After"]
A2[useChangesTab] --> B2[ChangesTabContent]
B2 --> C2[ChangesFileList]
C2 --> D2[files array - original order]
D2 --> E2[FileRow]
E2 --> F2[Single button element]
end
subgraph Retained["useViewedFiles still used in"]
G[DiffPane.tsx] -->|viewedSet, setViewed| H[DiffFileEntry per file]
end
Reviews (1): Last reviewed commit: "fix(desktop): remove viewed checkboxes f..." | Re-trigger Greptile |
…nges Match the v1 sidebar layout: top-level tabs are Changes (GitCompareArrows icon) and Files (FileText icon). Review is now a subtab within Changes alongside Diffs, using the same Tabs/TabsTrigger pattern as v1.
Use LuGitCompareArrows and LuFile from react-icons/lu (same as v1), getSidebarHeaderTabButtonClassName for the top-level tab buttons, and sidebarHeaderTabTriggerClassName for the Diffs/Review subtabs — matching the v1 sidebar layout and styling exactly.
Make the tab content wrapper a flex column so TabsContent can fill the available height, allowing the "Open a pull request" message to center vertically instead of sticking to the top.
When the sidebar width drops below 250px, the Changes and Files tabs collapse to icon-only buttons with tooltips, matching v1 behavior. Uses the same getSidebarHeaderTabButtonClassName compact mode.
Store activeTab (Changes/Files) and changesSubtab (Diffs/Review) in the workspace local state collection so the sidebar reopens to the same tab the user last had open.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts (1)
40-41: Consider usingz.enumforactiveTabfor stricter validation.
changesSubtabcorrectly usesz.enum(["diffs", "review"]), butactiveTabusesz.string()which accepts any value. Usingz.enum(["changes", "files"])would catch invalid tab IDs early and align with the constrained approach used forchangesSubtab.That said,
z.string()is more flexible if you anticipate adding tabs without schema migrations—this is a reasonable trade-off.♻️ Optional: Constrain activeTab to known values
- activeTab: z.string().default("changes"), + activeTab: z.enum(["changes", "files"]).default("changes"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts` around lines 40 - 41, Replace the loose z.string() schema for activeTab with a z.enum that matches the expected tab IDs to enforce stricter validation: update the activeTab schema (next to changesSubtab) to use z.enum(["changes","files"]).default("changes") so invalid tab values are rejected at validation time while keeping changesSubtab as-is; if you intentionally want open-ended tabs, leave activeTab as z.string() but add a comment explaining the choice.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx (1)
28-52: Consider extractingIconButtonto its own file.Per coding guidelines, components should follow "one component per file".
IconButtonis a small helper, but if it might be reused elsewhere, consider moving it toIconButton/IconButton.tsxwith a barrel export.This is a minor suggestion given it's currently only used within this file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx around lines 28 - 52, Extract the IconButton component into its own file: create IconButton/IconButton.tsx exporting the IconButton component (preserve the props signature: icon: React.ComponentType<{ className?: string }>, tooltip: string, onClick?: () => void) and keep the JSX using Tooltip, TooltipTrigger, TooltipContent and Button as-is; add a barrel export (IconButton/index.ts) that re-exports the component, then update the original file to import { IconButton } from the new barrel location where IconButton was used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx:
- Around line 28-52: Extract the IconButton component into its own file: create
IconButton/IconButton.tsx exporting the IconButton component (preserve the props
signature: icon: React.ComponentType<{ className?: string }>, tooltip: string,
onClick?: () => void) and keep the JSX using Tooltip, TooltipTrigger,
TooltipContent and Button as-is; add a barrel export (IconButton/index.ts) that
re-exports the component, then update the original file to import { IconButton }
from the new barrel location where IconButton was used.
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts`:
- Around line 40-41: Replace the loose z.string() schema for activeTab with a
z.enum that matches the expected tab IDs to enforce stricter validation: update
the activeTab schema (next to changesSubtab) to use
z.enum(["changes","files"]).default("changes") so invalid tab values are
rejected at validation time while keeping changesSubtab as-is; if you
intentionally want open-ended tabs, leave activeTab as z.string() but add a
comment explaining the choice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b244d4f-ade6-4b66-9b0b-1b52c28b0b9f
📒 Files selected for processing (4)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/SidebarHeader/SidebarHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/types.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/types.ts
- Remove duplicate React key on compact tab button (SidebarHeader) - Stabilize setActiveTab/setChangesSubtab callbacks via refs so they don't bust the combinedChangesTab useMemo on every collection update - Narrow activeTab schema from z.string() to z.enum(["changes","files"]) with a runtime guard in the setter
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts:40">
P2: Make `activeTab` backward-compatible for persisted data by coercing unknown values to a safe default instead of hard-failing validation.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| sectionId: z.string().uuid().nullable().default(null), | ||
| changesFilter: changesFilterSchema.default({ kind: "all" }), | ||
| baseBranch: z.string().nullable().default(null), | ||
| activeTab: z.enum(["changes", "files"]).default("changes"), |
There was a problem hiding this comment.
P2: Make activeTab backward-compatible for persisted data by coercing unknown values to a safe default instead of hard-failing validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts, line 40:
<comment>Make `activeTab` backward-compatible for persisted data by coercing unknown values to a safe default instead of hard-failing validation.</comment>
<file context>
@@ -37,7 +37,7 @@ export const workspaceLocalStateSchema = z.object({
changesFilter: changesFilterSchema.default({ kind: "all" }),
baseBranch: z.string().nullable().default(null),
- activeTab: z.string().default("changes"),
+ activeTab: z.enum(["changes", "files"]).default("changes"),
changesSubtab: z.enum(["diffs", "review"]).default("diffs"),
}),
</file context>
| activeTab: z.enum(["changes", "files"]).default("changes"), | |
| activeTab: z.enum(["changes", "files"]).catch("changes").default("changes"), |
…orkspaceSidebar No memo boundaries downstream benefit from stable references, so plain functions and plain objects are simpler and equivalent.
# Conflicts: # apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx (2)
143-161: Badge visibility logic differs between Diffs and Review.The Diffs badge renders when
changesTab.badge != null(including zero), while the Review badge requiresreviewTab.badge != null && reviewTab.badge > 0. This means a "0" badge would show for Diffs but not for Review.If intentional (e.g., always showing diff count), consider adding a brief comment. Otherwise, align the conditions for consistency.
Option to align badge logic
- {changesTab.badge != null && ( + {changesTab.badge != null && changesTab.badge > 0 && ( <span className="text-[11px] text-muted-foreground/60 tabular-nums"> {changesTab.badge} </span> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx around lines 143 - 161, In WorkspaceSidebar (inside the TabsTrigger for "Diffs" and "Review") the badge visibility logic is inconsistent: the Diffs badge checks changesTab.badge != null (showing zero), while Review uses reviewTab.badge != null && reviewTab.badge > 0 (hides zero). Make them consistent by choosing one rule and applying it to both triggers (either show when badge != null or show only when badge > 0); update the checks on changesTab.badge and reviewTab.badge accordingly and add a brief inline comment next to the chosen condition in WorkspaceSidebar to indicate that displaying zero is intentional if you opt to keep that behavior.
28-52: Consider extractingIconButtonto its own file.The coding guidelines suggest keeping one component per file.
IconButtoncould be co-located in a separate file within this component's folder (e.g.,IconButton/IconButton.tsx) or moved to a shared location if reused elsewhere.This is a minor suggestion given the component's small size and single usage.
As per coding guidelines: "/components//*.tsx: Keep one component per file; no multi-component files"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx around lines 28 - 52, The IconButton inner component should be extracted into its own file to follow the "one component per file" rule: create a new component file (e.g., IconButton.tsx next to WorkspaceSidebar) that exports the IconButton function/component with the same props and JSX, preserve its prop types (icon: React.ComponentType<{ className?: string }>, tooltip: string, onClick?: () => void) and keep the same usage of Tooltip, TooltipTrigger, TooltipContent and Button; then replace the local IconButton declaration in WorkspaceSidebar.tsx with an import of the new IconButton and ensure any necessary imports (Tooltip, TooltipTrigger, TooltipContent, Button, React) are present in the new file and the parent file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx:
- Around line 143-161: In WorkspaceSidebar (inside the TabsTrigger for "Diffs"
and "Review") the badge visibility logic is inconsistent: the Diffs badge checks
changesTab.badge != null (showing zero), while Review uses reviewTab.badge !=
null && reviewTab.badge > 0 (hides zero). Make them consistent by choosing one
rule and applying it to both triggers (either show when badge != null or show
only when badge > 0); update the checks on changesTab.badge and reviewTab.badge
accordingly and add a brief inline comment next to the chosen condition in
WorkspaceSidebar to indicate that displaying zero is intentional if you opt to
keep that behavior.
- Around line 28-52: The IconButton inner component should be extracted into its
own file to follow the "one component per file" rule: create a new component
file (e.g., IconButton.tsx next to WorkspaceSidebar) that exports the IconButton
function/component with the same props and JSX, preserve its prop types (icon:
React.ComponentType<{ className?: string }>, tooltip: string, onClick?: () =>
void) and keep the same usage of Tooltip, TooltipTrigger, TooltipContent and
Button; then replace the local IconButton declaration in WorkspaceSidebar.tsx
with an import of the new IconButton and ensure any necessary imports (Tooltip,
TooltipTrigger, TooltipContent, Button, React) are present in the new file and
the parent file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 77bfc545-91b2-49fc-a8a3-c46c63e87204
📒 Files selected for processing (4)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesTabContent/ChangesTabContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsxapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts
💤 Files with no reviewable changes (2)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesTabContent/ChangesTabContent.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
viewedSet/onSetViewedprops through the component chain (FileRow→ChangesFileList→ChangesTabContent→useChangesTab)partitionByViewedutilityTest plan
Summary by cubic
Removed the “viewed” checkboxes and updated the v2 diff sidebar to match v1. Changes (default) and Files are the top tabs; Review now lives under Changes with Diffs, uses v1 icons (
LuGitCompareArrows,LuFile), compacts to icon-only under 200px with tooltips, centers the empty Review state, and the sidebar remembers your last tab and subtab.viewedSet/onSetViewedand thepartitionByViewedutility;FileRowis now a single clickable row.getSidebarHeaderTabButtonClassNameandsidebarHeaderTabTriggerClassName; added count badges on Diffs/Review.activeTabandchangesSubtabin the workspace local state schema; lowered the compact breakpoint to 200px; simplifiedWorkspaceSidebar(removed unnecessary memoization) and fixed a duplicate key in compact mode.Written for commit 3207f8f. Summary will update on new commits.
Summary by CodeRabbit
Refactor
New Features