Fixing floating button style horizontal padding#2113
Fixing floating button style horizontal padding#2113daceball wants to merge 2 commits intomicrosoft:mainfrom
Conversation
| private var edgeInsets: EdgeInsets { | ||
| let size = size | ||
| let horizontalPadding = ButtonTokenSet.horizontalPadding(style: style, size: size) | ||
| let fabAlternativePadding = ButtonTokenSet.fabAlternativePadding(size) |
There was a problem hiding this comment.
Can you check to see how this affects FAB controls with text? IIRC this alternate padding was used to ensure that buttons with text had the right amount of padding, which was slightly higher on the trailing edge.
There was a problem hiding this comment.
Sure! In the Figma the trailing and leading padding have same values.
This is how the FAB with text looks after the update I made. Please let me know if it looks good and if there are other scenarios I should test. Thanks! (:
There was a problem hiding this comment.
@mischreiber Could you confirm if the FAB with text example I added above looks correct? thanks!
There was a problem hiding this comment.
There's something odd going on -- look at how much space there is between the right edge of the word Label and the actual bounding box that's being calculated. I'm investigating what's going on there now.
There was a problem hiding this comment.
Confirmed: there appears to be a 4pt spacing between the actual label's bounding box, and the box being measured against.
There was a problem hiding this comment.
It's definitely an intentional design decision. The component looks unbalanced without the extra padding.
There was a problem hiding this comment.
@mischreiber Thank you for confirming that this is by design. Could you please let me know if you agree with the fix I made by adding the isIconOnly parameter to the style init: FluentButtonStyle(style: .floatingSubtle, isIconOnly: true)? Thanks
There was a problem hiding this comment.
@daceball is there a way to internally determine if it's an iconOnly/textOnly button, I agree with Anand that this is a slightly painful api then
There was a problem hiding this comment.
@mischreiber Looks like ButtonStyle doesn’t have a built-in way to figure out if a button is icon-only or icon+text. In makeBody(configuration:), the configuration.label is just a View, so the style doesn’t get any extra info about whether there’s text or just an icon. We’d have to pass this info as a parameter (like in my last commit). I can think of the following approaches:
- Pass a parameter indicating the type of floating button: icon-only or icon+text.
- Create a new style type (e.g.,
style: .floatingSubtleIconOnly) and pick the right padding for each type. - Update the design and use the same padding for both icon-only and icon+text.
Let me know what you think about these options or if you have another idea to solve this bug. I'd really appreciate your input!
|
Closing this PR as it’s no longer needed |
4 participants
Platforms Impacted
Description of changes
Issue: #2112
Fixing UI bugs in
FluentButtonStyle(style: .floatingSubtle)style from the fluent library.###Actual behavior:
The icon in the Floating fluent button style is not centered because of some padding added to the icon. Icon must be centered. The fluent icon shape is an oval, but it must be a circle. This is the expected UI design: https://www.figma.com/design/OLIFtMJnKfD4FGcnNq9TVo/Fluent-2-iOS?node-id=54882-225133&t=UMzRDU5cGJ4wAVxL-4
###Expected behavior:
Icon must be centered, and button frame should be a circle.
Verification
The floating button horizontal margin from icon to frame must be symmetric.
Visual Verification
Pull request checklist
This PR has considered:
Microsoft Reviewers: Open in CodeFlow