Sidebar: Add dual-slot status icons for change detection and test results#34346
Merged
Conversation
…ults Separate change-detection statuses (new/modified/affected) from test statuses (error/warning/pending/success) into two independent icon slots in the sidebar tree. This allows both types of information to be visible simultaneously on a story leaf node. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
View your CI Pipeline Execution ↗ for commit 9499622
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit b8167af ☁️ Nx Cloud last updated this comment at |
valentinpalkovic
changed the base branch from
next
to
valentin/status-based-filtering
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSplit sidebar statuses into separate "change-detection" and "test" slots: added getGroupDualStatus and getChangeDetectionStatus, updated Tree to compute/render dual-slot statuses (two status buttons/icons and adjusted ordering), plus stories and tests exercising dual-slot scenarios. Changes
Sequence Diagram(s)(Skipped — changes are internal UI/status rendering and utilities; no multi-actor runtime flow requiring sequence visualization.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/src/manager/components/sidebar/Tree.tsx`:
- Around line 303-305: Branch/group rows currently flatten branch-local statuses
into a single StatusValue[] and use getMostCriticalStatusValue(), which loses
the split (change vs test) aggregation; instead, compute the two-part
aggregation for branches using getChangeDetectionStatus() (like for leaves) so
you obtain changeStatus and testStatus for branch/group rows, then call
getStatus(theme, changeStatus) and getStatus(theme, testStatus) to derive
changeIcon, testIcon and textColor; replace any flattening of statuses (the code
using statuses || {} and the getMostCriticalStatusValue merge) with calls to
getChangeDetectionStatus on the branch-local status map and merge
branch+descendant results per-channel (change/test) rather than as a single
flattened value.
- Around line 344-365: The two StatusButton elements must be rendered as a fixed
two-slot action cluster so LeafNodeStyleWrapper's adjacent-sibling selectors
continue to work; update the JSX around StatusButton to always render a
container (e.g., a "status-cluster" div) with two children (left/change slot and
right/test slot) and when changeIcon or testIcon is null render an empty
placeholder element with the same tag/attributes (data-testid, ariaLabel, status
prop) but no children so the DOM structure and selectors remain stable; ensure
you reuse the existing identifiers StatusButton, changeIcon, testIcon,
changeStatus, testStatus and preserve ariaLabel/data-testid values for each
slot.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9519f012-92c3-4c81-a503-42b6be8ac18c
📒 Files selected for processing (3)
code/core/src/manager/components/sidebar/Tree.stories.tsxcode/core/src/manager/components/sidebar/Tree.tsxcode/core/src/manager/utils/status.test.ts
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
Contributor
There was a problem hiding this comment.
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 `@code/core/src/manager/components/sidebar/Tree.stories.tsx`:
- Around line 323-332: dualSlotData currently only seeds the immediate parent
and story entries (dualSlotParentId, dualSlotStoryId), which breaks when the
parent itself has a parent; update the construction of dualSlotData to include
the full ancestor chain from index by walking index[parent] repeatedly and
adding each ancestor entry (preserving their children arrays) so the tree
hierarchy remains intact; reference the symbols dualSlotStoryId,
dualSlotParentId, dualSlotData, index and ComponentEntry and ensure the
resulting map contains every ancestor id -> ComponentEntry needed for rendering.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3920fa72-e16d-47e2-9296-cd89c2c9a204
📒 Files selected for processing (1)
code/core/src/manager/components/sidebar/Tree.stories.tsx
ghengeveld
approved these changes
Mar 31, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/src/manager/components/sidebar/Tree.tsx`:
- Around line 441-456: The branch rows currently render two separate status
controls (branchChange and branchTest) and compute overallStatus from both;
instead derive a single branch status by choosing branchChange when it is not
'status-value:unknown' otherwise branchTest (e.g. const branchStatus =
branchChange !== 'status-value:unknown' ? branchChange : branchTest), then
compute a single branchIcon via getStatus(theme, branchStatus) or the small dot
SVG when branchStatus equals 'status-value:error' | 'status-value:warning', and
use that single branchStatus for overallStatus/color (replace
getMostCriticalStatusValue([branchChange, branchTest]) with branchStatus).
Update any places that expect branchChangeIcon/branchTestIcon to use this single
branchIcon so the left/right slot rendering no longer depends on both icons
existing and change-detection precedence is honored (refer to symbols:
branchChange, branchTest, branchStatus, branchIcon, overallStatus,
getMostCriticalStatusValue, getStatus, UseSymbol, theme).
- Around line 310-311: getChangeDetectionStatus is being partially destructured
(only testStatus) which drops the change-detection icon slot; instead capture
the full return value and derive the icon from it when rendering. Change the
code around getChangeDetectionStatus/getStatus so you keep the whole result
(e.g., const change = getChangeDetectionStatus(statuses || {})) and then call
getStatus(theme, change.testStatus || change.status) to obtain icon/textColor
(use those values where testIcon/textColor are used) and ensure the left-slot
rendering (the block around the current test button rendering at the 350-360
area) checks for change.* and renders the icon slot even when only
storybook/change-detection statuses exist.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a68abb06-8856-430f-98d7-6c4007516b39
📒 Files selected for processing (3)
code/core/src/manager/components/sidebar/Tree.stories.tsxcode/core/src/manager/components/sidebar/Tree.tsxcode/core/src/manager/utils/status.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/manager/components/sidebar/Tree.stories.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/core/src/manager/utils/status.tsx (1)
171-175: Consider extracting shared leaf-gathering logic.Lines 171-175 duplicate the leaf-gathering logic from
getGroupStatus(lines 142-146). A helper function could reduce duplication:const getStoryLeaves = (collapsedData, itemId) => getDescendantIds(collapsedData, itemId, false) .map((id) => collapsedData[id]) .filter((i) => i.type === 'story');This is a minor refactor that could be deferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/manager/utils/status.tsx` around lines 171 - 175, Duplicate logic that gathers story leaf nodes (using getDescendantIds, mapping collapsedData by id and filtering by type === 'story') appears in getGroupStatus and the block handling item.type === 'group'|'component'|'story'; extract that into a helper like getStoryLeaves(collapsedData, itemId) and replace both occurrences by calling getStoryLeaves(collapsedData, item.id) to remove duplication and centralize the leaf-gathering behavior; update imports/exports or file scope accordingly and keep the helper typed to accept collapsedData and an item id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code/core/src/manager/utils/status.tsx`:
- Around line 171-175: Duplicate logic that gathers story leaf nodes (using
getDescendantIds, mapping collapsedData by id and filtering by type === 'story')
appears in getGroupStatus and the block handling item.type ===
'group'|'component'|'story'; extract that into a helper like
getStoryLeaves(collapsedData, itemId) and replace both occurrences by calling
getStoryLeaves(collapsedData, item.id) to remove duplication and centralize the
leaf-gathering behavior; update imports/exports or file scope accordingly and
keep the helper typed to accept collapsedData and an item id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2f1246ab-27b1-47af-90a0-921debb5c41c
📒 Files selected for processing (1)
code/core/src/manager/utils/status.tsx
valentinpalkovic
added
ci:daily
8 tasks
valentinpalkovic
added
ci:normal
and removed
ci:daily
10 tasks
This was referenced Apr 29, 2026
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.
Closes #34266
What I did
Separated change-detection statuses (
new,modified,affected) from test statuses (error,warning,pending,success) into two independent icon slots in the sidebar tree. This allows both types of status information to be visible simultaneously on story leaf nodes.getChangeDetectionStatushelper that splits statuses bytypeIdinto change-detection vs test categoriesStatusButtoncomponents per leaf node (change slot + test slot)new,modified, andaffectedstatus values usingUseSymbolstatusOrderto include all status valuesChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
cd code && yarn storybook:uiSidebar/TreestoriesWithChangeDetectionOnly,WithChangeDetectionAndTestStatus,WithTestStatusOnly,WithAffectedStatus,BranchWithChangeDetectionPriority) render correctly with two separate status icon slotsDocumentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
Storybook
Bug Fixes
Tests