-
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.
I'm missing changelog items for the added icons (and would expect them to follow our naming convention for resource-type icons), also it seems that most of them aren't included in the mapping?
I move icons to another issue&pr. Hover Issues that will be fixed by this pr & pr in web are discussed in Design polishing v3, should I create a separate issue for them? |
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.
LGTM but a changelog item would be needed 👍🏼
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.
LGTM 👍🏼 rebase needed ^^
@lookacat @pascalwengerter I guess this is ready to be merged? |
Kudos, SonarCloud Quality Gate passed! |
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.
LGTM 👍
icons have been removed from the PR (reason for request for changes)
Description
Related Issue
Fixes hover menus issues from owncloud/web#6555
Motivation and Context
More consistency in style of different menus:
Screenshots (if appropriate):
Types of changes
Checklist: