Sidebar: Soften change-detection signals + add Review CTA#34701
Merged
Conversation
Tree branch icons: hide affected unconditionally; gate modified on the modified status filter being active; new stays always-visible. Filter menu drops the icon for the related (affected) row to match. Add ReviewChangesButton: ghost CTA between search bar and explorer menu that toggles new+modified status filters together via setAllStatusFilters (preserves user-set excluded entries). Hidden when feature off, when both counts are zero, or while search results are rendered. Mounted via new belowSearchContent slot on Search. Refactor Tree to subscribe via useStorybookState() instead of wrapping in Consumer — Consumer's useRef(children) cached the first-render closure, so live filter changes never propagated to TreeInner. Harden findLeafEntry against missing entries to avoid a runtime crash when clicking ancestor nodes whose descendants are filtered out. Add stories covering all new behaviors (CTA states, branch icon gating, live toggle integration, hide-during-search). Update AGENTS.md to prefer Storybook stories over unit tests for React components.
valentinpalkovic
changed the title
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:
📝 WalkthroughWalkthroughAdds a sidebar "Review" CTA and wiring: new ReviewChangesButton, Search prop to host it, Sidebar/Tree/FilterPanel rendering changes to show/hide status icons and CTA, Storybook stories/interaction tests for CTA/tree behaviors, an API change making leaf lookups optional, and docs/test guidance updates in AGENTS.md and change-detection docs. ChangesChange Detection Review Feature
Sequence DiagramsequenceDiagram
participant User as User
participant Sidebar as Sidebar (UI)
participant CTA as ReviewChangesButton
participant StatusStore as StatusStore
participant Filters as FilterManager
User->>Sidebar: open sidebar
Sidebar->>StatusStore: read counts (new, modified)
StatusStore-->>CTA: provide counts
CTA->>CTA: compute active from included/excluded filters
CTA-->>Sidebar: render CTA with counts & aria-pressed
User->>CTA: click CTA
CTA->>Filters: request update (include/exclude new+modified)
Filters-->>Sidebar: filters updated
Sidebar->>StatusStore: re-read / re-filter statuses
Sidebar-->>User: update tree rendering (icons shown/hidden)
User->>Sidebar: type in search
Sidebar->>CTA: hide CTA (belowSearchContent not rendered)
Sidebar-->>User: show search results (CTA hidden)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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: 1
🧹 Nitpick comments (2)
code/core/src/manager/components/sidebar/ReviewChangesButton.stories.tsx (1)
246-304: ⚡ Quick winAdd one play test for the deactivate branch of the CTA toggle.
Current interaction coverage verifies activation paths, but not active→deactivate behavior. A small regression story here would lock the other half of
onClickcontract (removingnew/modifiedfrom included while preserving unrelated excluded entries).Proposed story addition
+const toggleDeactivateMock = fn().mockName('api::setAllStatusFilters'); + +export const ToggleDeactivatePreservesExcluded: Story = { + parameters: { + contextOptions: { + includedStatusFilters: ['status-value:new', 'status-value:modified', 'status-value:error'], + excludedStatusFilters: ['status-value:warning'], + setAllStatusFilters: toggleDeactivateMock, + }, + }, + beforeEach: () => { + toggleDeactivateMock.mockClear(); + return twoStoriesBeforeEach(); + }, + play: async ({ canvasElement, parameters }) => { + const canvas = within(canvasElement); + const button = await canvas.findByRole('button'); + await userEvent.click(button); + const mock = parameters.contextOptions.setAllStatusFilters; + await expect(mock).toHaveBeenCalledOnce(); + const [included, excluded] = mock.mock.calls[0]; + await expect(included).not.toContain('status-value:new'); + await expect(included).not.toContain('status-value:modified'); + await expect(included).toContain('status-value:error'); + await expect(excluded).toEqual(['status-value:warning']); + }, +};As per coding guidelines:
code/**/*.stories.tsx: Write Storybook stories with play functions using 'expect', 'userEvent', 'within' from 'storybook/test' for component behavior testing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/manager/components/sidebar/ReviewChangesButton.stories.tsx` around lines 246 - 304, Add a new Story (e.g., ToggleDeactivate) mirroring ToggleActivate/TogglePreservesExcluded that tests the deactivate branch: create a mock like toggleDeactivateMock (mockName 'api::setAllStatusFilters'), set parameters.contextOptions with includedStatusFilters seeded ['status-value:new','status-value:modified'] and excludedStatusFilters seeded with an unrelated value (e.g., 'status-value:other') and setAllStatusFilters: toggleDeactivateMock, clear the mock in beforeEach (like toggleActivateMock.mockClear()), then in play use within + userEvent.click(button) and assert the mock was called once and that the returned [included, excluded] from mock.mock.calls[0] no longer contains 'status-value:new'/'status-value:modified' in included while the unrelated excluded entry remains in excluded (use the same expect style as existing stories).code/core/src/manager/components/sidebar/Sidebar.stories.tsx (1)
19-23: 💤 Low valueConsider adding a clarifying comment for the dual-store imports.
The file imports
internal_fullStatusStorefrom two different sources: the mock (line 20-22) and the real API (line 23). While the aliasinternal_ctaStatusStorehelps distinguish them, a brief comment would help future maintainers understand the intent—the mock is for general sidebar state testing, while the real store is needed for CTA status counting.📝 Suggested clarification
import { internal_fullStatusStore, internal_universalChecklistStore, } from '../../manager-stores.mock.ts'; +// Real status store for CTA stories (needs actual countStatusesByValue behavior) import { internal_fullStatusStore as internal_ctaStatusStore } from 'storybook/manager-api';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/manager/components/sidebar/Sidebar.stories.tsx` around lines 19 - 23, Add a short clarifying comment above the dual imports in Sidebar.stories.tsx explaining that internal_fullStatusStore is imported twice from different sources: the mock (internal_fullStatusStore, internal_universalChecklistStore) is used to provide general sidebar/test state, while the aliased import internal_fullStatusStore as internal_ctaStatusStore from 'storybook/manager-api' is the real store used specifically for CTA status counting; place the comment directly above the import lines so future maintainers understand the intent of each import.
🤖 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-api/modules/stories.ts`:
- Around line 251-259: The JSDoc for findLeafEntry and findLeafStoryId still
says the functions return “or null” but their signatures return | undefined;
update the return descriptions in both JSDoc blocks (the one above findLeafEntry
and the one above findLeafStoryId) to state they return undefined when no value
is found (e.g., “The ID of the leaf story, or undefined if no leaf story was
found”) so the docs match the | undefined contract.
---
Nitpick comments:
In `@code/core/src/manager/components/sidebar/ReviewChangesButton.stories.tsx`:
- Around line 246-304: Add a new Story (e.g., ToggleDeactivate) mirroring
ToggleActivate/TogglePreservesExcluded that tests the deactivate branch: create
a mock like toggleDeactivateMock (mockName 'api::setAllStatusFilters'), set
parameters.contextOptions with includedStatusFilters seeded
['status-value:new','status-value:modified'] and excludedStatusFilters seeded
with an unrelated value (e.g., 'status-value:other') and setAllStatusFilters:
toggleDeactivateMock, clear the mock in beforeEach (like
toggleActivateMock.mockClear()), then in play use within +
userEvent.click(button) and assert the mock was called once and that the
returned [included, excluded] from mock.mock.calls[0] no longer contains
'status-value:new'/'status-value:modified' in included while the unrelated
excluded entry remains in excluded (use the same expect style as existing
stories).
In `@code/core/src/manager/components/sidebar/Sidebar.stories.tsx`:
- Around line 19-23: Add a short clarifying comment above the dual imports in
Sidebar.stories.tsx explaining that internal_fullStatusStore is imported twice
from different sources: the mock (internal_fullStatusStore,
internal_universalChecklistStore) is used to provide general sidebar/test state,
while the aliased import internal_fullStatusStore as internal_ctaStatusStore
from 'storybook/manager-api' is the real store used specifically for CTA status
counting; place the comment directly above the import lines so future
maintainers understand the intent of each import.
🪄 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: d0a6c957-7433-4f4a-af29-f60a43787e38
📒 Files selected for processing (10)
AGENTS.mdcode/core/src/manager-api/modules/stories.tscode/core/src/manager/components/sidebar/FilterPanel.tsxcode/core/src/manager/components/sidebar/ReviewChangesButton.stories.tsxcode/core/src/manager/components/sidebar/ReviewChangesButton.tsxcode/core/src/manager/components/sidebar/Search.tsxcode/core/src/manager/components/sidebar/Sidebar.stories.tsxcode/core/src/manager/components/sidebar/Sidebar.tsxcode/core/src/manager/components/sidebar/Tree.stories.tsxcode/core/src/manager/components/sidebar/Tree.tsx
- Document new visibility rules in the status table (related has no icon) - Reframe the callout around why modified/related are quiet by default - Add a "Reviewing changes" section describing the new Review CTA - Reframe Filtering around the related opt-in path
valentinpalkovic
commented
May 4, 2026
Co-authored-by: Valentin Palkovic <dev@valentinpalkovic.dev>
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 `@docs/configure/user-interface/change-detection.mdx`:
- Line 56: Update the sentence "The button hides while you're typing in the
search input" to accurately describe the behavior: change it to indicate the
button is hidden whenever search-results mode is active/shown (e.g., "The button
hides when search results are shown/active"), so the doc reflects the
mode-driven behavior rather than only keystroke activity; edit this phrasing in
the user-interface/change-detection.mdx content where that sentence appears.
- Line 52: Update the active-state description text that currently reads
"Active: `Reviewing N new, M changed` (highlighted, primary color)" to reflect
the implemented styling: describe it as a hoverable/background-highlight with
secondary text (e.g., "Active: `Reviewing N new, M changed` (highlighted with
hoverable background and secondary text)") so the docs match the actual UI
behavior.
🪄 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: 9db6579e-f38c-44e1-9a5b-76ce3680cf3b
📒 Files selected for processing (1)
docs/configure/user-interface/change-detection.mdx
8 tasks
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/configure/user-interface/change-detection.mdx (1)
51-52:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the active-state styling copy.
primary colorstill doesn't match the rendered state; the active button uses a hoverable background and secondary text instead.Suggested edit
-- Active: `Reviewing N new, M changed` (highlighted, primary color) +- Active: `Reviewing N new, M changed` (highlighted with the sidebar hoverable background and secondary text)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/configure/user-interface/change-detection.mdx` around lines 51 - 52, The doc's active-state description is inaccurate — replace the current line "Active: `Reviewing N new, M changed` (highlighted, primary color)" with wording that matches the actual rendered state, e.g. "Active: `Reviewing N new, M changed` (hoverable background, secondary text)", so the copy accurately documents the active button's hoverable background and secondary-text styling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/configure/user-interface/change-detection.mdx`:
- Around line 54-56: Update the wording that currently says "clears those
filters" and "returns the tree to its normal state" to clearly state that the
toggle only enables/disables the `new` and `modified` include filters and does
not modify other user-selected filters; e.g., replace the sentence with a line
indicating "clicking it again disables the `new`/`modified` include filters and
preserves any other active filters, returning the sidebar to the prior filtered
view." Ensure the new text references the `new`/`modified` include filters
explicitly so readers understand other filters remain unchanged.
---
Duplicate comments:
In `@docs/configure/user-interface/change-detection.mdx`:
- Around line 51-52: The doc's active-state description is inaccurate — replace
the current line "Active: `Reviewing N new, M changed` (highlighted, primary
color)" with wording that matches the actual rendered state, e.g. "Active:
`Reviewing N new, M changed` (hoverable background, secondary text)", so the
copy accurately documents the active button's hoverable background and
secondary-text styling.
🪄 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: cb2181de-3d65-425a-83ee-96f3813dfb84
📒 Files selected for processing (1)
docs/configure/user-interface/change-detection.mdx
Contributor
|
@valentinpalkovic — I tried to kick off a canary workflow, but it failed. Can you please make a canary release so I can verify/update the docs changes? Thanks. |
Member
|
I think there’s a UX inconsistency in the new Review changes CTA behavior:
So the control behaves like a contextual filter toggle, but presents itself like a global summary badge. In filtered states, that can make the numbers feel misleading. I see two coherent paths:
I’d lean toward (2) for consistency and user trust in the label. If we prefer (1) for implementation simplicity, I think we should still update copy/tooltips/docs to make the global scope explicit. |
Member
|
One more important gap: Why:
Net effect: Review CTA interactions are missing from I don’t think we should skip this. We should emit telemetry for this path as well (either in |
- Align findLeafEntry/findLeafStoryId JSDoc with `| undefined` return contract - Clarify Review CTA active styling and toggle behavior in change-detection docs
- Drop "0 new" / "0 changed" from CTA label and aria-label when count is zero - Add OnlyNew / OnlyModified stories covering the new label branches - Rename Tree stories: WithModifiedFilterActive→WithModified, WithNewAlwaysVisible→WithNew - Remove redundant WithModifiedFilterInactive story
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/configure/user-interface/change-detection.mdx (1)
60-60: 💤 Low valueConsider clarifying that related stories never display an icon.
The sentence mentions that modified branch icons appear conditionally, and uses "only related stories" as an example filtering scenario. Since the status table shows no icon for related status, readers might benefit from an explicit statement that related stories don't display a status icon (regardless of filter state), to avoid any ambiguity when they use the related filter.
Optional clarity enhancement
-For more granular control — for example, to inspect only **related** stories — open the filter menu next to the search bar and check or uncheck the individual statuses. Modified branch icons appear in the tree only while the **modified** filter is checked. +For more granular control — for example, to inspect only **related** stories — open the filter menu next to the search bar and check or uncheck the individual statuses. Modified branch icons appear in the tree only while the **modified** filter is checked. Related stories do not display a status icon.🤖 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 `@docs/configure/user-interface/change-detection.mdx` at line 60, Update the paragraph that references "related stories" and "modified branch icons" (the sentence starting "For more granular control — for example, to inspect only **related** stories — ...") to explicitly state that the "related" status never shows a status icon; e.g., add a short clause after the example clarifying "related stories do not display an icon regardless of filter state" so readers understand the status table's omission and know the related filter won't show an icon even when enabled.
🤖 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 `@docs/configure/user-interface/change-detection.mdx`:
- Around line 47-56: Update the "Reviewing changes" section to explicitly state
that the Review button is hidden when the change-detection feature flag is
disabled by adding a short note mentioning `features.changeDetection` (e.g.,
"The button is not shown when change detection is disabled via configuration
(see Configuration)"). Place this note alongside the existing visibility
conditions in the "Reviewing changes" paragraph so readers see the
new/modified/search and feature-flag rules together.
- Line 8: Update the sentence describing the Review button to clarify that its
live counts are global totals (all new and modified stories in the project)
rather than contextual to active filters, and indicate that clicking the Review
button applies the "new/modified" filter while preserving any other active
filters; modify the existing paragraph that mentions "shows live counts of new
and modified stories" to include this explicit clarification (referencing the
"Review" button and the status store behavior described in the same paragraph)
so readers understand counts may not match the currently filtered view.
---
Nitpick comments:
In `@docs/configure/user-interface/change-detection.mdx`:
- Line 60: Update the paragraph that references "related stories" and "modified
branch icons" (the sentence starting "For more granular control — for example,
to inspect only **related** stories — ...") to explicitly state that the
"related" status never shows a status icon; e.g., add a short clause after the
example clarifying "related stories do not display an icon regardless of filter
state" so readers understand the status table's omission and know the related
filter won't show an icon even when enabled.
🪄 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: e72b24c6-1b4b-4b7c-896a-c972b56110bd
📒 Files selected for processing (2)
code/core/src/manager-api/modules/stories.tsdocs/configure/user-interface/change-detection.mdx
✅ Files skipped from review due to trivial changes (1)
- code/core/src/manager-api/modules/stories.ts
valentinpalkovic
commented
May 5, 2026
Co-authored-by: Valentin Palkovic <dev@valentinpalkovic.dev>
Diff old vs. new included/excluded status arrays and emit one SIDEBAR_FILTER_CHANGED interaction event per changed filter. Closes the telemetry gap for the Review CTA toggle, which routes through this API.
Compute the new / changed counts shown in the CTA from the story index under the currently active tag and status filters (with new/modified removed from the status filter so the CTA still surfaces them). This aligns the label semantics with the toggle action, which has always been filter-aware. Stories now seed internal_index so the contextual filter has entries to walk; new ContextualTagFilter story locks the behavior under an active tag filter.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 60 | 60 | 0 |
| Self size | 20.45 MB | 20.49 MB | 🚨 +33 KB 🚨 |
| Dependency size | 32.84 MB | 32.84 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 194 | 194 | 0 |
| Self size | 890 KB | 881 KB | 🎉 -9 KB 🎉 |
| Dependency size | 84.44 MB | 84.47 MB | 🚨 +33 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 187 | 187 | 0 |
| Self size | 32 KB | 32 KB | 🎉 -36 B 🎉 |
| Dependency size | 82.94 MB | 82.97 MB | 🚨 +33 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 61 | 61 | 0 |
| Self size | 1.07 MB | 1.07 MB | 🚨 +4 B 🚨 |
| Dependency size | 53.29 MB | 53.32 MB | 🚨 +33 KB 🚨 |
| Bundle Size Analyzer | node | node |
ReviewChangesButton counts statuses by joining storyId against state.internal_index.entries. The Sidebar stories only seeded indexJson with a single dummy entry, so the new/modified/affected storyIds from mockDataset.withRoot never matched and the CTA stayed hidden. Build an indexJsonWithAllStories from mockDataset.withRoot and apply it to StatusesNew, StatusesModified, WithCTAActive, CTAToggleUpdatesLive, and CTAHiddenDuringSearch. Align play assertions with the component text format: when a count is zero the corresponding segment is omitted, so "Review N new, 0 changed" becomes "Review N new" (and the modified-only path uses "Review N changed" with a /Review \d+ changed/ aria regex). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tatus The branch-level modified change-detection icon only renders when the modified status filter is active. Without that filter, the dual-slot design (change icon + test status dot) collapsed to test-status-only in WithChangeDetectionAndTestStatus. Pass an includedStatusFilters override via contextOverride so the story renders both slots. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The branch-level "Modified" change-detection icon is now gated on the modified status filter being active, and the "Affected" icon is no longer rendered at branch level. Update the e2e suite to reflect this: - "shows modified" — click the ReviewChangesButton CTA before asserting, so the modified status filter is active and the badge renders. - "shows related" — drop the test entirely; affected has no visible branch icon anymore. Remove the now-unused buttonComponentPath helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CTA now reads "Review new stories", "Review modified stories", or "Review new and modified stories" (and "Reviewing ..." while active), without numeric counts. Update the assertions in ReviewChangesButton.stories.tsx, Sidebar.stories.tsx, and the e2e selector in change-detection.spec.ts to match the new copy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…state Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Flatten ReviewChangesButton: drop Consumer/filterMapper/Inner split in favor of useStorybookState + useStorybookApi. - Collapse label assembly into a single ternary; replace Object.values(...).map(...).includes(...) with .some(...) for the count loop. - Replace the negated branch-change-icon ternary in Tree.tsx with a named shouldShowBranchChangeIcon predicate. - Collapse the secondary guard in findLeafEntry. - Extract a setChangeStatuses fixture helper in ReviewChangesButton.stories.tsx; reuse it across stories. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kasperpeulen
approved these changes
May 6, 2026
ReviewChangesButton now uses ToggleButton, which exposes role="switch" and aria-checked instead of role="button"/aria-pressed. Update the play-function assertions in Sidebar.stories.tsx to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
valentinpalkovic
force-pushed
the
valentin/change-detection-changes
branch
from
29dd19a to
3219ec3
Compare
15 tasks
4 tasks
32 tasks
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
4 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 #
What I did
Softens the change-detection signals in the sidebar after team feedback that branch icons produced too much noise on real-world repos, and adds a CTA to enter "review mode" when there are pending changes.
Sidebar branch icons
newmodifiedaffected/relatedReviewChangesButton
Ghost toggle below the search bar. Visible when the change-detection feature is on and at least one new/modified story exists.
Review new and modified stories→Reviewing new and modified storieswhile pressed.Tree state subscription fix
Tree.tsxswitched fromConsumertouseStorybookState(). TheConsumercache was holding the first-render closure, so live filter changes didn't propagate.Crash hardening
findLeafEntryreturnsundefinedinstead of throwing when recursion lands on a story id missing from the filtered index.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
cd code && yarn storybook:uiDocumentation
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 pull request has been released as version
0.0.0-pr-34701-sha-181521d2. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-34701-sha-181521d2 sandboxor in an existing project withnpx storybook@0.0.0-pr-34701-sha-181521d2 upgrade.More information
0.0.0-pr-34701-sha-181521d2valentin/change-detection-changes181521d21778090344)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=34701Summary by CodeRabbit
New Features
Behavior
Documentation
Tests