Add icon support to push buttons (#522) #526
Conversation
|
I checked the box about updating documentation but I am just assuming that will get updated because of my comments in the push_button.dart class but is that true? If not, I need to update that an iconData can be passed. |
Your assumption is correct. Would you mind providing a screenshot of what the buttons look like, please? |
|
Sure, here is a screenshot of the modified example next to a blank canvas in swiftui with a push button with an icon for comparison. |
|
I pushed an update to fix the tests that were failing. Apologies - I didn't know that I could run 'flutter test' to run all of the tests - I will do that from now on. I am a little new to Flutter and Dart |
|
Do the SwiftUI versions of these buttons have this extra space on the left of the icon, for mini and small control sizes? I think it looks kind of lopsided. |
|
I think you're right. I pushed a change to allow us to control that padding. Here is a screenshot of what it looks like now: |
|
It looks like the dart-code-metrics failing isn't due to anything in my PR |
|
Yes, that check needs to be removed |
| /// An optional iconData that can be added to a button. | ||
| /// This is an iconData instead of icon so we can control the size |
There was a problem hiding this comment.
| /// An optional iconData that can be added to a button. | |
| /// This is an iconData instead of icon so we can control the size | |
| /// An optional `IconData` that can be added to a button. | |
| /// | |
| /// This is an `IconData` instead of an `Icon` so that the icon's size can be controlled internally. |
| child: Row( | ||
| children: [ | ||
| widget.iconData != null | ||
| ? Padding(padding: widget.controlSize.iconPadding,) |
There was a problem hiding this comment.
| ? Padding(padding: widget.controlSize.iconPadding,) | |
| ? Padding(padding: widget.controlSize.iconPadding) |
| widget.iconData != null | ||
| ? Padding(padding: widget.controlSize.iconPadding,) | ||
| : const SizedBox.shrink(), | ||
| widget.iconData != null ? Icon(widget.iconData, |
There was a problem hiding this comment.
Please change this Icon to a MacosIcon
| widget.iconData != null ? Icon(widget.iconData, | ||
| size: widget.controlSize.iconSize, | ||
| color: Colors.white) : const SizedBox.shrink(), | ||
| Padding( | ||
| padding: widget.controlSize.padding, | ||
| child: Align( | ||
| alignment: widget.alignment, | ||
| widthFactor: 1.0, | ||
| heightFactor: 1.0, | ||
| child: Row(children: [ | ||
| DefaultTextStyle( | ||
| style: widget.controlSize.textStyle( | ||
| baseStyle), | ||
| child: widget.child, | ||
| ), | ||
| ],) |
There was a problem hiding this comment.
Please run dart formaton this file
This PR adds support for adding an icon to a push button. This approach allows passing in an iconData instead of an icon so that we can control the size of the icon proportional to the button size. I have also updated the buttons_page.dart class for the example to include both primary and secondary buttons with icons. Comparing against a fresh SwiftUI project side by side, these look identical to those produced by it.
Pre-launch Checklist
CHANGELOG.mdwith my changes