-
Notifications
You must be signed in to change notification settings - Fork 68
Port sidebar icon to material #981
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
Conversation
Signed-off-by: Carl Schwan <[email protected]>
29393fc to
2f5da2e
Compare
| id: 'activity', | ||
| name: t('activity', 'Activity'), | ||
| icon: 'icon-activity', | ||
| icon: LightningBolt, |
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.
Sexy, didn't we could do that :)
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.
Don't get too excited, this is a bad approach I regret implementing 🙈
We can have conflicts in vue template compilation versions between those renders methods
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.
The real good way is to import the icon as svg, similarly.
Then we can render the svg inline with sanitization. And forcing the same color="currentColor" trick the vue-material-design-icons is doing 😉
- https://nextcloud.github.io/nextcloud-files/interfaces/Entry.html#iconSvgInline
- https://github.com/nextcloud/nextcloud-upload/blob/7037cccbc34bdba250c659f300ebc9ac6d39b84a/lib/components/UploadPicker.vue#L36
- https://github.com/nextcloud/nextcloud-upload/blob/7037cccbc34bdba250c659f300ebc9ac6d39b84a/lib/components/ActionIcon.vue
- https://github.com/nextcloud/nextcloud-upload/blob/7037cccbc34bdba250c659f300ebc9ac6d39b84a/lib/components/ActionIcon.vue#L56
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.
Oh wait, I thought that was my code @CarlSchwan
https://github.com/nextcloud/server/pull/34769/files#diff-91cd7c7f024a3f52536789f769ad017e1b60f17806d42a8342f19f67caaee4ee
I also did that in other locations 😁
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.
@skjnldsv is there a good way to import material icons as svg? or do we end up copying the svg in a bunch of places?
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.
No it's as easy as what you're doing here :)
Just use the official material design icon
nextcloud-libraries/nextcloud-upload#444 (comment)
import FolderPlus from '@mdi/svg/svg/folder-plus.svg'|
If it works, fine by me. but cypress is unhappy |
|
Done in #1154 |
Require nextcloud/server#34769