[fix] Remove heading padding in horizontal containers for proper alignment#14419
[fix] Remove heading padding in horizontal containers for proper alignment#14419sfc-gh-lwilby merged 2 commits intodevelopfrom
Conversation
Fixes #12434 — headings (h1–h6) had vertical padding that caused misalignment with sibling elements in horizontal containers. Zero out the padding and compensating negative marginBottom when FlexContext indicates a horizontal layout. Made-with: Cursor Co-authored-by: lawilby <[email protected]>
sfc-gh-lwilby
added
change:bugfix
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ PR preview is ready!
|
sfc-gh-lwilby
added
the
ai-review
github-actions
bot
removed
the
ai-review
|
The AI review job failed to complete. Please check the workflow run for details. You can retry by adding the 'ai-review' label again or manually triggering the workflow. |
There was a problem hiding this comment.
Pull request overview
This PR fixes heading (st.title, st.header, st.subheader) vertical misalignment inside horizontal containers by removing the extra vertical padding that inflates heading height and by disabling the related negative margin compensation in that layout context.
Changes:
- Add a horizontal-layout flag to
StyledStreamlitMarkdownto remove h1–h6 padding when used in horizontal layouts. - Remove
StyledStreamlitMarkdown’s compensating negativemarginBottomin horizontal layouts. - Add unit tests to verify heading padding behavior differs between horizontal vs vertical layouts.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| frontend/lib/src/components/shared/StreamlitMarkdown/styled-components.ts | Adds horizontal-layout conditional heading padding + disables negative margin compensation in that mode. |
| frontend/lib/src/components/shared/StreamlitMarkdown/Heading.tsx | Reads FlexContext and passes horizontal-layout state to StyledStreamlitMarkdown. |
| frontend/lib/src/components/shared/StreamlitMarkdown/Heading.test.tsx | Adds tests asserting padding is removed in horizontal layout and preserved in vertical layout. |
| it("removes heading padding in horizontal layout", () => { | ||
| const props = getHeadingProps({ body: "hello", tag: "h1" }) | ||
| const horizontalContext: IFlexContext = { | ||
| direction: Direction.HORIZONTAL, | ||
| isInHorizontalLayout: true, | ||
| isInRoot: false, | ||
| isInContentWidthContainer: false, | ||
| } | ||
|
|
||
| render( | ||
| <FlexContext.Provider value={horizontalContext}> | ||
| <Heading {...props} /> | ||
| </FlexContext.Provider> | ||
| ) | ||
|
|
||
| const markdownContainer = screen.getByTestId("stMarkdownContainer") | ||
| expect(markdownContainer).toHaveStyle({ "margin-bottom": "" }) | ||
|
|
||
| const heading = screen.getByRole("heading") | ||
| expect(heading).toHaveStyle({ padding: "0" }) | ||
| }) |
There was a problem hiding this comment.
The new style assertions are likely brittle/incorrect for jest-dom: toHaveStyle expects concrete CSS values and typically camelCase keys. {"margin-bottom": ""} is not a valid CSS value (computed style is usually 0px or the property is omitted), and padding: "0" may not match the computed value (0px). Consider asserting with camelCase (marginBottom) and checking for 0px (or asserting the negative margin is not applied), and for padding prefer asserting paddingTop/paddingBottom are 0px in horizontal layout and non-zero in vertical layout.
There was a problem hiding this comment.
Thanks for the review. The assertions here are intentional and match how Emotion generates CSS:
padding: "0"— The styled component setspadding: 0(the number) whenisInHorizontalLayoutis true.toHaveStyle({ padding: "0" })correctly matches this since jest-dom normalizes it. CI confirms the test passes.marginBottom: ""— The styled component explicitly setsmarginBottom: ""(empty string) to effectively unset the negative margin compensation. This is the actual value emitted by Emotion, so asserting against it is correct.
Also fixed the import ordering lint violation (IsSidebarContext before Layout/FlexContext) and updated the E2E snapshots for the horizontal-tabs layout in the follow-up commit.
Fix eslint import/order violation in Heading.test.tsx (IsSidebarContext must sort before Layout/FlexContext) and update horizontal-tabs E2E snapshots to reflect the heading padding removal. Made-with: Cursor Co-authored-by: lawilby <[email protected]>
Describe your changes
Fixes #12434
Headings (
st.title,st.header,st.subheader) have vertical padding that causes misalignment when placed in horizontal containers. The padding inflates the heading's box height, making it taller than sibling elements like buttons, sovertical_alignment="center"doesn't visually align the text.FlexContextmarginBottomonStyledStreamlitMarkdownin horizontal layoutsScreenshot or video (only for visual changes)
GitHub Issue Link (if applicable)
Fixes #12434
Testing Plan
Heading.test.tsxverifying padding is removed in horizontal layout and preserved in vertical layoutverify.pyapp showing title, header, and subheader alignment in horizontal containers with center/top/bottom alignmentContribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.