-
Notifications
You must be signed in to change notification settings - Fork 19
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found some small things.
Regarding the new color variable I'm a bit confused now. I thought that the background-secondary
was supposed to be another surface color variable with the color value that was previously given to the highlight color. And that you wanted to come up with a completely new color for the already existing background-highlight
color. 🤔 could you resolve my confusion?
eade890
to
43258b5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Still not happy with the usage/changes regarding color variables (see detailed comment)
- Sorting icons in table header are only visible on hover in dark mode. In light mode not even on hover. In both modes not permanently for the currently sorted column.
But:
- OcCheckbox looks really good again 🥳
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍 Discussed in a call that you'll add the background-hover
color token for highlighting in the files table, that's also good to get in.
eeb4a9e
to
6721cb1
Compare
Kudos, SonarCloud Quality Gate passed! |
Description
See owncloud/web#6207
Types of changes
Checklist: