UI: Ensure consistent heading anchor margin across themes#34035
UI: Ensure consistent heading anchor margin across themes#34035travisbreaks wants to merge 1 commit into
Conversation
The anchor link icon on docs headings was clipped in the light theme because OcticonHeaders lacked left padding to accommodate the floated OcticonAnchor's negative margin (-24px). The dark theme appeared unaffected due to different container styling. Add paddingLeft: 24 and marginLeft: -24 to OcticonHeaders so the anchor icon has consistent space in both themes, while keeping heading text aligned with surrounding content. Closes storybookjs#31505 Co-authored-by: Egger <egger@horny-toad.com>
📝 WalkthroughWalkthroughAdds left padding (24px) and negative left margin (-24px) to Octicon header element styles in the MDX blocks documentation component. This is purely a styling adjustment with no functional logic changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/addons/docs/src/blocks/blocks/mdx.tsx (1)
147-148: Extract the 24px anchor gutter into a shared constant.These offsets now have to stay in sync with
OcticonAnchor’smarginLeft: '-24px'on Line 168. Keeping the gutter as repeated literals makes this easy to regress the next time the anchor spacing is adjusted.♻️ Proposed refactor
+const OCTICON_ANCHOR_GUTTER = 24; + const OcticonHeaders = SUPPORTED_MDX_HEADERS.reduce( (acc, headerType) => ({ ...acc, [headerType]: styled(headerType)({ - paddingLeft: 24, - marginLeft: -24, + paddingLeft: OCTICON_ANCHOR_GUTTER, + marginLeft: -OCTICON_ANCHOR_GUTTER, '& svg': { position: 'relative', top: '-0.1em', visibility: 'hidden', @@ const OcticonAnchor = styled.a( () => ({ float: 'left', lineHeight: 'inherit', paddingRight: '10px', - marginLeft: '-24px', + marginLeft: `-${OCTICON_ANCHOR_GUTTER}px`, // Allow the theme's text color to override the default link color. color: 'inherit', }) as const );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/docs/src/blocks/blocks/mdx.tsx` around lines 147 - 148, The paddingLeft/marginLeft literal 24px used in the block styles should be extracted into a shared constant (e.g., ANCHOR_GUTTER) and referenced wherever that spacing is applied (the style block using paddingLeft and marginLeft and the OcticonAnchor component which currently uses marginLeft: '-24px'); replace the numeric literals with the constant (and the negative form for the anchor) so both the block styles and OcticonAnchor stay in sync and avoid future regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code/addons/docs/src/blocks/blocks/mdx.tsx`:
- Around line 147-148: The paddingLeft/marginLeft literal 24px used in the block
styles should be extracted into a shared constant (e.g., ANCHOR_GUTTER) and
referenced wherever that spacing is applied (the style block using paddingLeft
and marginLeft and the OcticonAnchor component which currently uses marginLeft:
'-24px'); replace the numeric literals with the constant (and the negative form
for the anchor) so both the block styles and OcticonAnchor stay in sync and
avoid future regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 83dd966c-02b6-448f-97a1-a115bc5fbac1
📒 Files selected for processing (1)
code/addons/docs/src/blocks/blocks/mdx.tsx
|
Closing. A PR already exists to fix the bug: #33957 |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 76 KB | 76 KB | 0 B |
| Dependency size | 32.19 MB | 32.17 MB | 🎉 -19 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 139 KB | 139 KB | 0 B |
| Dependency size | 30.40 MB | 30.38 MB | 🎉 -19 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 189 | 189 | 0 |
| Self size | 15 KB | 15 KB | 🚨 +18 B 🚨 |
| Dependency size | 28.90 MB | 28.88 MB | 🎉 -19 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 534 | 534 | 0 |
| Self size | 648 KB | 648 KB | 🎉 -120 B 🎉 |
| Dependency size | 59.86 MB | 59.84 MB | 🎉 -19 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 | 🚨 +36 B 🚨 |
| Dependency size | 22.47 MB | 22.45 MB | 🎉 -19 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.76 MB | 23.74 MB | 🎉 -19 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.25 MB | 20.24 MB | 🎉 -19 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 271 | 271 | 0 |
| Self size | 24 KB | 24 KB | 🚨 +12 B 🚨 |
| Dependency size | 44.53 MB | 44.51 MB | 🎉 -19 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 197 | 197 | 0 |
| Self size | 16 KB | 16 KB | 🚨 +12 B 🚨 |
| Dependency size | 33.44 MB | 33.42 MB | 🎉 -19 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 183 | 183 | 0 |
| Self size | 779 KB | 779 KB | 0 B |
| Dependency size | 67.35 MB | 67.33 MB | 🎉 -19 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 176 | 176 | 0 |
| Self size | 32 KB | 32 KB | 🚨 +36 B 🚨 |
| Dependency size | 65.87 MB | 65.86 MB | 🎉 -19 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 167 | 167 | 0 |
| Self size | 18 KB | 18 KB | 0 B |
| Dependency size | 31.40 MB | 31.38 MB | 🎉 -19 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 58 | 58 | 0 |
| Self size | 1.19 MB | 1.19 MB | 0 B |
| Dependency size | 13.21 MB | 13.19 MB | 🎉 -19 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
Summary
paddingLeft: 24andmarginLeft: -24toOcticonHeadersto create consistent space for the floatedOcticonAnchoricon across both light and dark themesDetails
The
OcticonAnchorcomponent usesfloat: leftwithmarginLeft: -24pxto position the link icon to the left of heading text. However, theOcticonHeadersstyled components had no left padding to accommodate this, causing the icon to be clipped at the container edge in certain theme configurations.Closes #31505
Test plan
Co-authored-by: Egger egger@horny-toad.com
Summary by CodeRabbit