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 header inversion bug #42963

Merged
merged 2 commits into from
Jan 22, 2024
Merged

fix header inversion bug #42963

merged 2 commits into from
Jan 22, 2024

Conversation

szaimen
Copy link
Contributor

@szaimen szaimen commented Jan 19, 2024

Should fix #42962

I am not really satisfied with the solution but it is what it is...

@szaimen szaimen added this to the Nextcloud 29 milestone Jan 19, 2024
@szaimen szaimen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 19, 2024
@szaimen szaimen marked this pull request as ready for review January 19, 2024 14:39
@szaimen
Copy link
Contributor Author

szaimen commented Jan 19, 2024

/backport to stable28

@@ -172,7 +172,20 @@
/* Right header standard */
.header-right {
> .header-menu:not(.user-menu) {
filter: var(--background-image-invert-if-bright);
// For general
> .header-menu__trigger {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
> .header-menu__trigger {
> button {

this should work for all I think

Copy link
Contributor Author

@szaimen szaimen Jan 19, 2024

Choose a reason for hiding this comment

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

I fear the assistant button is not a button but a link... (anker)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I would rather not risk using the button tag here...

Copy link
Contributor

Choose a reason for hiding this comment

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

Then we should probably adjust the two other that do not use .header-menu__trigger.
But for the moment your solution works fine!

Copy link
Contributor

Choose a reason for hiding this comment

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

Also I would rather not risk using the button tag here...

button,
a {}

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even more risky...

Copy link
Contributor

Choose a reason for hiding this comment

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

or even better only apply to img tag. Everything else should handle it correctly and we prevent future issues

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

works!

@blizzz
Copy link
Member

blizzz commented Jan 22, 2024

/compile amend /

szaimen and others added 2 commits January 22, 2024 09:44
Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: nextcloud-command <[email protected]>
@blizzz
Copy link
Member

blizzz commented Jan 22, 2024

manually rebased

@blizzz blizzz added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 22, 2024
@szaimen
Copy link
Contributor Author

szaimen commented Jan 22, 2024

CI failure unrleated afaics

@szaimen szaimen merged commit 93db067 into master Jan 22, 2024
42 of 44 checks passed
@szaimen szaimen deleted the szaimen-patch-1 branch January 22, 2024 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notifications menu and contact-search menu broken with certain predefined backgrounds
5 participants