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

primary/element color values for dark/light themes #21586

Closed
AndyScherzinger opened this issue Jun 25, 2020 · 6 comments · Fixed by #21594
Closed

primary/element color values for dark/light themes #21586

AndyScherzinger opened this issue Jun 25, 2020 · 6 comments · Fixed by #21594
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement

Comments

@AndyScherzinger
Copy link
Member

How to use GitHub

  • Please use the 👍 reaction to show that you are interested into the same feature.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Is your feature request related to a problem? Please describe.
The server's API allows the clients (in this case the Android files app) to read the theming values like the primary and the element color. As discussed with @tobiasKaminsky based on the feedback coming from @juliushaertl the primary color is the actual primary color and the element color is basically a background color aware primary color that has a fallback calculation for very, very low contrast primary colors (as in fallback to grey in case you use white primary on white background). This helps the clients with the theming regarding such a scenario. With nextcloud/android#6332 (comment) we (as in I) dropped the use of element color for wrong reasons. The reason being fallback handling for dark themes (supported by the mobile apps) since the element color logic doesn't work for dark themes (not present on server) and also not linked to the client. So at the moment we switched to using the server-side primary color with a client side fallback calculation for low-contrast dark/light scenarios. This is bad since all dark/light supporting client would have to re-implemend this.

Describe the solution you'd like
It be great if

  • the server calculated a primary color value for both worlds: (dark/light) backgrounds and
  • offers both values via the API

so the client could retrieve and store both variants and use them according to the app's chosen theming (dark/light). That way we would

  • have a central logic for all mobile apps which helps with its maintenance costs (only one piece of code in a central place)
  • made sure the colors are consistent throughout mobile apps that offer dark/light theming

Describe alternatives you've considered
The alternative is in place to some extend as-in implement the "color-correction" on the client-side which leads to the down-sides mentioned above.

Additional context
looping in @tobiasKaminsky for additional Android files app skills, @juliushaertl for server-side theming skills and color-calculation wizardry, @marinofaggiana for iOS feedback

@AndyScherzinger AndyScherzinger added enhancement 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Jun 25, 2020
@nickvergessen
Copy link
Member

nickvergessen commented Jun 25, 2020

Capabilities has a theming > color-text
Is that not sufficient?

https://github.com/nextcloud/server/blob/master/apps/theming/lib/Capabilities.php#L79

@AndyScherzinger
Copy link
Member Author

@nickvergessen I believe that none of these values is dark/light aware 🤔

@tobiasKaminsky
Copy link
Member

  • the server calculated a primary color value for both worlds: (dark/light) backgrounds and

Same should be done for element and font color 👍

@nickvergessen
Copy link
Member

Same should be done for element and font color +1

Well primary will not be touched, it is just the plain value. But you should use color-element to style the objects and color-text to write text on to it (as the font color on primary is always the same as on color-element).

I will add color-element-bright (same behaviour as color-element: brighter than 0.8 => #aaaaaa) and color-element-dark (inverted behaviour as color-element: darker than 0.2 => #555555

@AndyScherzinger
Copy link
Member Author

@nickvergessen so that means always use color-element for the "primary" color (default Nc blue) and use color-text for text on primary buttons (for example). So with the proposed change:

  • replace color-element with color-element-bright and color-element-dark
  • on (client) dark theme use color-element-dark
  • on (client) light theme use color-element-bright

@nickvergessen can you confirm this is how we should utilize the (then new) colors?

@nickvergessen
Copy link
Member

Yeah that's how I understand it too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants