Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Push Buttons with Icons #522

Open
skythomp16 opened this issue Oct 20, 2024 · 9 comments · May be fixed by #526
Open

Push Buttons with Icons #522

skythomp16 opened this issue Oct 20, 2024 · 9 comments · May be fixed by #526

Comments

@skythomp16
Copy link

Use case

I know that the current proposed solution to this problem is to use a MacosIconButton but the look this gives is not the same as a push button. In native SwiftUI, you can easily create a primary or (more commonly) a secondary push button with an icon in it.

Proposal

Expand the MacosIconButton class to allow you to style it like a push button with an easy toggle (something like how the PushButton class allows you to turn it into a secondary button by setting the "secondary" boolean property) OR simply allow passing an icon to the push button class to make it an icon button instead.

@GroovinChip
Copy link
Collaborator

This is part of my native UI research, so we will get this at some point.

@skythomp16
Copy link
Author

Are you open to a contribution to add that functionality? I can look at doing it.

@GroovinChip
Copy link
Collaborator

Are you open to a contribution to add that functionality? I can look at doing it.

Certainly. Look over our contribution guidelines, and try to keep it as close as possible to SwiftUI as you can.

@skythomp16
Copy link
Author

So after looking at this a bit last night it looks like it is currently possible to put an icon within a push button because the push button accepts a widget as its child and not specifically Text, it just doesn't look right because it turns it into a square. I am looking at patching it to where it looks like it does in SwiftUI - does that sound good?

@GroovinChip
Copy link
Collaborator

Yes, if you can make it match SwiftUI that would be great.

@GroovinChip
Copy link
Collaborator

@skythomp16 you can look at https://github.com/macosui/native-components for reference

@skythomp16
Copy link
Author

I have made the necessary changes to allow adding an icon to a push button by allowing you to pass in an IconData object (not an Icon object so that we can control the size). The way i've done this, it looks identical to a SwiftUI push button with an icon and text when I look at them side by side in macOS. However, the problem that now needs to be dealt with is having a push button with only an icon like SwiftUI supports...

Technically you could pass an empty text (Text('')) into the child object and populate iconData and it will work but the padding is a bit messed up. One way to solve this problem is by reducing the kXButtonSize variables in the push_button.dart class by allowing you to pass in an iconOnly flag as well. We can then also adjust the kXButtonPadding variables as well if that flag is passed in.

However, i'm not sure that's the right way to do this because we are basically saying the best way to do this is by putting an empty Text object as the child. It does seem to be at least the easiest way to add this functionality though. Do you have any good suggestions on how I should implement the Icon only capability here?

@GroovinChip
Copy link
Collaborator

I wouldn't worry about icon-only PushButtons because we have MacosIconButton for that.

skythomp16 added a commit to skythomp16/macos_ui that referenced this issue Oct 30, 2024
@skythomp16
Copy link
Author

I have opened a PR for this: #526

skythomp16 added a commit to skythomp16/macos_ui that referenced this issue Nov 2, 2024
skythomp16 added a commit to skythomp16/macos_ui that referenced this issue Nov 3, 2024
@GroovinChip GroovinChip linked a pull request Nov 7, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

2 participants