Skip to content

Conversation

@sfc-gh-lwilby
Copy link
Collaborator

@sfc-gh-lwilby sfc-gh-lwilby commented Sep 22, 2025

Describe your changes

Adds a new height parameter to st.plotly_chart using the Height type system ("stretch", "content", or pixel values) for consistency with other chart elements.

Changes Made:

  • Added new parameter: Added height: Height = "stretch" parameter
  • Added native chart height support: When height="content", respects plotly.Figure's native height if specified
  • Enhanced parameter validation: Validates height parameter with appropriate error messages

Key Implementation Details:

Height Parameter Addition:

  • New functionality: Adds height parameter with default height="stretch"
  • Native height support: height="content" extracts height from native chart object if available, falls back to "stretch"

GitHub Issue Link (if applicable)

Testing Plan

  • Unit Tests (Python) - New height parameter functionality
  • Unit Tests (Python) - Native chart height extraction
  • E2E Tests - Visual height behavior across different values (height="content", height="stretch", height=400)
  • Manual testing completed for all height scenarios

Screenshots/Demos:
Screenshot 2025-09-22 at 4 16 26 PM
Screenshot 2025-09-22 at 4 16 32 PM

Additional Notes:

This extends the AdvancedLayouts initiative by adding height support to st.plotly_chart, bringing it in line with other chart elements that support the Height type system.


Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@sfc-gh-lwilby sfc-gh-lwilby added security-assessment-completed Security assessment has been completed for PR change:feature PR contains new feature or enhancement implementation impact:users PR changes affect end users labels Sep 22, 2025
@snyk-io
Copy link
Contributor

snyk-io bot commented Sep 22, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 22, 2025

✅ PR preview is ready!

Name Link
📦 Wheel file https://core-previews.s3-us-west-2.amazonaws.com/pr-12593/streamlit-1.50.0-py3-none-any.whl
📦 @streamlit/component-v2-lib Download from artifacts
🕹️ Preview app pr-12593.streamlit.app (☁️ Deploy here if not accessible)

@sfc-gh-lwilby sfc-gh-lwilby changed the title [AdvancedLayouts] Add height to st.plotly_chart [WIP][AdvancedLayouts] Add height to st.plotly_chart Sep 22, 2025
@sfc-gh-lwilby sfc-gh-lwilby force-pushed the feature/add-height-to-plotly-charts branch from 9b9cc6b to 0d3bb6c Compare September 22, 2025 15:43
Copy link
Collaborator Author

@sfc-gh-lwilby sfc-gh-lwilby Sep 22, 2025

Choose a reason for hiding this comment

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

I can't reproduce this change locally in chrome, firefox or safari, but it seems persistent in CI. With that in mind, it seems likely a minor impact to users if it is reproducible outside of CI.

@sfc-gh-lwilby sfc-gh-lwilby changed the title [WIP][AdvancedLayouts] Add height to st.plotly_chart [AdvancedLayouts] Add height to st.plotly_chart Sep 22, 2025
@sfc-gh-lwilby sfc-gh-lwilby force-pushed the feature/add-height-to-plotly-charts branch from 0a8bff4 to d6b0dce Compare September 23, 2025 12:51
@github-actions
Copy link
Contributor

github-actions bot commented Sep 23, 2025

📉 Frontend coverage change detected

The frontend unit test (vitest) coverage has decreased by 0.0200%

  • Current PR: 85.8600% (48407 lines, 6843 missed)
  • Latest develop: 85.8800% (48394 lines, 6830 missed)

✅ Coverage change is within normal range.

📊 View detailed coverage comparison

@sfc-gh-lwilby sfc-gh-lwilby force-pushed the feature/add-height-to-plotly-charts branch from 69f6ebc to 1c0aef3 Compare October 20, 2025 15:58
@sfc-gh-lwilby sfc-gh-lwilby force-pushed the feature/add-height-to-plotly-charts branch from 1c0aef3 to 4e24a57 Compare October 22, 2025 19:35
)
st.plotly_chart(fig, theme="streamlit")

# Width parameter tests
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had to split up these tests because otherwise the tests were timing out consistently in CI.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Question: This change seems a little strange to me - any idea why the movement in the time selection buttons?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nvm - just noticed your comment above

@sfc-gh-lwilby sfc-gh-lwilby merged commit 02a8507 into develop Nov 1, 2025
38 checks passed
@sfc-gh-lwilby sfc-gh-lwilby deleted the feature/add-height-to-plotly-charts branch November 1, 2025 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:feature PR contains new feature or enhancement implementation impact:users PR changes affect end users security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants