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

Consistent color scheme across all apps #37859

Open
mahibi opened this issue Apr 21, 2023 · 17 comments
Open

Consistent color scheme across all apps #37859

mahibi opened this issue Apr 21, 2023 · 17 comments
Assignees

Comments

@mahibi
Copy link

mahibi commented Apr 21, 2023

Nextcloud icon colors are different between NC files and NC talk (and most probably for other apps).

Icons for NC files (dark icons for light theme)
grafik

Icons for NC talk (light icons for light theme)
grafik

There should be a consistent color scheme across all apps.

@mahibi mahibi added enhancement design Design, UI, UX, etc. labels Apr 21, 2023
@szaimen szaimen transferred this issue from nextcloud/server Apr 21, 2023
@nickvergessen nickvergessen transferred this issue from nextcloud-libraries/nextcloud-vue Apr 21, 2023
@nickvergessen
Copy link
Member

This is not an issue in the vue app @szaimen
It's a generic thing we need to discuss that then needs implementing across all the board.

@nimishavijay
Copy link
Member

It's a generic thing we need to discuss that then needs implementing across all the board.

Some of the issues may solve themselves if the vue components are used across all apps. However, I do agree that these "placeholder" icons need to be standardised

@nimishavijay
Copy link
Member

What do you think about having 2 variants of icons, and depending on the use case one of these 2 will be used across the different apps:

  • One would be a "primary" icon type with the --color-primary-light background and a --color-primary SVG fill. This would be used in places where there are colorful avatars and avatar placeholder icons, like in Talk and Mail, as well as to indicate the primary action in a list of actions, eg. the share link action in the sharing sidebar.
  • The second would be a "secondary" icon type with --color-main-background bg and --color-main-text SVG fill. This would be used similar to a tertiary button with an icon. For eg, in "Others with access" and "Internal link" in the sharing sidebar or the icons in the details of a calendar event

Alternatives: "primary icon" which has primary background color and main-background SVG fill, "secondary icon" which has primary-light background and primary color SVG fill.

I would prefer the first option because the softer colors are more suited to the Nextcloud design, for eg. the avatar placeholders.
What do you think? @jancborchardt and @szaimen

@nickvergessen
Copy link
Member

In NcButton naming those are secondary and tertiary-no-background, correct?

I'm not sure about the missing background to be honest.

@nimishavijay
Copy link
Member

An example comparing the two options, what do you think?

image

My worry with introducing such a bright coloured icon is its overusage leading to a screen with many primary button looking elements, when the primary color usage should be quite restrained so that it is eye catching.

@szaimen
Copy link
Contributor

szaimen commented May 9, 2023

I think the biggest problem is still that we allow any color as primary color which introduces a lot of edge cases since the color is used in a lot of places and then always intoduces contrast problems with dark- and/or white-mode...

@jancborchardt
Copy link
Member

@szaimen in the left example of @nimishavijay though, the issue is not there with the bottom 2 icons as they are just normal icon color, not themed.
Then for the regular icon, the contrast logic is the same as for the secondary buttons. And there we also have contrast cutoff rules. If they don’t work properly, we need to adjust there.

@szaimen
Copy link
Contributor

szaimen commented May 9, 2023

The problem that I see is rather that so many combinations of the different elements like background color, primary color and others are possible which makes a automatic adjustment complicated and introduces a lot of edge cases which are really hard to catch. But I get that reducing it to a predefined set is not possible anymore since that would basically mean a regression.

And sorry for being off-topic here.

@nickvergessen
Copy link
Member

/**
* get color for on-page elements:
* theme color by default, grey if theme color is to bright
* @param string $color
* @param bool $brightBackground
* @return string
*/
public function elementColor($color, bool $brightBackground = true) {
$luminance = $this->calculateLuminance($color);
if ($brightBackground && $luminance > 0.8) {
// If the color is too bright in bright mode, we fall back to a darker gray
return '#aaaaaa';
}
if (!$brightBackground && $luminance < 0.2) {
// If the color is too dark in dark mode, we fall back to a brighter gray
return '#8c8c8c';
}
return $color;
}
takes care of that @szaimen

@szaimen
Copy link
Contributor

szaimen commented May 9, 2023

I am working on improving the prooblems that I mentioned in in nextcloud-libraries/nextcloud-vue#4067 and #38159

@tobiasKaminsky
Copy link
Member

Some of the issues may solve themselves if the vue components are used across all apps.

Please always be aware of clients, where there is no vue-components, so we shall have a central design info, where those info are available, so that every client/app dev/3rd party can rely on.

@jancborchardt
Copy link
Member

Yup @tobiasKaminsky, we should have them available in the "Color" section of our styleguide once it is decided. :) https://docs.nextcloud.com/server/latest/developer_manual/design/foundations.html#color

@szaimen
Copy link
Contributor

szaimen commented May 15, 2023

Added to #37859 (comment)

@jancborchardt jancborchardt removed their assignment Aug 22, 2023
@jancborchardt
Copy link
Member

What is left to do here @nimishavijay @szaimen? :)

@szaimen
Copy link
Contributor

szaimen commented Aug 22, 2023

With the new sharing flow, I think that part should already be resolved. Are there further places we should have a look at? Talk fox example now also has chat avatars which kind of resolves the issue, no? cc @nimishavijay

@nickvergessen
Copy link
Member

nickvergessen commented Aug 22, 2023

Talk fox example now also has chat avatars which kind of resolves the issue, no?

Kind of, but kind of also creates an issue:

Also just for the record, so far Talk is not aware that we would/should need to change anything. Is that correct? Are the colors to be used documented anywhere?
Edit: seems like it's https://docs.nextcloud.com/server/latest/developer_manual/design/foundations.html#color Maybe it should be added to https://docs.nextcloud.com/server/latest/developer_manual/app_publishing_maintenance/app_upgrade_guide/upgrade_to_28.html to raise awareness?

@szaimen
Copy link
Contributor

szaimen commented Aug 22, 2023

I think we have a general issue with avatar theming. See also e.g. #34503

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📐 At design
Development

No branches or pull requests

6 participants