StatusValue: Add 'status-value:<new|modified|affected>'#34305
Merged
valentinpalkovic merged 9 commits intoMar 25, 2026
Conversation
…nts with changeDetection feature flag
|
View your CI Pipeline Execution ↗ for commit 0b1442a
☁️ Nx Cloud last updated this comment at |
valentinpalkovic
changed the title
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds three status values—status-value:new, status-value:modified, status-value:affected—across core types, priority ordering, icon/color mappings, sidebar aggregation/display, vitest addon typing, stories, and tests; updates aria-label text from "Test status:" to "Status:". Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/core/src/manager/components/sidebar/StatusContext.tsx (1)
53-55:⚠️ Potential issue | 🟠 MajorInclude change-detection statuses in summary gating.
useStatusSummarynow definesnew/modified/affectedbuckets, but this gate still excludes those group statuses, so their summaries won’t populate.🧩 Proposed fix
- ['status-value:pending', 'status-value:warning', 'status-value:error'].includes( + [ + 'status-value:pending', + 'status-value:new', + 'status-value:modified', + 'status-value:affected', + 'status-value:warning', + 'status-value:error', + ].includes( groupStatus[item.id] )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/manager/components/sidebar/StatusContext.tsx` around lines 53 - 55, The gating check in StatusContext.tsx excludes change-detection statuses so summaries from useStatusSummary's new/modified/affected buckets never populate; update the conditional that checks groupStatus[item.id] (currently testing ['status-value:pending','status-value:warning','status-value:error']) to also include the change-detection statuses (e.g. 'status-value:new', 'status-value:modified', 'status-value:affected') or replace the literal list with a helper that derives allowed summary statuses from useStatusSummary so those buckets are considered when deciding whether to render population.
🤖 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/addons/vitest/src/use-test-provider-state.ts`:
- Around line 23-29: The current narrowing to TestStatusValue and casting (e.g.,
using status.value as TestStatusValue to index StatusValueToStoryIds) can crash
if new StatusValue variants appear; update the code to avoid unsafe casts by
making the mapping creation and access exhaustive or guarded: initialize
StatusValueToStoryIds with all possible StatusValue keys (or use a Map keyed by
StatusValue) and replace direct casts like status.value as TestStatusValue with
a safe check/lookup (e.g., verify the key exists or use a default bucket) before
calling .push; adjust functions that reference TestStatusValue and
StatusValueToStoryIds so they either compute keys from StatusValue without
casting or handle unknown values explicitly.
In `@code/core/src/manager/components/sidebar/StatusButton.tsx`:
- Around line 21-23: The three status color entries in the StatusButton
component are all set to theme.fgColor.accent, losing semantic distinction;
update the mapping in StatusButton (the 'status-value:new',
'status-value:modified', 'status-value:affected' entries) to use distinct
semantic theme colors (e.g. map 'status-value:new' to theme.fgColor.success,
'status-value:modified' to theme.fgColor.warning, and 'status-value:affected' to
theme.fgColor.danger or the equivalent names used in your theme) so each status
retains its unique color in the status rendering.
In `@code/core/src/manager/utils/status.tsx`:
- Around line 32-34: The statusPriority array currently places
'status-value:new' after 'status-value:affected'; update the ordering in the
statusPriority constant so it follows the intended precedence success < new <
modified < affected < warning by placing 'status-value:new' immediately after
'status-value:success', then 'status-value:modified', then
'status-value:affected', and keeping 'status-value:warning' last; modify the
statusPriority definition (and any references to it) accordingly to ensure
change-detection statuses rank as: 'status-value:success', 'status-value:new',
'status-value:modified', 'status-value:affected', 'status-value:warning'.
In `@code/core/src/types/modules/core-common.ts`:
- Around line 539-544: The JSDoc for the optional flag changeDetection claims
"@default true" but the features preset does not set it, causing an effective
default of false; to fix, update the features PresetProperty<'features'> export
(the features variable in the common preset) to include changeDetection: true so
runtime matches the documented default, and verify the changeDetection?: boolean
JSDoc remains accurate.
---
Outside diff comments:
In `@code/core/src/manager/components/sidebar/StatusContext.tsx`:
- Around line 53-55: The gating check in StatusContext.tsx excludes
change-detection statuses so summaries from useStatusSummary's
new/modified/affected buckets never populate; update the conditional that checks
groupStatus[item.id] (currently testing
['status-value:pending','status-value:warning','status-value:error']) to also
include the change-detection statuses (e.g. 'status-value:new',
'status-value:modified', 'status-value:affected') or replace the literal list
with a helper that derives allowed summary statuses from useStatusSummary so
those buckets are considered when deciding whether to render population.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 57b7d38b-0b5e-4e77-9705-f52018a2f0aa
📒 Files selected for processing (8)
code/addons/vitest/src/use-test-provider-state.tscode/core/src/manager/components/sidebar/IconSymbols.tsxcode/core/src/manager/components/sidebar/StatusButton.tsxcode/core/src/manager/components/sidebar/StatusContext.tsxcode/core/src/manager/utils/status.test.tscode/core/src/manager/utils/status.tsxcode/core/src/shared/status-store/index.tscode/core/src/types/modules/core-common.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
code/core/src/manager/utils/status.tsx (1)
28-37:⚠️ Potential issue | 🟠 MajorFix
statusPriorityorder for change-detection statuses.The current order places
affectedbeforemodifiedbeforenew, but sincegetMostCriticalStatusValuereturns the last matching value in the array, the current order makesnewoutrankaffected. Per the linked issue#34292, the intended priority issuccess < new < modified < affected < warning.🔁 Proposed fix
export const statusPriority: StatusValue[] = [ 'status-value:unknown', 'status-value:pending', 'status-value:success', - 'status-value:affected', - 'status-value:modified', 'status-value:new', + 'status-value:modified', + 'status-value:affected', 'status-value:warning', 'status-value:error', ];🤖 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 28 - 37, The statusPriority array currently orders change-detection statuses so that getMostCriticalStatusValue (which returns the last matching value) incorrectly gives 'new' higher priority than 'affected'; update the statusPriority constant so the sequence reflects the intended priority (success < new < modified < affected < warning) by reordering the entries for 'status-value:new', 'status-value:modified', and 'status-value:affected' accordingly in the exported statusPriority array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@code/core/src/manager/utils/status.tsx`:
- Around line 28-37: The statusPriority array currently orders change-detection
statuses so that getMostCriticalStatusValue (which returns the last matching
value) incorrectly gives 'new' higher priority than 'affected'; update the
statusPriority constant so the sequence reflects the intended priority (success
< new < modified < affected < warning) by reordering the entries for
'status-value:new', 'status-value:modified', and 'status-value:affected'
accordingly in the exported statusPriority array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 702064f6-301c-431a-806b-5c49925cb785
📒 Files selected for processing (7)
code/core/src/manager/components/sidebar/IconSymbols.tsxcode/core/src/manager/components/sidebar/Sidebar.stories.tsxcode/core/src/manager/components/sidebar/StatusContext.tsxcode/core/src/manager/components/sidebar/Tree.tsxcode/core/src/manager/utils/status.tsxtest-storybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.tstest-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts
ghengeveld
approved these changes
Mar 25, 2026
| ); | ||
| }); | ||
|
|
||
| it('should rank affected above modified and below warning', () => { |
valentinpalkovic
deleted the
valentin/statusvalue-extension-change-detection
branch
valentinpalkovic
changed the title
15 tasks
This was referenced Mar 26, 2026
Closed
8 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 #34292
What I did
Added two new
StatusValuevariants:status-value:new,status-value:modifiedandstatus-value:affected. These are intended for use in agentic UI review workflows to surface which stories are new or have changed content.Changes include:
StatusValueunion type (status-store/index.ts) withstatus-value:newandstatus-value:modifiedAddIconfornew,EditIconformodified) to the sidebarIconSymbolsnewusestheme.color.positive(green),modifiedusestheme.color.secondary(purple)successandwarningstatusValueToStoryIdsmap to include the new valuesChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Not testable yet. This needs further integration.
Documentation
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 do not 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
Tests
Chores