UI: Fix code/copy buttons overlap with content#33889
Open
Conversation
|
View your CI Pipeline Execution ↗ for commit 0516ea7
☁️ Nx Cloud last updated this comment at |
Contributor
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs✨ Finishing Touches
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
🤖 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/components/components/ActionBar/ActionBar.stories.tsx`:
- Around line 30-32: The Storybook meta uses args.sb11FlexLayout which doesn't
match ActionBar's prop flexLayout and the stories don't pass args through the
render so toggling has no effect; rename the meta arg from sb11FlexLayout to
flexLayout and update the story renders (the story function that returns
<ActionBar ...>) to accept and spread args (e.g., (args) => <ActionBar {...args}
/>) so the flexLayout arg is forwarded to the ActionBar component.
Comment on lines
+30
to
+32
| args: { | ||
| sb11FlexLayout: true, | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate and examine the ActionBar component definition
fd -t f "ActionBar.tsx" --exclude "*.stories.tsx"Repository: storybookjs/storybook
Length of output: 124
🏁 Script executed:
#!/bin/bash
# Search for sb11FlexLayout usage throughout the codebase
rg -n "sb11FlexLayout" --type ts --type tsx --type js --type jsxRepository: storybookjs/storybook
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Examine the ActionBar.stories.tsx file to understand the current state
cat -n code/core/src/components/components/ActionBar/ActionBar.stories.tsxRepository: storybookjs/storybook
Length of output: 1445
🏁 Script executed:
#!/bin/bash
# Examine the ActionBar component definition
head -50 code/core/src/components/components/ActionBar/ActionBar.tsxRepository: storybookjs/storybook
Length of output: 1309
🏁 Script executed:
#!/bin/bash
# Search for sb11FlexLayout with correct flags (no -t for now)
rg "sb11FlexLayout"Repository: storybookjs/storybook
Length of output: 158
🏁 Script executed:
#!/bin/bash
# Run ESLint on the ActionBar.stories.tsx file
yarn lint:js:cmd code/core/src/components/components/ActionBar/ActionBar.stories.tsxRepository: storybookjs/storybook
Length of output: 283
🏁 Script executed:
#!/bin/bash
# Get the full ActionBar.tsx to see exported component and props
cat code/core/src/components/components/ActionBar/ActionBar.tsxRepository: storybookjs/storybook
Length of output: 2664
🏁 Script executed:
#!/bin/bash
# Also check if ActionBarProps type is defined elsewhere
rg "ActionBarProps" code/core/src/components/components/ActionBar/Repository: storybookjs/storybook
Length of output: 312
Ensure the default args map to ActionBar's flexLayout prop
The meta args use sb11FlexLayout, but this key does not match any prop on ActionBar (which expects flexLayout). Additionally, the stories render <ActionBar ...> without consuming args, so the toggle never reaches the component. Wire args into the story render and rename the arg to flexLayout.
Suggested adjustment
-import { ActionBar } from './ActionBar';
+import { ActionBar, type ActionBarProps } from './ActionBar';
export default {
component: ActionBar,
decorators: [
(storyFn: () => ReactNode) => (
<div
style={{
position: 'relative',
width: '300px',
height: '64px',
margin: '1rem',
background: 'papayawhip',
border: '1px solid rgba(0,0,0,.05)',
}}
>
{storyFn()}
</div>
),
],
- args: {
- sb11FlexLayout: true,
- },
+ args: {
+ flexLayout: true,
+ },
};
-export const SingleItem = () => <ActionBar actionItems={[{ title: 'Clear', onClick: action1 }]} />;
+export const SingleItem = (args: ActionBarProps) => (
+ <ActionBar {...args} actionItems={[{ title: 'Clear', onClick: action1 }]} />
+);
-export const ManyItems = () => (
- <ActionBar
- actionItems={[
- { title: 'Action string', onClick: action1 },
- { title: <div>Action node</div>, onClick: action2 },
- { title: 'Long action string', className: 'long-action-button', onClick: action3 },
- ]}
- />
-);
+export const ManyItems = (args: ActionBarProps) => (
+ <ActionBar
+ {...args}
+ actionItems={[
+ { title: 'Action string', onClick: action1 },
+ { title: <div>Action node</div>, onClick: action2 },
+ { title: 'Long action string', className: 'long-action-button', onClick: action3 },
+ ]}
+ />
+);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| args: { | |
| sb11FlexLayout: true, | |
| }, | |
| import { ActionBar, type ActionBarProps } from './ActionBar'; | |
| export default { | |
| component: ActionBar, | |
| decorators: [ | |
| (storyFn: () => ReactNode) => ( | |
| <div | |
| style={{ | |
| position: 'relative', | |
| width: '300px', | |
| height: '64px', | |
| margin: '1rem', | |
| background: 'papayawhip', | |
| border: '1px solid rgba(0,0,0,.05)', | |
| }} | |
| > | |
| {storyFn()} | |
| </div> | |
| ), | |
| ], | |
| args: { | |
| flexLayout: true, | |
| }, | |
| }; | |
| export const SingleItem = (args: ActionBarProps) => ( | |
| <ActionBar {...args} actionItems={[{ title: 'Clear', onClick: action1 }]} /> | |
| ); | |
| export const ManyItems = (args: ActionBarProps) => ( | |
| <ActionBar | |
| {...args} | |
| actionItems={[ | |
| { title: 'Action string', onClick: action1 }, | |
| { title: <div>Action node</div>, onClick: action2 }, | |
| { title: 'Long action string', className: 'long-action-button', onClick: action3 }, | |
| ]} | |
| /> | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/src/components/components/ActionBar/ActionBar.stories.tsx` around
lines 30 - 32, The Storybook meta uses args.sb11FlexLayout which doesn't match
ActionBar's prop flexLayout and the stories don't pass args through the render
so toggling has no effect; rename the meta arg from sb11FlexLayout to flexLayout
and update the story renders (the story function that returns <ActionBar ...>)
to accept and spread args (e.g., (args) => <ActionBar {...args} />) so the
flexLayout arg is forwarded to the ActionBar component.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 535 | 535 | 0 |
| Self size | 648 KB | 648 KB | 0 B |
| Dependency size | 59.86 MB | 59.90 MB | 🚨 +37 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 92 | 92 | 0 |
| Self size | 1.12 MB | 1.12 MB | 0 B |
| Dependency size | 22.39 MB | 22.43 MB | 🚨 +37 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 124 | 124 | 0 |
| Self size | 30 KB | 30 KB | 0 B |
| Dependency size | 23.68 MB | 23.72 MB | 🚨 +37 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 82 | 82 | 0 |
| Self size | 35 KB | 35 KB | 0 B |
| Dependency size | 20.17 MB | 20.21 MB | 🚨 +37 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 274 | 274 | 0 |
| Self size | 24 KB | 24 KB | 0 B |
| Dependency size | 44.52 MB | 44.56 MB | 🚨 +37 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 58 | 57 | 🎉 -1 🎉 |
| Self size | 1.19 MB | 1.23 MB | 🚨 +37 KB 🚨 |
| Dependency size | 13.20 MB | 12.94 MB | 🎉 -264 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
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 #23642
This is the proper fix for the PR I had to revert earlier today (#33877).
What I did
Used a wrapping flex container for Preview and SyntaxHighlighter. The copy buttons will now wrap to the next line if there isn't enough room to draw them without overlap with the content. Horizontal scrolling is preserved both with vanilla and Radix scrollbars.
Note: I chose not to port ActionLogger because it has a two-way scroll container that is a bit harder to get right and it has padding that makes the initial issue much less severe than for Preview.
Peek.2026-02-20.15-08.webm
Checklist for Contributors
Testing
A story was added to showcase the wrapping in preview.
The changes in this PR are covered in the following automated tests:
Manual testing
Documentation
ø
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
Release Notes