-
Notifications
You must be signed in to change notification settings - Fork 4k
[feature] support width="content" on st.container
#12848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ 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!
|
| > & { | ||
| node: ElementNode | ||
| } | ||
| > = ({ node, ...rest }) => { | ||
| const { isInHorizontalLayout } = useRequiredContext(FlexContext) | ||
|
|
||
| let minStretchBehavior: MinFlexElementWidth = "fit-content" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default for minStretchBehavior is now determined in useLayoutStyles only.
e8ab812 to
a7b4028
Compare
6da4b36 to
336c094
Compare
| // Buffer to account for padding/margins (in pixels) | ||
| const PADDING_BUFFER = 32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: This should either be defined outside of the scope of this function, or added as an optional argument with a default set to 32
| } | ||
|
|
||
| export type MinFlexElementWidth = "fit-content" | "14rem" | "8rem" | undefined | ||
| export type MinFlexElementWidth = "14rem" | "8rem" | "fit-content" | undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Seems like an unnecessary change. Consider reverting this to maintain a smaller diff.
| """Snapshot test for the container with map in st_layouts_container_various_elements.py.""" | ||
|
|
||
| # Wait for map elements to load | ||
| # Wait for map elements to load (now we have 2 maps) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Consider reworking this comment and the one above so that they are more durable over time, rather than referencing a change that was made in this specific commit.
| ) | ||
| st.map(map_data, width="stretch") | ||
|
|
||
| # fixed width container with width 100 and a dataframe inside with stretch width |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment '# fixed width container with width 100 and a dataframe inside with stretch width' violates the Python Guide rule that requires comments to be capitalized, use proper grammar and punctuation. The comment should start with a capital letter: '# Fixed width container with width 100 and a dataframe inside with stretch width.'
| # fixed width container with width 100 and a dataframe inside with stretch width | |
| # Fixed width container with width 100 and a dataframe inside with stretch width |
Spotted by Graphite Agent (based on custom rule: Python Guide)
Is this helpful? React 👍 or 👎 to let us know.
| if (parentWidth < minWidthInPixels && parentWidth > buffer) { | ||
| return `${parentWidth - buffer}px` | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a safeguard for when parentWidth is less than or equal to buffer, which would result in a negative or zero width. This edge case should be handled to ensure the calculated width is always positive:
if (parentWidth < minWidthInPixels && parentWidth > buffer) {
return `${parentWidth - buffer}px`
} else if (parentWidth <= buffer) {
// Return a small minimum width when parent is extremely narrow
return "1px" // or some other reasonable minimum
}| if (parentWidth < minWidthInPixels && parentWidth > buffer) { | |
| return `${parentWidth - buffer}px` | |
| } | |
| if (parentWidth < minWidthInPixels && parentWidth > buffer) { | |
| return `${parentWidth - buffer}px` | |
| } else if (parentWidth <= buffer) { | |
| // Return a small minimum width when parent is extremely narrow | |
| return "1px" // or some other reasonable minimum | |
| } |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
…ments_test Skip on Chromium due to 1px snapshot width mismatch in CI. See failing workflow: https://github.com/streamlit/streamlit/actions/runs/18964912763/job/54180100557\n\nRecent snapshot touched by 9d3a016 (#12891); feature introduced in 5409b98 (#12848). We'll stabilize and re-enable later.
Describe your changes
Content width containers will size themselves based on the content you place inside.
Note: If you are using elements with
width="stretch"inside of a container withwidth="content"then some elements will shrink to their minimum size. Use fixed widths or content width on these elements to render them larger.Example: maps don't have a defined content width so they will render at the minimum width. Define a larger fixed width value to make them bigger when inside a content width container!
This is similar for dataframes. Use
width=contentor a fixed width to show the entire dataframe when inside a content width container.GitHub Issue Link (if applicable)
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.