Skip to content

Revert "Toolbar: Remove extra toolbar divider when zoom controls not shown"#34099

Merged
valentinpalkovic merged 1 commit intonextfrom
revert-33731-fix/toolbar-divider-21429
Mar 11, 2026
Merged

Revert "Toolbar: Remove extra toolbar divider when zoom controls not shown"#34099
valentinpalkovic merged 1 commit intonextfrom
revert-33731-fix/toolbar-divider-21429

Conversation

@valentinpalkovic
Copy link
Copy Markdown
Contributor

@valentinpalkovic valentinpalkovic commented Mar 11, 2026

Reverts #33731

Summary by CodeRabbit

  • Refactor
    • Simplified menu rendering logic by removing unnecessary view mode dependency, improving code maintainability without affecting user-facing functionality.

@valentinpalkovic valentinpalkovic self-assigned this Mar 11, 2026
@valentinpalkovic valentinpalkovic merged commit 2f7360f into next Mar 11, 2026
9 checks passed
@valentinpalkovic valentinpalkovic deleted the revert-33731-fix/toolbar-divider-21429 branch March 11, 2026 09:35
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 11, 2026

Fails
🚫 PR description is missing the mandatory "#### Manual testing" section. Please add it so that reviewers know how to manually test your changes.

Generated by 🚫 dangerJS against 04efb8b

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 11, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1e9da4b3-9c13-4cd6-a5fa-9db7fc25e0e1

📥 Commits

Reviewing files that changed from the base of the PR and between 943b2b6 and 04efb8b.

📒 Files selected for processing (1)
  • code/core/src/manager/components/preview/tools/menu.tsx

📝 Walkthrough

Walkthrough

Removed viewMode dependency from menu tool's Separator rendering logic in a single file. The Separator now renders unconditionally when the menu is shown, subject to existing !singleStory && !isVisible conditions. Minimal change affecting only 2-3 lines of code.

Changes

Cohort / File(s) Summary
Menu Tool Separator Logic
code/core/src/manager/components/preview/tools/menu.tsx
Removed viewMode from menuMapper result and destructured props; replaced conditional Separator rendering based on viewMode with unconditional rendering when menu is present (still gated by existing conditions).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • storybookjs/storybook#33731: Also modifies Separator rendering logic in the same file, but adds viewMode==='story' guard whereas this PR removes the viewMode condition entirely.
✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)

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

@github-actions github-actions bot mentioned this pull request Mar 11, 2026
10 tasks
@Sidnioulz
Copy link
Copy Markdown
Member

For history: I asked Valentin to revert this because there has been a misunderstanding on the PR author and reviewer's behalf. The PR modified the separator for the "Show sidebar" button which we wanted to keep as-is, instead of probably the separator for custom canvases which we have deprecated.

The original bug report is likely fixed during the 10.1 toolbar rewrite, but even if it wasn't, the UI code responsible for canvases and the separator will be removed in SB 11. So it's not worth investing more time into that.

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.

2 participants