Skip to content

Conversation

@yannbf
Copy link
Member

@yannbf yannbf commented Nov 6, 2025

Closes #32825

What I did

This PR removes the duplicated element:
image

Additionally, the status icons are now kept on hover:
image

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make 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-32965-sha-87d93a51. Try it out in a new sandbox by running npx [email protected] sandbox or in an existing project with npx [email protected] upgrade.

More information
Published version 0.0.0-pr-32965-sha-87d93a51
Triggered by @yannbf
Repository storybookjs/storybook
Branch yann/improve-sidebar-context-menu-statuses
Commit 87d93a51
Datetime Mon Nov 24 19:23:49 UTC 2025 (1764012229)
Workflow run 19646539456

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=32965

Summary by CodeRabbit

  • Bug Fixes
    • Context-menu status indicators now reflect each item's actual status dynamically.
    • Status icons use precise visuals for leaf items (error, warning, success) and a simplified icon for non-leaf items.
    • Sidebar no longer shows redundant status links for non-story/group entries, reducing clutter.
    • Context menus subscribe to status updates to keep item status and icons current.

✏️ Tip: You can customize this high-level summary in your review settings.

@yannbf yannbf self-assigned this Nov 6, 2025
@yannbf yannbf added bug ui patch:yes Bugfix & documentation PR that need to be picked to main branch ci:normal labels Nov 6, 2025
@yannbf yannbf requested a review from ghengeveld November 6, 2025 12:12
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

📝 Walkthrough

Walkthrough

Context menu now reads StatusContext to compute per-item status and choose icons differently for leaf (story/docs) vs non-leaf (component/group) nodes. Tree no longer generates status links for non-leaf nodes; status links remain only for leaf items.

Changes

Cohort / File(s) Change Summary
Status-Aware Context Menu Rendering
code/core/src/manager/components/sidebar/ContextMenu.tsx
Subscribes to StatusContext; reads allStatuses and groupStatus; computes itemStatus (leaf: getMostCriticalStatusValue(allStatuses[item.id]); non-leaf: groupStatus[item.id] with fallback). Chooses MenuIcon based on node type and status (dot for non-leaf non-success/non-unknown, status icons for leaf, default Ellipsis). Wires itemStatus into FloatingStatusButton status prop. Adds/updates imports (getMostCriticalStatusValue, StatusContext, StatusValue, useContext, useSymbol) and updates memo dependencies. Adjusts test provider links filter cast to unknown as ExcludesNull.
Status Link Removal in Tree
code/core/src/manager/components/sidebar/Tree.tsx
Removes retrieval of counts and statusesByValue from useStatusSummary(item) and eliminates generation of status links for component/group nodes. Updates Node useMemo dependency list to drop counts/statusesByValue; per-item status links remain only for leaf items.

Sequence Diagram(s)

sequenceDiagram
  participant Tree as Tree
  participant ContextMenu as ContextMenu
  participant StatusCtx as StatusContext
  participant UI as FloatingStatusButton

  Note over Tree,ContextMenu: User opens context menu for a sidebar node
  Tree->>ContextMenu: open(node)
  ContextMenu->>StatusCtx: read allStatuses, groupStatus

  alt node is leaf (story/docs)
    ContextMenu->>StatusCtx: getMostCriticalStatusValue(allStatuses[node.id])
    ContextMenu-->>UI: render FloatingStatusButton(status=leafStatus, icon=statusIcon)
  else node is non-leaf (component/group)
    ContextMenu->>StatusCtx: lookup groupStatus[node.id] or default unknown
    ContextMenu-->>UI: render FloatingStatusButton(status=groupStatus, icon=dotOrEllipsis)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect status derivation in ContextMenu.tsx (leaf vs non-leaf, fallback).
  • Verify memo dependency updates to avoid stale renders.
  • Confirm removal of status-link generation in Tree.tsx doesn't break UI or tests.

Possibly related PRs

✨ Finishing touches
  • 📝 Generate docstrings

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
code/core/src/manager/components/sidebar/ContextMenu.tsx (1)

126-149: Consider memoizing isLeafNode for stable dependencies.

The isLeafNode variable is recalculated on every render and used as a dependency in the itemStatus useMemo (line 149), which may cause unnecessary recalculations when other props change but context.type remains the same.

