fix(team-view): revert heatmap column growth; member popup links to the IC page#200
Merged
Conversation
…from Expand details In the Members × metrics popup, "Open in IC view" opened the details sheet while "Expand details" toggled an inline bullet strip under the row. Rework per review of #198: "Open in IC view" is now a router link to the member's personal IC page, and "Expand details" opens the details sheet (which already shows the full metric set after #193). The inline ExpandedBullets strip and its expansion state go away. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Roman Mitasov <roman.mitasov@constructor.tech>
📝 WalkthroughWalkthroughThe members heatmap now renders a fixed team-and-bullet grid, simplifies metric formatting, builds detail-sheet rows from grid cells, and replaces expanded bullet rows with details-sheet and IC-view actions. Tests mock router links and verify the new navigation flow. ChangesMembers heatmap
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: private package registry requires authentication. Disable ESLint in CodeRabbit settings or use public packages. 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 |
ktursunov
approved these changes
Jul 15, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/widgets/v2/members-heatmap/index.tsx (1)
293-295: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse previous bullet data without requiring
prevMember.Bullet columns only need
prevBullets, but this guard discards them unlesspreviousMembersalso contains the person, silently disabling bullet WoW indicators.Proposed fix
- const previous = prevMember - ? valueForColumn(col, prevMember, prevBullets) - : null; + const previous = + col.source === "team_row" + ? prevMember + ? valueForColumn(col, prevMember, prevBullets) + : null + : valueForColumn(col, m, prevBullets);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/widgets/v2/members-heatmap/index.tsx` around lines 293 - 295, Update the previous-value calculation around valueForColumn so bullet columns use prevBullets whenever previous bullet data exists, without requiring prevMember to be truthy. Preserve the existing member-based lookup for non-bullet columns and ensure missing data still produces null.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/widgets/v2/members-heatmap/index.tsx`:
- Around line 427-432: Update the cell display formatting in the row.cells
mapping and the corresponding paths at the additional locations to use the
shared unit-suffix formatter for both values and medians. Preserve null handling
and rounding while ensuring multi-character units are separated from the numeric
value consistently with the existing bullet-sheet formatting.
---
Outside diff comments:
In `@src/components/widgets/v2/members-heatmap/index.tsx`:
- Around line 293-295: Update the previous-value calculation around
valueForColumn so bullet columns use prevBullets whenever previous bullet data
exists, without requiring prevMember to be truthy. Preserve the existing
member-based lookup for non-bullet columns and ensure missing data still
produces null.
🪄 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: 661991c4-9085-4df6-a8ec-c27698c42f85
📒 Files selected for processing (2)
src/components/widgets/v2/members-heatmap/index.test.tsxsrc/components/widgets/v2/members-heatmap/index.tsx
Comment on lines
427
to
+432
| const fromCells: MemberDetailRow[] = row.cells.map((c) => ({ | ||
| key: c.col.key, | ||
| label: c.col.label, | ||
| display: | ||
| c.value == null | ||
| ? "—" | ||
| : c.format | ||
| ? formatMetricValue(c.value, c.format, c.unit || null) | ||
| : `${Math.round(c.value)}${unitSuffix(c.unit)}`, | ||
| display: c.value == null ? "—" : `${Math.round(c.value)}${c.unit ?? ""}`, | ||
| medianDisplay: | ||
| c.median == null | ||
| ? null | ||
| : c.format | ||
| ? formatMetricValue(c.median, c.format, c.unit || null) | ||
| : `${Math.round(c.median)}${unitSuffix(c.unit)}`, | ||
| c.median != null ? `${Math.round(c.median)}${c.unit ?? ""}` : null, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve spacing for multi-character units.
Direct interpolation renders arbitrary bullet units as strings such as 12hours. Reuse a shared unit-suffix formatter for cell values and medians; the remaining bullet-sheet path already inserts this separator.
Proposed approach
+function unitSuffix(unit?: string): string {
+ return unit ? (unit.length === 1 ? unit : ` ${unit}`) : "";
+}
-`${Math.round(c.value)}${c.unit ?? ""}`
+`${Math.round(c.value)}${unitSuffix(c.unit)}`
-`${Math.round(value)}${unit ?? ""}`
+`${Math.round(value)}${unitSuffix(unit)}`Also applies to: 576-579, 612-614
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/widgets/v2/members-heatmap/index.tsx` around lines 427 - 432,
Update the cell display formatting in the row.cells mapping and the
corresponding paths at the additional locations to use the shared unit-suffix
formatter for both values and medians. Preserve null handling and rounding while
ensuring multi-character units are separated from the numeric value consistently
with the existing bullet-sheet formatting.
mitasovr
added a commit
that referenced
this pull request
Jul 15, 2026
…201) The diff-coverage gate on #200 flagged the column-sort branch of sortedRows (COLUMNS.findIndex lookup) — the revert re-introduced those lines and no test clicked a column header. Add a test that sorts by a higher-is-better column, checks the row order flips, and exercises the lower-is-better path with missing values. Signed-off-by: Roman Mitasov <roman.mitasov@constructor.tech> Co-authored-by: Roman Mitasov <roman.mitasov@constructor.tech> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
Problem
Two things in the Members × metrics widget:
Change
1a44492): the grid is back to the curated 7 columns.<Link>to the member's personal IC page (/ic/$person/personal), rendered through the Button render prop — real navigation, middle-click and copy-link work.ExpandedBulletsstrip, its expansion state, and the unusedMemberRowplumbing are removed.Verification
Verified in the browser against the mock stack: the grid renders 7 columns; the popup's "Open in IC view" navigates SPA-style to the member's personal dashboard; "Expand details" opens the sheet with the member's full metric set (60 rows on the mock roster).
Typecheck, ESLint, and the full vitest suite pass; the component test asserts the link href and opens the sheet through "Expand details".
Related to constructorfabric/insight#1729
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes