Skip to content

fix(desktop): clean up review sidebar UI and remove redundant states#2989

Merged
Kitenite merged 2 commits into
mainfrom
clean-up-review-sidebar-ui-and-remove-redundant-st
Mar 29, 2026
Merged

fix(desktop): clean up review sidebar UI and remove redundant states#2989
Kitenite merged 2 commits into
mainfrom
clean-up-review-sidebar-ui-and-remove-redundant-st

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Mar 29, 2026

Summary

  • Replace inline AlertDialogs with reusable DiscardConfirmDialog component (-80 lines of duplication)
  • Flatten comment layout: active comments shown directly under Comments header, Resolved as its own collapsible section
  • Remove redundant states: duplicate comment counts, PR state text already conveyed by icon color, review label duplicating badge text
  • Align collapsible trigger styles (padding, hover, chevron sizes) with CategorySection pattern used in Diffs tab
  • Compact PR header with hover-to-reveal external link, section dividers, and simplified loading state

Test plan

  • Open a workspace with a linked PR and verify Review tab renders correctly
  • Verify Checks section shows status and is collapsible
  • Verify Comments section shows active comments directly (no nested "Open" group)
  • Verify Resolved section appears as standalone collapsible when resolved comments exist
  • Verify hover actions (external link, copy) work on comments
  • Verify PR header external link appears on hover
  • Verify discard dialogs still work for staged/unstaged changes

Summary by cubic

Cleaned up the Review sidebar UI and removed redundant states to make the panel clearer and consistent with the Diffs tab. Simplifies the PR header, checks, and comments with flatter grouping and fewer labels.

  • Refactors
    • Replaced inline @superset/ui/alert-dialog with a reusable DiscardConfirmDialog.
    • Flattened comments: active comments listed directly; Resolved is its own collapsible.
    • Removed redundant UI: duplicate comment counts, PR state text (icon conveys), and extra review label.
    • Unified collapsible trigger styles with CategorySection, added section dividers, and simplified loading to a single line.
    • PR header and comments: hover-to-reveal external link; moved per-comment actions to top-right, age to far right; exposed “Copy all” on the Comments header.

Written for commit c5ae344. Summary will update on new commits.

Summary by CodeRabbit

  • Refactor
    • Consolidated discard confirmation dialogs for staged and unstaged changes into a single shared dialog component.
    • Simplified the pull request review header by removing the PR number from listings and showing reviewer-awaiting state only when reviewers exist.
    • Reorganized comments into separate "Active" and "Resolved" collapsibles and added an always-visible "Copy all" action for active comments.
    • Replaced skeleton loading with a centered "Loading review..." message and adjusted spacing/positioning for comment rows.

- Replace inline AlertDialogs with reusable DiscardConfirmDialog component
- Simplify ReviewPanel loading skeleton to match app-wide pattern
- Flatten comment layout: show active comments directly, Resolved as own section
- Remove redundant review label that duplicated badge text
- Remove redundant PR state text already conveyed by icon color
- Remove duplicate comment count between parent and child sections
- Align collapsible trigger styles (padding, hover, chevron sizes) with CategorySection
- PR header: compact layout with hover-to-reveal external link
- Add section dividers between PR header, checks, and comments
- Push comment age to far right, absolutely position hover actions
- Clean up unused imports
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 29, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 53895fba-fa57-4024-b67e-e6fecf61ba90

📥 Commits

Reviewing files that changed from the base of the PR and between d235332 and c5ae344.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx

📝 Walkthrough

Walkthrough

Consolidated discard confirmation dialogs in ChangesView into a shared DiscardConfirmDialog component and refactored ReviewPanel to simplify loading state, flatten comment rendering, separate resolved comments into their own collapsible, and adjust comment action positioning and header content.

Changes

Cohort / File(s) Summary
Dialog Consolidation
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx
Removed direct AlertDialog/AlertDialog* and Button usages; replaced two discard dialogs with a shared DiscardConfirmDialog prop-driven component. Confirm handlers now call the appropriate discard mutation with worktreePath; dialog close is no longer handled here.
Comments Section Refactor
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx
Removed unused typings/utilities and related logic; simplified loading UI to a single message; changed comments rendering to a single "Comments" collapsible for active comments and a separate "Resolved" collapsible; switched comment action container to absolute positioning and adjusted header content (removed PR number and fallback label).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 I hopped through code with nimble paws,
Dialogs now one, no duplicate cause.
Comments sorted, resolved set apart,
Loading whispers clear from the start.
A tiny hop — cleaner UI art ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(desktop): clean up review sidebar UI and remove redundant states' clearly summarizes the main changes: cleaning up the review sidebar UI and removing redundant states, which aligns with the file summaries and PR objectives.
Description check ✅ Passed The PR description includes a comprehensive summary, detailed test plan with checkboxes, and an auto-generated description by cubic explaining the refactoring work. All key template sections are present and well-populated with relevant information about the changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch clean-up-review-sidebar-ui-and-remove-redundant-st

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx`:
- Line 134: The badge uses the total comments count but the panel shows only
activeComments; change the computation of commentsCountLabel in ReviewPanel
(where isCommentsLoading, comments and activeComments are referenced) to show
"..." when isCommentsLoading is true and otherwise use activeComments.length
(falling back to 0 if undefined) so the “Comments” badge reflects only
active/unresolved comments instead of comments.length.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7ab7c172-d794-42e8-83d8-2c8e909ec496

📥 Commits

Reviewing files that changed from the base of the PR and between 6338ce3 and d235332.

📒 Files selected for processing (2)
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx

const commentsCountLabel = isCommentsLoading
? "..."
: countOpenPullRequestComments(comments);
const commentsCountLabel = isCommentsLoading ? "..." : comments.length;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use active-comment count for the “Comments” badge.

Line 134 uses total comments (comments.length), but this section renders only activeComments. This can show a non-zero count while displaying “No comments yet.” when all comments are resolved.

💡 Suggested fix
- const commentsCountLabel = isCommentsLoading ? "..." : comments.length;
+ const commentsCountLabel = isCommentsLoading ? "..." : activeComments.length;
📝 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.

Suggested change
const commentsCountLabel = isCommentsLoading ? "..." : comments.length;
const commentsCountLabel = isCommentsLoading ? "..." : activeComments.length;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx`
at line 134, The badge uses the total comments count but the panel shows only
activeComments; change the computation of commentsCountLabel in ReviewPanel
(where isCommentsLoading, comments and activeComments are referenced) to show
"..." when isCommentsLoading is true and otherwise use activeComments.length
(falling back to 0 if undefined) so the “Comments” badge reflects only
active/unresolved comments instead of comments.length.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files

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/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx">

<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx:418">
P3: The empty-state uses `activeComments.length`, so if all comments are resolved the UI still shows “No comments yet.” Update the check to only show the empty state when there are truly no comments.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

…w/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx

Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
@Kitenite Kitenite merged commit b5fe8b4 into main Mar 29, 2026
6 of 7 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch
  • ⚠️ Electric Fly.io app

Thank you for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant