fix(ui): resolve count inconsistencies between sidebar and overview headings#252
Merged
fix(ui): resolve count inconsistencies between sidebar and overview headings#252
Conversation
…eadings Sub-components (ReviewQueue, MyPRs, Issues, ActivityFeed) rendered their own SectionHead with counts from sliced preview arrays when embedded in Overview, conflicting with Overview's own correct badges and the sidebar. - Add hideHeader prop to all 4 section components - Overview passes hideHeader to suppress duplicate headers - Filter PRs to open+draft before slicing (matching Issues pattern) - Add "View all" drill-down buttons for Reviews and My PRs sections - Add tests for hideHeader behavior and View All edge cases Closes #244
📝 WalkthroughWalkthroughMultiple dashboard components (ActivityFeed, Issues, MyPRs, ReviewQueue) now support an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/Overview/Overview.test.tsx (1)
318-336: Add a draft-PR assertion to fully lock the open+draft contract.Line 318’s test validates merged exclusion, but it doesn’t explicitly assert that
draftPRs are included in count/preview. Given the filter now includes bothopenanddraft, add one draft PR to this case to prevent regressions.💡 Suggested test extension
it("should only show open PRs in overview and exclude merged from count", () => { const openPrs = Array.from({ length: 3 }, (_, i) => makePr(i + 10)); + const draftPr = { + ...makePr(99), + pullRequest: { ...makePr(99).pullRequest, state: "draft" as const }, + }; const mergedPrs = Array.from({ length: 4 }, (_, i) => ({ ...makePr(i + 20), pullRequest: { ...makePr(i + 20).pullRequest, state: "merged" as const }, })); - setupMock(makeDashboard({ myPullRequests: [...openPrs, ...mergedPrs] })); + setupMock(makeDashboard({ myPullRequests: [...openPrs, draftPr, ...mergedPrs] })); @@ - // "View all" hidden because openPrCount (3) <= MAX_PRS (5) + // "View all" hidden because openPrCount (4: open + draft) <= MAX_PRS (5) expect(screen.queryByTestId("overview-prs-view-all")).not.toBeInTheDocument(); - // Badge shows correct open count - expect(screen.getByText("3 open")).toBeInTheDocument(); + // Badge shows correct open/draft count + expect(screen.getByText("4 open")).toBeInTheDocument(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Overview/Overview.test.tsx` around lines 318 - 336, The test "should only show open PRs in overview and exclude merged from count" (in Overview.test.tsx) doesn't assert that draft PRs are treated like open, so add a draft PR to the mock dashboard and assert it appears in the preview and in the open count; update the setupMock call that builds makeDashboard({ myPullRequests: [...] }) to include one makePr(...) whose pullRequest.state is "draft" (use the same pattern used for merged PRs), then assert the draft PR title (e.g., "PR `#21`") is present with the other open previews, and adjust the expected badge text to reflect the increased open+draft count (and confirm overview-prs-view-all visibility logic against MAX_PRS remains correct).src/components/Overview/Overview.tsx (1)
152-161: Consider extracting duplicated “View all” button markup.Both blocks share near-identical structure/classes; extracting a tiny helper/component would reduce repetition and keep future style/behavior edits in one place.
Also applies to: 188-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Overview/Overview.tsx` around lines 152 - 161, Duplicate "View all" button markup in Overview.tsx should be extracted into a small reusable component or helper: create a ViewAllButton component (e.g., function ViewAllButton({count, onClick, testId})) that renders the exact button markup and uses FOCUS_RING for classes; replace both occurrences (the block rendering when !showLoadingState && reviewRequests.length > MAX_REVIEWS and the similar block at the other location) with <ViewAllButton count={reviewRequests.length} onClick={() => useDashboardStore.getState().setView("reviews")} testId="overview-reviews-view-all" /> so style/behavior edits are centralized and no duplicate class strings remain in Overview.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/Overview/Overview.test.tsx`:
- Around line 318-336: The test "should only show open PRs in overview and
exclude merged from count" (in Overview.test.tsx) doesn't assert that draft PRs
are treated like open, so add a draft PR to the mock dashboard and assert it
appears in the preview and in the open count; update the setupMock call that
builds makeDashboard({ myPullRequests: [...] }) to include one makePr(...) whose
pullRequest.state is "draft" (use the same pattern used for merged PRs), then
assert the draft PR title (e.g., "PR `#21`") is present with the other open
previews, and adjust the expected badge text to reflect the increased open+draft
count (and confirm overview-prs-view-all visibility logic against MAX_PRS
remains correct).
In `@src/components/Overview/Overview.tsx`:
- Around line 152-161: Duplicate "View all" button markup in Overview.tsx should
be extracted into a small reusable component or helper: create a ViewAllButton
component (e.g., function ViewAllButton({count, onClick, testId})) that renders
the exact button markup and uses FOCUS_RING for classes; replace both
occurrences (the block rendering when !showLoadingState && reviewRequests.length
> MAX_REVIEWS and the similar block at the other location) with <ViewAllButton
count={reviewRequests.length} onClick={() =>
useDashboardStore.getState().setView("reviews")}
testId="overview-reviews-view-all" /> so style/behavior edits are centralized
and no duplicate class strings remain in Overview.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ef24219-09a5-4d3b-8cdd-a90f57d1677e
📒 Files selected for processing (10)
src/components/ActivityFeed/ActivityFeed.test.tsxsrc/components/ActivityFeed/ActivityFeed.tsxsrc/components/Issues/Issues.test.tsxsrc/components/Issues/Issues.tsxsrc/components/MyPRs/MyPRs.test.tsxsrc/components/MyPRs/MyPRs.tsxsrc/components/Overview/Overview.test.tsxsrc/components/Overview/Overview.tsxsrc/components/ReviewQueue/ReviewQueue.test.tsxsrc/components/ReviewQueue/ReviewQueue.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes count inconsistencies in the Overview by adding a
Confidence Score: 5/5Safe to merge — all remaining findings are P2 style/UX suggestions with no data or logic impact The core bug fixes (filter-then-slice, hideHeader suppression, View All buttons) are correctly implemented and well-tested. The only open finding is a P2 UX inconsistency where MyPRs renders an empty 'Merged 0' tab in the Overview panel, unlike Issues which uses hideTabs to suppress its filter chrome. This is cosmetic and does not affect correctness or data integrity. src/components/MyPRs/MyPRs.tsx and its usage in src/components/Overview/Overview.tsx — missing hideTabs prop causes an empty 'Merged 0' tab in the Overview panel Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
D[dashboard data] --> RR[reviewRequests]
D --> MP[myPullRequests]
D --> AI[assignedIssues]
D --> RA[recentActivity]
RR -->|slice 0..5| reviews["reviews (≤5)"]
MP -->|filter open+draft| openPrs[openPrs]
openPrs -->|slice 0..5| prs["prs (≤5)"]
AI -->|filter open| openIssues[openIssues]
openIssues -->|slice 0..5| issues["issues (≤5)"]
RA -->|slice 0..5| activities["activities (≤5)"]
reviews -->|hideHeader| RQ[ReviewQueue]
prs -->|hideHeader| MYP[MyPRs]
issues -->|hideHeader + hideTabs| ISS[Issues]
activities -->|hideHeader| AF[ActivityFeed]
RR -->|length > 5| VR["View all N reviews btn"]
openPrs -->|length > 5| VP["View all N PRs btn"]
openIssues -->|length > 5| VI["View all N issues btn"]
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #244 — count inconsistencies between sidebar badges, stats bar, and Overview section headings.
SectionHeadwith counts from sliced preview arrays when embedded in Overview, conflicting with Overview's correct badges and the sidebar statsChanges
hideHeaderprop to ReviewQueue, MyPRs, Issues, and ActivityFeed — suppressesSectionHeadwhen embedded in Overview (which has its own card headers with correct counts)hideHeaderbehavior for all 4 components, View All button visibility, mixed-state PR edge caseTest plan
npx tsc --noEmit— passesnpm run lint— 0 warnings, 0 errorsnpx vitest run— 667 tests pass, 0 failuresSummary by cubic
Fixes inconsistent counts between the sidebar badges and Overview cards by suppressing duplicate headers and aligning PR filtering. Adds “View all” actions for Reviews and My PRs when items exceed the preview limit.
Bug Fixes
hideHeadertoReviewQueue,MyPRs,Issues, andActivityFeed; Overview now hides sub-headers to avoid mismatched counts.open+draftbefore slicing so Overview counts match the sidebar and badges.New Features
Written for commit 39bc3a7. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Tests