Button: add support for custom text sizes in button component #1864
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enables the button component to support custom text sizes by fixing the .button_text_size() method which was previously being overridden. The implementation updates the button_text_size method in styled.rs to handle all Size variants explicitly and removes the hardcoded text size application in the button's render method.
Key Changes:
- Enhanced
button_text_size()to explicitly handle all Size enum variants (XSmall, Small, Medium, Large, and custom Size values) - Removed hardcoded
.button_text_size(self.size)call that was overriding custom text sizes - Added comprehensive test examples demonstrating custom text sizing capabilities
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/ui/src/styled.rs | Updated button_text_size() to explicitly match all Size variants instead of using catch-all pattern, enabling proper handling of Large size and custom pixel sizes |
| crates/ui/src/button/button.rs | Removed hardcoded .button_text_size(self.size) from h_flex container and cleaned up imports to allow custom text sizes to take effect |
| crates/story/src/stories/button_story.rs | Added test section demonstrating buttons with different text sizes (Large, Medium, Small, XSmall, and custom pixel values) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .id("label") | ||
| .w_full() | ||
| .items_center() | ||
| .justify_center() |
There was a problem hiding this comment.
Removing .button_text_size(self.size) here fixes the issue where custom text sizes were being overridden, but it creates a new problem: buttons that don't explicitly call .button_text_size() will no longer have appropriate default text sizes based on their size variant.
The proper fix requires applying .button_text_size(self.size) to self.base (around line 540, before .refine_style(&self.style)). This would ensure that:
- Buttons have default text sizes matching their size variant (XSmall → text_xs, Small → text_sm, Medium → text_base, Large → text_lg)
- Custom text sizes set via
.button_text_size()on the Button instance override the default (since.refine_style()is applied after the default)
Without this default, existing buttons that rely on automatic size-appropriate text sizing will break.
| Size::Small => self.text_sm(), | ||
| _ => self.text_base(), | ||
| Size::Medium => self.text_base(), | ||
| Size::Large => self.text_lg(), |
There was a problem hiding this comment.
Large and custom size button should keep the text base.
There was a problem hiding this comment.
What if I want to display small text inside a large button?
2 participants
Description
Currently, the button component does not allow setting the label text size, even if specified within the button
.button_text_size(Size::XSmall), it does not take effect. This PR makes the font setting method work correctly.Screenshot