-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
fix: Adapt the menubar to the primary color if a custom backgorund image has been set #36808
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also add a use case for this in the cypress tests please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesnt seem to work if you choose a dark primary color. Also the app title in the heading doesnt seem to be inverted correctly...
We kinda had that discussion with #35482 |
Won't have time looking into that further this week, so If anyone wants to jump in feel free. Otherwise I'll check again somewhen next week. |
Related: #37379 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One tiny comment, but all good otherwise
/backport to stable26 |
This comment was marked as resolved.
This comment was marked as resolved.
@skjnldsv the PR hasnt changed since I reviewed it so what shall I review here? |
19722c9
to
9797822
Compare
…age has been set Signed-off-by: Julius Härtl <jus@bitgrid.net>
9797822
to
710285d
Compare
I revived this and pushed a cypress test.
I could not reproduce this anymore |
Thinking more about it i don't think this is the right approach, as long as we have no detection or setting for if the background is dark or bright, we can only make it work in either combination. The primary color is only a good indicator for inverting app icons if no background is used. Will close this. |
Steps to reproduce:
Before
App icons where white on white
After
App icons properly adapt to the primary color