- const isLeafNode = context.type === 'story' || context.type === 'docs';
+ const isLeafNode = useMemo(
+   () => context.type === 'story' || context.type === 'docs',
+   [context.type]
+ );

Note: The defensive check for !context on line 130 is unnecessary since context is a required parameter.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0484989 and 7fe0822.

📒 Files selected for processing (2)
  • code/core/src/manager/components/sidebar/ContextMenu.tsx (7 hunks)
  • code/core/src/manager/components/sidebar/Tree.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (use yarn lint:js:cmd <file>)

Files:

  • code/core/src/manager/components/sidebar/Tree.tsx
  • code/core/src/manager/components/sidebar/ContextMenu.tsx
**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Export functions from modules when they need to be unit-tested

Files:

  • code/core/src/manager/components/sidebar/Tree.tsx
  • code/core/src/manager/components/sidebar/ContextMenu.tsx
code/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

In application code, use Storybook loggers instead of console.* (client code: storybook/internal/client-logger; server code: storybook/internal/node-logger)

Files:

  • code/core/src/manager/components/sidebar/Tree.tsx
  • code/core/src/manager/components/sidebar/ContextMenu.tsx
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/core/src/manager/components/sidebar/Tree.tsx
  • code/core/src/manager/components/sidebar/ContextMenu.tsx
🧠 Learnings (3)
📚 Learning: 2025-11-05T09:37:25.903Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.903Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.

Applied to files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
📚 Learning: 2025-11-05T09:38:47.694Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.694Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.

Applied to files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
📚 Learning: 2025-11-05T09:36:55.919Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.919Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.

Applied to files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
🧬 Code graph analysis (1)
code/core/src/manager/components/sidebar/ContextMenu.tsx (4)
code/core/src/shared/status-store/index.ts (1)
  • StatusValue (9-14)
code/core/src/manager/utils/status.tsx (1)
  • getMostCriticalStatusValue (58-63)
code/core/src/manager/components/sidebar/IconSymbols.tsx (1)
  • UseSymbol (111-159)
