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

Fix color of app menu entries and background color #43319

Merged
merged 2 commits into from
Feb 6, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Feb 3, 2024

Summary

This fixes two issues:

  1. a96a7f0 introduced a regression because it applied the color invert filter on the text which is already colored correctly (primary-text).
  2. Most of the time the background color is color-primary but for some shipped background images the primary color does not lead to the same text color -> app menu names and the right header menu entries have wrong colors.

Especially try following: Set default background -> white text -> click "remove image" -> suddenly the app menu color is black on the same blue.

A better solution will be #42977 but for the moment this one is simpler.

before after
Screenshot_20240203_140942 Screenshot_20240203_141051
Screenshot_20240203_141005 Screenshot_20240203_141035

Checklist

@susnux susnux added this to the Nextcloud 29 milestone Feb 3, 2024
@susnux
Copy link
Contributor Author

susnux commented Feb 3, 2024

/backport to stable28

Signed-off-by: Ferdinand Thiessen <[email protected]>
@susnux susnux force-pushed the fix/theming-background-primary branch from 9923e79 to 1ddc9b6 Compare February 3, 2024 13:28
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙈

@AndyScherzinger AndyScherzinger merged commit 375dade into master Feb 6, 2024
138 of 140 checks passed
@AndyScherzinger AndyScherzinger deleted the fix/theming-background-primary branch February 6, 2024 13:27
@skjnldsv
Copy link
Member

skjnldsv commented Feb 6, 2024

@AndyScherzinger the failure was related, revert in #43401

@blizzz blizzz mentioned this pull request Mar 5, 2024
@Jerome-Herbinet
Copy link
Member

Hi @AndyScherzinger
Not sure to understand this PR's history.
I still have the problem with Nextcloud 28.0.6.
Is this normal ? Bad fix or revert ? ... or will it be only fixed from NC 29?
Thanks

@AndyScherzinger
Copy link
Member

@Jerome-Herbinet according to the backport at #43392 it is shipped with 28.0.3, so if you already face issues in later versions of 28 I expect it to either be a regression or a new bug. So best to raise that in a new ticket since the source of the issue might be different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: NC 28.0.2 - Icons in toolbar are black instead of white
5 participants