Use dynamic button label for button group #9834
Conversation
lukasmasuch
changed the title
…amic-button-label-for-button-group
lukasmasuch
added
security-assessment-completed
…amic-button-label-for-button-group
lukasmasuch
changed the title
| if len(transformed_parts) > 0: | ||
| maybe_icon = transformed_parts[0].strip() | ||
| try: | ||
| # we only want to extract material icons because we treat them |
There was a problem hiding this comment.
What was the reason to not treat emojis as icons?
There was a problem hiding this comment.
I am not sure why, @raethlein could know better, but this is based on the comment below
transformed = " ".join(transformed_parts[1:])
happened previously only for material icon cases, but not for emoji (transformed_parts are after applying format_func)
There was a problem hiding this comment.
I chatted with @raethlein and concluded that it is probably OK to merge it this way. There probably where some concerns around the styling / spacing.. but merging this makes it more consistent with how its treated in our buttons.
| label: string | ||
| icon?: string | ||
| label?: string | ||
| iconSize?: "sm" | "md" | "lg" | "base" | "xs" | "xl" | "twoXL" | "threeXL" |
There was a problem hiding this comment.
suggestion: Can we derive this by exporting the type from frontend/lib/src/theme/primitives/iconSizes.ts instead?
export const iconSizes = {
xs: "0.5rem",
sm: "0.75rem",
md: "0.9rem",
base: "1rem",
lg: "1.25rem",
xl: "1.5rem",
twoXL: "1.8rem",
threeXL: "2.3rem",
} as const
export type IconSize = keyof typeof iconSizesThere was a problem hiding this comment.
Yep sounds good 👍
There was a problem hiding this comment.
The type was already exported in @streamlit/lib/src/theme" -> so I just had to use it
…amic-button-label-for-button-group
## Describe your changes Reuse the `DynamicButtonLabel` for pills & segmented control. Currently, the usage of markdown in pills / segmented control is not limited -> which isn't desired. Using `DynamicButtonLabel` applies the same limits to the markdown as in all other button labels. This also changes the starting emojis to be treated as icons instead of text to have the same special icon spacing that we also have for emoji icons on buttons. ## Testing Plan - Only visual changes -> Update snapshots. --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.
lukasmasuch
deleted the
refactor/use-dynamic-button-label-for-button-group
branch
Describe your changes
Reuse the
DynamicButtonLabelfor pills & segmented control. Currently, the usage of markdown in pills / segmented control is not limited -> which isn't desired. UsingDynamicButtonLabelapplies the same limits to the markdown as in all other button labels.This also changes the starting emojis to be treated as icons instead of text to have the same special icon spacing that we also have for emoji icons on buttons.
Github Issues
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.