code/core/src/manager/components/sidebar/Tree.tsx (1)
  • ExcludesNull (56-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Danger JS
  • GitHub Check: normal
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (5)
code/core/src/manager/components/sidebar/Tree.tsx (1)

239-258: LGTM! Dependency array correctly reflects actual usage.

The updated dependency array accurately captures the values used in the statusLinks callback, improving memoization precision.

code/core/src/manager/components/sidebar/ContextMenu.tsx (4)

48-48: LGTM! StatusContext integration is correct.

The StatusContext usage aligns with the provider setup in Tree.tsx, and the dependency arrays accurately reflect the values used in the callbacks.

Also applies to: 93-93


151-187: LGTM! Status-aware icon rendering logic is correct.

The MenuIcon calculation properly differentiates between leaf (story/docs) and non-leaf (component/group) nodes, rendering appropriate icons based on status. The logic aligns with the PR objective to improve status visibility.


212-218: LGTM! Dynamic status rendering correctly implemented.

The FloatingStatusButton now uses computed itemStatus and MenuIcon, and the dependency array on line 218 correctly includes all values used in the returned object.


269-269: Good type safety improvement.

Changing from any to unknown provides better type safety by requiring proper type narrowing.

@nx-cloud
Copy link

nx-cloud bot commented Nov 6, 2025

View your CI Pipeline Execution ↗ for commit 87d93a5

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 1m 2s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-24 19:39:42 UTC

@yannbf yannbf marked this pull request as draft November 6, 2025 14:48
@storybook-app-bot
Copy link

storybook-app-bot bot commented Nov 12, 2025

Package Benchmarks

Commit: 87d93a5, ran on 24 November 2025 at 19:32:31 UTC

No significant changes detected, all good. 👏

@yannbf yannbf marked this pull request as ready for review November 13, 2025 10:24
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7fe0822 and 0de2d7d.

📒 Files selected for processing (2)
  • code/core/src/manager/components/sidebar/ContextMenu.tsx (7 hunks)
  • code/core/src/manager/components/sidebar/Tree.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • code/core/src/manager/components/sidebar/Tree.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (use yarn lint:js:cmd <file>)

Files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Export functions from modules when they need to be unit-tested

Files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
code/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

In application code, use Storybook loggers instead of console.* (client code: storybook/internal/client-logger; server code: storybook/internal/node-logger)

Files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
🧠 Learnings (3)
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.

Applied to files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.

Applied to files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
📚 Learning: 2025-11-05T09:36:55.944Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.

Applied to files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
🧬 Code graph analysis (1)
code/core/src/manager/components/sidebar/ContextMenu.tsx (4)
code/core/src/shared/status-store/index.ts (1)
  • StatusValue (9-14)
code/core/src/manager/utils/status.tsx (1)
  • getMostCriticalStatusValue (57-62)
code/core/src/manager/components/sidebar/IconSymbols.tsx (1)
  • UseSymbol (111-159)
code/core/src/manager/components/sidebar/Tree.tsx (1)
  • ExcludesNull (56-56)
🔇 Additional comments (9)
code/core/src/manager/components/sidebar/ContextMenu.tsx (9)

2-2: LGTM: Imports added for status handling.

The new imports support the status-aware rendering logic introduced in this PR.

Also applies to: 10-10, 22-23, 25-25


48-48: LGTM: StatusContext consumption.

The context is properly consumed and provides the necessary status data for both leaf and non-leaf nodes.


93-93: LGTM: Dependency array correctly updated.

Including api in the dependency array is correct since it's referenced in the openInEditor callback.


126-149: LGTM: Status computation logic is well-designed.

The distinction between leaf nodes (story/docs) and non-leaf nodes (component/group) is appropriate. The logic correctly:

  • Aggregates multiple status values for stories/docs using getMostCriticalStatusValue
  • Uses group-level status for components/groups
  • Treats success and undefined as unknown for non-leaf nodes to maintain clean UI on hover

212-212: Status prop correctly passed, but incomplete without MenuIcon.

The itemStatus is correctly passed to FloatingStatusButton, but this change alone won't achieve the visual status indication mentioned in the PR objectives. The computed MenuIcon must also be rendered (see previous comment).


220-220: LGTM: Dependency array updated for itemStatus.

Including itemStatus in the dependency array is correct since it's used in the memoized JSX (line 212).


271-271: LGTM: Improved type safety.

Changing the type assertion from any to unknown is a good practice for better type safety.


1-271: Ensure formatting and linting before commit.

Per coding guidelines, please run Prettier and ESLint on this file before committing:

  • Prettier: Format the file
  • ESLint: yarn lint:js:cmd code/core/src/manager/components/sidebar/ContextMenu.tsx

126-149: Status computation correctly eliminates duplicates through proper node-type distinction.

The logic works as follows:

  1. Tree.tsx deduplication (lines 229-246): statusLinks are only generated for leaf nodes (stories/docs). Non-leaf nodes (components/groups) return an empty array, preventing duplicate status items from being added to the context menu.

  2. ContextMenu itemStatus computation (lines 126-149): Correctly distinguishes between node types:

    • Leaf nodes: Aggregates all statuses via Object.values(allStatuses?.[context.id] || {}), selecting the most critical value
    • Non-leaf nodes: Uses pre-computed groupStatus, showing a single aggregated status or 'unknown'
  3. Result: The context menu receives only leaf-node statusLinks, preventing duplicates. The itemStatus computation properly handles both cases without duplication.

Note: MenuIcon (lines 151-176) is computed but never rendered—the button uses a hardcoded <EllipsisIcon /> instead. Verify whether this is intentional dead code or if the rendering implementation is incomplete.

@valentinpalkovic valentinpalkovic added needs qa Indicates that this needs manual QA during the upcoming minor/major release and removed patch:yes Bugfix & documentation PR that need to be picked to main branch labels Nov 24, 2025
…om:storybookjs/storybook into yann/improve-sidebar-context-menu-statuses
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
code/core/src/manager/components/sidebar/ContextMenu.tsx (1)

151-187: MenuIcon mapping aligns with node type and status; consider reusing isLeafNode (optional)

The icon selection logic is consistent with the status model: non‑leaf nodes show a dot for any non‑success/non‑unknown group status and default to ellipsis; leaf nodes get specific symbols for error/warning/success with ellipsis as the fallback, which should satisfy the “keep status icons visible on hover” requirement.

To avoid duplicating the leaf/non‑leaf condition, you could optionally reuse isLeafNode here instead of repeating context.type !== 'story' && context.type !== 'docs', but that’s purely a readability tweak and not required for correctness.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a552e6 and 87d93a5.

📒 Files selected for processing (2)
  • code/core/src/manager/components/sidebar/ContextMenu.tsx (7 hunks)
  • code/core/src/manager/components/sidebar/Tree.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • code/core/src/manager/components/sidebar/Tree.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use camelCase for variable and function names

Files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Enable TypeScript strict mode
Export functions from modules for testing purposes

Files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
**/*.{ts,tsx,js,jsx,json,html,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx,js,jsx,json,html,mjs}: Use ESLint and Prettier for code style enforcement
Run 'yarn prettier --write ' to format code after making changes
Run 'yarn lint:js:cmd ' to check for ESLint issues after making changes

Files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
code/**/!(*.test).{ts,tsx,js,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/!(*.test).{ts,tsx,js,mjs}: Use 'logger' from 'storybook/internal/node-logger' for server-side (Node.js) logging, not console.log/console.warn/console.error
Use 'logger' from 'storybook/internal/client-logger' for client-side (browser) logging, not console.log/console.warn/console.error
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
🧠 Learnings (5)
📚 Learning: 2025-11-05T09:36:55.944Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.

Applied to files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.

Applied to files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.

Applied to files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
📚 Learning: 2025-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: The Storybook repository is a TypeScript/React monorepo with main codebase in 'code/' directory and tooling in 'scripts/' directory

Applied to files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.

Applied to files:

  • code/core/src/manager/components/sidebar/ContextMenu.tsx
🧬 Code graph analysis (1)
code/core/src/manager/components/sidebar/ContextMenu.tsx (4)
code/core/src/manager/utils/status.tsx (1)
  • getMostCriticalStatusValue (65-70)
code/core/src/components/index.ts (1)
  • PopoverProvider (71-71)
code/core/src/components/components/Popover/PopoverProvider.tsx (1)
  • PopoverProvider (55-106)
code/core/src/manager/components/sidebar/Tree.tsx (1)
  • ExcludesNull (56-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: normal
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/core/src/manager/components/sidebar/ContextMenu.tsx (3)

48-49: StatusContext usage and itemStatus derivation look correct; confirm provider is always present

The leaf vs non‑leaf status logic is coherent: leaf nodes derive status from allStatuses[context.id] via getMostCriticalStatusValue, while components/groups rely on groupStatus[context.id] and normalize success/undefined to status-value:unknown. This matches the intended “only show non‑ellipsis on non‑success group statuses” behavior and should avoid spurious indicators.

One thing to double‑check: const { allStatuses, groupStatus } = useContext(StatusContext); assumes StatusContext’s value is never undefined. Please confirm the context is always provided (or has a non‑null default like { allStatuses: {}, groupStatus: {} }) everywhere useContextMenu is used so we don’t risk a destructuring crash if a provider is missed in the tree.

Also applies to: 126-150


207-216: Context menu button now correctly reflects status and uses MenuIcon

Wiring status={itemStatus} into FloatingStatusButton and rendering {MenuIcon} instead of a hardcoded <EllipsisIcon /> resolves the earlier gap where the computed icon wasn’t actually shown. The updated useMemo dependencies (itemStatus, MenuIcon) ensure the hover button re-renders when status or icon mapping changes without introducing stale state.

Also applies to: 220-220


271-272: Type‑guard cast for filter is safer with unknown than any

Changing .filter(Boolean as unknown as ExcludesNull) from an any‑based cast keeps the runtime behavior (filtering out the null entries produced by the mapper) while better aligning with TypeScript’s strict typing for ExcludesNull. This is a good cleanup and should play nicely with strict mode.

@yannbf yannbf merged commit 1a860fd into next Nov 25, 2025
67 of 68 checks passed
@yannbf yannbf deleted the yann/improve-sidebar-context-menu-statuses branch November 25, 2025 08:22
@github-actions github-actions bot mentioned this pull request Nov 25, 2025
10 tasks
@yannbf yannbf removed the needs qa Indicates that this needs manual QA during the upcoming minor/major release label Nov 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Remove duplicate status items in context menu

4 participants