Add icon support to push buttons (#522)

macosui / macos_ui

Flutter widgets and themes implementing the current macOS design language.

https://macosui.github.io/macos_ui/#/

MIT License

1.88k stars 183 forks source link

Add icon support to push buttons (#522) #526

Open skythomp16 opened 3 weeks ago

skythomp16 commented 3 weeks ago

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

[x] I have incremented the package version as appropriate and updated CHANGELOG.md with my changes
[x] I have added/updated relevant documentation
[x] I have run "optimize/organize imports" on all changed files
[x] I have addressed all analyzer warnings as best I could

skythomp16 commented 3 weeks ago

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.

GroovinChip commented 3 weeks ago

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?

skythomp16 commented 3 weeks ago

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.

skythomp16 commented 3 weeks ago

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

GroovinChip commented 3 weeks ago

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.

skythomp16 commented 3 weeks ago

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:

skythomp16 commented 3 weeks ago

It looks like the dart-code-metrics failing isn't due to anything in my PR

GroovinChip commented 2 weeks ago

Yes, that check needs to be removed