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

The header icons are not visible when choosing a light background #35482

Closed
szaimen opened this issue Nov 29, 2022 · 15 comments · Fixed by #42977
Closed

The header icons are not visible when choosing a light background #35482

szaimen opened this issue Nov 29, 2022 · 15 comments · Fixed by #42977
Labels
1. to develop Accepted and waiting to be taken care of design Design, UI, UX, etc. technical debt

Comments

@szaimen
Copy link
Contributor

szaimen commented Nov 29, 2022

How can we work around the following issue?

The header icons are not visible when choosing a light background image (same for admin chosen background image):
image

What about adding a switch that allows to invert the header icons? (I fear Ii don't see any other good solution to automate this)

cc @nimishavijay @jancborchardt

@szaimen szaimen added bug design Design, UI, UX, etc. 1. to develop Accepted and waiting to be taken care of 25-feedback 26-feedback labels Nov 29, 2022
@jancborchardt
Copy link
Member

I mean, they are also not properly visible if you pick a checkerboard background or something like that.

If you pick a custom background, the automatic color extraction by @skjnldsv will mostly take care of it (and set e.g. white as background color, so the icons would then be dark).

However if the majority of the image is not white, it can still be fixed by changing the color. No additional setting needed I'd say.

@szaimen
Copy link
Contributor Author

szaimen commented Nov 29, 2022

If you pick a custom background, the automatic color extraction by @skjnldsv will mostly take care of it (and set e.g. white as background color, so the icons would then be dark).

I fear this is not how it works in my testing unfortunately... See the screenshot I've sent. (It chose dark blue for an almost completel white picture...

@skjnldsv
Copy link
Member

If you pick a custom background, the automatic color extraction by @skjnldsv will mostly take care of it (and set e.g. white as background color, so the icons would then be dark).

No, this will not work reliably.
Have a look at an image like this, bright top-left header, so icons are not visible. But overall luminance is nowhere bright :)
Such edge cases are impossible to automate imho

image

@skjnldsv
Copy link
Member

However if the majority of the image is not white, it can still be fixed by changing the color. No additional setting needed I'd say.

The invert of the top header is not based on the primary colour.

@szaimen
Copy link
Contributor Author

szaimen commented Nov 29, 2022

What about adding a switch that allows to invert the header icons? (I fear Ii don't see any other good solution to automate this)

What about this idea?

@skjnldsv
Copy link
Member

skjnldsv commented Nov 30, 2022

What about adding a switch that allows to invert the header icons? (I fear Ii don't see any other good solution to automate this)

What about this idea?

For me the only one that would work.
Automating this is not easily (if at all) doable unfortunately

@florianwgnr

This comment was marked as resolved.

@szaimen
Copy link
Contributor Author

szaimen commented Dec 4, 2022

What about adding a switch that allows to invert the header icons? (I fear Ii don't see any other good solution to automate this)

What about this idea?

For me the only one that would work. Automating this is not easily (if at all) doable unfortunately

@jancborchardt would this be fine for you as well?

@florianwgnr

This comment was marked as resolved.

@susnux
Copy link
Contributor

susnux commented May 3, 2023

I do not think inverting the color does work here, as soon as you allow to set a background image you can not assume one color would work on the whole width of the menu bar.

I think the only solution working for every image is to not render the icon directly on the background but use a layer between with a fixed background like the dashboard panels.

This is also true for all other places where we render text directly on the background like the footer text on the login page, the site title on the header of public template or the "hello, NAME" on the dashboard.

So maybe we could add a option to enable a background on the header. I have created a mockup, it could look like this:

bright color mode
header-bright-mode
dark color mode
header-darkmode

@skjnldsv
Copy link
Member

skjnldsv commented May 4, 2023

@jancborchardt @nimishavijay ⬆️

@susnux
Copy link
Contributor

susnux commented May 4, 2023

After slept a night to think about it: While this fixes the dashboard, for every other app I am not sure if this is design-wise good 😕

files app files app dark files app with margin
rounded background on the headerbar rounded background on the header bar using dark color mode giving the rounded background on the headerbar some margin on the top and bottom

@nimishavijay
Copy link
Member

Yes, these are edge cases and unfortunately it would be difficult to make sure all cases are taken care of perfectly through automation. A switch for icons sounds the best to me as well, it can be a toggle with "Invert icon colors in the header" and it is off by default every time a new background is chosen.

A background for all the other items where there may be visibility issues would look like this:
image

I am not sure if this is a design we want. Perhaps if the background is causing so many issues it's simpler to just change the background :) cc @jancborchardt

@susnux
Copy link
Contributor

susnux commented May 4, 2023

I agree, so just adding the background on the greeting (dashboard) + login page would allow much more images so that adding an invert-icons switch should work :)

Especially on the login page we need an background, as even for the default image it breaks (currently only works for some users, see issue #36950 ).

@jancborchardt
Copy link
Member

jancborchardt commented May 4, 2023

I think on the login page yep that’s fine and needed, as you say @susnux. The "Welcome" text starts looking a bit strange and ugly with the container around. We could make the text a tiny bit bigger as well, which can also fix contrast issues.

And also tbh, custom backgrounds are not covered by contrast requirements. As said if there’s a checkerboard background, or a screenshot or similar, then that’s just not something you should choose. That’s why I would say we shouldn’t do anything like a container for the big "Welcome" text or the apps header.

For example, we should also never put the Nextcloud logo on the login page inside a container because that will just make it look very ugly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of design Design, UI, UX, etc. technical debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants