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

weather_status: Improve contrast of favorite icon #38333

Merged
merged 1 commit into from
May 26, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented May 17, 2023

Summary

Do not use a yellow icon but the default colored one and use the StarOff icon for removing a favorite instead of using StarOutline.

This also adds a feature: The current selected favorite is highlighted by setting the icon color to the primary color.

screenshots

before after
image image
image image
image image

Checklist

@susnux susnux added design Design, UI, UX, etc. 3. to review Waiting for reviews accessibility labels May 17, 2023
@susnux susnux requested review from jancborchardt, a team, nfebe, Pytal and szaimen and removed request for a team May 17, 2023 02:34
@szaimen
Copy link
Contributor

szaimen commented May 17, 2023

@nimishavijay @jancborchardt I think we need to think here how to properly display the current state: enabled <-> disabled

@susnux
Copy link
Contributor Author

susnux commented May 17, 2023

@nimishavijay @jancborchardt I think we need to think here how to properly display the current state: enabled <-> disabled

Yes, mainly we should decide whether we want to keep the yellow star or not, because we have the same problem on the files list (yellow star on white background => too low contrast).

@susnux susnux added this to the Nextcloud 28 milestone May 17, 2023
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

StarOff looks strange, cause it looks as if a click on it would remove the star.

Instead we should use the "non-filled" star icon for the state when it's not a favorite, that is more common.

Regarding color, just dark is fine, or do we find maybe a dark yellow/gold that still looks like a star color? In dark mode it can/must of course be light yellow.

@susnux
Copy link
Contributor Author

susnux commented May 17, 2023

do we find maybe a dark yellow/gold that still looks like a star color

@jancborchardt yellow will not work on white background, but dark golden at least fulfills the AA criteria of 3:1 contrast.
For bright color theme #B38F00 and for dark #E6B800. It will look like this:
image
image

Do you think this looks better than the default color?

@susnux susnux force-pushed the fix/weather-star-contrast branch from 7d0f782 to 6e524c6 Compare May 17, 2023 22:29
@jancborchardt
Copy link
Member

@susnux yep, I would say that dark gold is good! It at least conveys a little bit of a star color, otherwise favorite gets lost in the sea of other functions.

(Different issue, but we should probably also make sure that the "Delete" icon in all the action menus is colored accordingly – also something for the Penpot library @nimishavijay )

apps/weather_status/src/App.vue Outdated Show resolved Hide resolved
@susnux susnux force-pushed the fix/weather-star-contrast branch from 6e524c6 to f533928 Compare May 19, 2023 17:12
@susnux
Copy link
Contributor Author

susnux commented May 19, 2023

I had to adjust the color slightly to #a08b00 to guarantee the contrast even when hovered.

@szaimen
Copy link
Contributor

szaimen commented May 25, 2023

Conflicts

@szaimen szaimen added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 25, 2023
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the fix/weather-star-contrast branch from f533928 to b4ec3a3 Compare May 26, 2023 12:15
@susnux
Copy link
Contributor Author

susnux commented May 26, 2023

Conflicts

Resolved

@szaimen szaimen enabled auto-merge May 26, 2023 12:29
@szaimen szaimen removed the 2. developing Work in progress label May 26, 2023
@szaimen szaimen added the 4. to release Ready to be released and/or waiting for tests to finish label May 26, 2023
@szaimen szaimen merged commit a3baad1 into master May 26, 2023
@szaimen szaimen deleted the fix/weather-star-contrast branch May 26, 2023 13:26
@susnux susnux mentioned this pull request Jul 6, 2023
3 tasks
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 accessibility design Design, UI, UX, etc.
Projects
None yet
5 participants