-
Notifications
You must be signed in to change notification settings - Fork 166
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
Added play, pause, reboot and stop icons #4710
Conversation
Demo starting at https://vanilla-framework-4710.demos.haus |
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 am not too familiar with all the constraints for vanilla. Can someone more familiar with it have a 2nd eye on this one?
The power-off/power-error duplicate was already there. I saw it and thought it was fine to have a duplicate for reboot/restart as well. If instead this is judged to be wrong, I can remove either of the reboot/restart icons and keep just one of them - the old one, probably, should be kept. For the other duplicate, feel free to file an issue as I feel it's out of the scope of this PR, but I can remove also that one if someone instructs me about which one should be kept - this has potential implications, there might be apps relying on either of them and we would break them by removing one now. |
We shouldn't be adding icons if they already exist. From the screenshot, seems both power and reboot already exist. |
The "power" icon is different. And not really relevant to the icons added by this PR - both in terms of visual appearance and in terms of function, probably. The 4 icons next to each other are attached to this JIRA task which is currently blocked by this PR. |
179d9f9
to
628b98f
Compare
Removed the |
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.
Great, thanks!
Done
In the context of having icons for instance management (LXD-UI, Anbox Cloud dashboard...), this PR adds 4 new icons approved by design to the "Additional" icon set.
QA
Check if PR is ready for release
If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:
Feature 🎁
,Breaking Change 💣
,Bug 🐛
,Documentation 📝
,Maintenance 🔨
.package.json
should be updated relative to the most recent release, following semver convention: