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

wrong PushButton color #486

Open
pyxiscloud opened this issue Oct 14, 2023 · 16 comments · Fixed by #484
Open

wrong PushButton color #486

pyxiscloud opened this issue Oct 14, 2023 · 16 comments · Fixed by #484
Assignees
Labels
bug:verified For bugs that have been successfully reproduced

Comments

@pyxiscloud
Copy link

pyxiscloud commented Oct 14, 2023

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, )

Tasks

Preview Give feedback
No tasks being tracked yet.

Tasks

Preview Give feedback
No tasks being tracked yet.
@pyxiscloud pyxiscloud added the bug Something isn't working label Oct 14, 2023
@GroovinChip GroovinChip added bug:verified For bugs that have been successfully reproduced and removed bug Something isn't working labels Oct 15, 2023
@GroovinChip
Copy link
Collaborator

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
Copy link
Member

Adrian-Samoticha commented Oct 16, 2023

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
Copy link
Author

will "color" be deprecated for PushButton?

@kwang87
Copy link

kwang87 commented Oct 19, 2023

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

@Adrian-Samoticha
Copy link
Member

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
Copy link
Member

Adrian-Samoticha commented Oct 19, 2023

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
Copy link

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
Copy link
Member

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
Copy link
Member

Adrian-Samoticha commented Dec 7, 2023

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.

@GroovinChip GroovinChip linked a pull request Jan 16, 2024 that will close this issue
4 tasks
@GroovinChip GroovinChip moved this to In Review in macos_ui Jan 16, 2024
@GroovinChip GroovinChip added this to the General Native Parity milestone Jan 16, 2024
@github-project-automation github-project-automation bot moved this from In Review to Done in macos_ui Jan 17, 2024
@Adrian-Samoticha
Copy link
Member

@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
Copy link
Collaborator

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

@ralph-bergmann
Copy link

ralph-bergmann commented Jan 18, 2024

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
Copy link
Member

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
Copy link
Member

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
Copy link

akhudek commented May 7, 2024

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

@Adrian-Samoticha
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:verified For bugs that have been successfully reproduced
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants