macosui / macos_ui

Flutter widgets and themes implementing the current macOS design language.
https://macosui.github.io/macos_ui/#/
MIT License
1.79k stars 174 forks source link

wrong PushButton color #486

Open pyxiscloud opened 9 months ago

pyxiscloud commented 9 months ago

Description After update to 2.0.2, PushButton colors always blue, even if I set it to another color

Steps To Reproduce

PushButton( controlSize: ControlSize.large, color: Colors.redAccent, ) ```[tasklist] ### Tasks ``` ```[tasklist] ### Tasks ```
GroovinChip commented 9 months ago

Thanks for reporting this bug @pyxiscloud. I've reproduced it.

@Adrian-Samoticha could this be related to the changes we shipped for PushButton in version 2.0.1? We listen for the system accent color now, but I don't recall if that change means we disregard the color property now. I don't think that's the case, because we have this function: _getBackgroundColor

As highlighted, line 285 shows that we default to the color property.

I observed that we were not adding the backgroundColor to our BoxDecoration, as highlighted on line 360 below, but that did not resolve the issue.

building_the_button

However, adding it to the Container does result in the button being the specified color; unfortunately, it also results in the rest of the button's styling being completely ignored:

Screenshot 2023-10-15 at 2 06 20 PM

I am stumped.

Adrian-Samoticha commented 9 months ago

That’s interesting. I’ll try to reproduce it and see what I can find out.

EDIT: The container exists solely to apply the “click effect” decoration because the Container widget offers a foregroundDecoration property and that decoration needed to be in the foreground. It makes sense that adding a color to it would hide the decoration of its parent DecoratedBox.

_getBackgroundColor seems to only be used to determine the foreground color (the text is either white or black depending on the brightness of the background color), so that explains why the color doesn’t show up.

The reason why adding the background color to the BoxDecoration doesn’t do anything is that it is styled using a gradient now, rather than a singular color.

You can assign me to that issue, I think I know how to fix it.


That said, @pyxiscloud, if the reason for why you would like to change the background color of the button is that you would like to style the entire application in that particular color, the proper way to do so is like this:

Open the macos/Runner.xcworkspace folder of your project using Xcode and search for the Assets.xcassets file. Then, hit the plus-button and choose “Color Set”:

image

Rename the newly created color set to AccentColor and choose your preferred color:

Screenshot 2023-10-16 at 14 14 46

Then, go to Runner.xcodeproj > Build Settings and under “Asset Catalog Compiler – Options” locate the “Global Accent Color Name” property, and set it to AccentColor:

Screenshot 2023-10-16 at 14 11 44

Violà:

Screenshot 2023-10-16 at 14 14 15

The benefit of this approach is that, for one, macos_ui will choose a gradient that matches those of native macOS applications, and for two, the user will still be able to override the color via the System Settings, which is how native macOS applications behave.

I guess it wouldn’t hurt to add this guide to macos_ui’s readme, come to think of it.

pyxiscloud commented 9 months ago

will "color" be deprecated for PushButton?

kwang87 commented 9 months ago

I used a pushbutton to remove the background color and use it like a text button. Is this impossible now?

Adrian-Samoticha commented 9 months ago

will "color" be deprecated for PushButton?

No, but if you’re trying to style the entire application, rather than just a single button, the way I described is the way to do so. The fact that the color property doesn’t change the color of the button is, indeed, a bug that will need to be fixed.

Adrian-Samoticha commented 9 months ago

I used a pushbutton to remove the background color and use it like a text button. Is this impossible now?

That’s possible, but this is considered a bug, so it will be fixed.

EDIT: That said, I am not aware of any native text buttons. Are you trying to emulate a specific macOS UI component that is missing in macos_ui?

ralph-bergmann commented 7 months ago

The workaround doesn't work for all buttons/UI elements for me :-(

SCR-20231207-mbes

I wonder why it doesn't just use the MacosThemeData primaryColor for this? Isn't that what the accent color is for?

On the other hand, the workaround fixes #395.

Adrian-Samoticha commented 7 months ago

The workaround doesn't work for all buttons/UI elements for me :-(

On the other hand, the workaround fixes #395.

Currently, only the buttons support accent colors. #484 introduces accent color support for the sidebar items but has not yet been merged. The other widgets are, of course, planned, but development is rather slow right now.

Adrian-Samoticha commented 7 months ago

I wonder why it doesn't just use the MacosThemeData primaryColor for this? Isn't that what the accent color is for?

I tried applying the colors provided by appkit_ui_element_colors to the widgets directly, however, this didn’t make them look accurate to their native counterparts. It’s necessary to overhaul each widget individually to make it look good.

Adrian-Samoticha commented 6 months ago

@GroovinChip While #484 is somewhat related to this issue, it doesn’t actually resolve it. I’ll quickly reopen this issue and see if I’ll have time to work on it sometime.

GroovinChip commented 6 months ago

Ah yeah, you're right. My bad. Thanks for catching this!

ralph-bergmann commented 6 months ago

I don't know if each widget should read the color from the system itself. Then, the theme's primary color becomes useless. If a user of this library wants to overwrite the color, he has to do this manually for each widget, as the theme no longer supports it (the PushButtonTheme is already deprecated).

IMHO, this should be done by the theme (primaryColor ?== readSystemnAccentColor()), and all widgets should use the primaryColor from the theme.

Adrian-Samoticha commented 6 months ago

I don't know if each widget should read the color from the system itself. Then, the theme's primary color becomes useless. If a user of this library wants to overwrite the color, he has to do this manually for each widget, as the theme no longer supports it (the PushButtonTheme is already deprecated).

IMHO, this should be done by the theme (primaryColor ?== readSystemnAccentColor()), and all widgets should use the primaryColor from the theme.

In order to make the macos_ui components’ appearance as accurate to their native counterparts as possible, the supported colors currently use hard-coded gradients for each system accent color setting, rather than being generated from a single color. Therefore, that wouldn’t really be possible without sacrificing accuracy to macOS’ native UI components.

The intended way to theme the entire application is the one I’ve outlined above, and I am planning to add this guide to macos_ui’s readme, once all widget actually support system accent colors.

Adrian-Samoticha commented 2 months ago

The workaround doesn't work for all buttons/UI elements for me :-(

I know it’s been a while, but version 2.0.7 applies the accent color to most widgets now.

akhudek commented 2 months ago

My use case for this is to make destructive buttons like delete red. Is there another way of doing this?

Adrian-Samoticha commented 2 months ago

My use case for this is to make destructive buttons like delete red. Is there another way of doing this?

Apple likes to style these buttons by making the text red, rather than the button’s background:

image

Since the PushButton class takes a widget as a child, you could probably just supply it with a red Text widget. Be sure to set the secondary property of the PushButton to true to prevent it from being colored and I think you’re good to go.