-
Notifications
You must be signed in to change notification settings - Fork 21
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
App 2049 icons update #267
App 2049 icons update #267
Conversation
@nathartman How do you like this iteration of the current icon set in Prime? I tried to make more consistent and use outline versions where it made sense. That being said, I'm happy to do whatever you suggest! Current set: https://prime.viam.com/?path=/docs/elements-icon--docs |
Looks great from what I can tell! Yay |
Here is the final set @nathartman ... there are some knock out versions of |
package.json
Outdated
@@ -95,5 +95,9 @@ | |||
"vite": "4.3.1", | |||
"vue": "^3.2.45", | |||
"vue-tsc": "^1.2.0" | |||
}, | |||
"dependencies": { | |||
"@mdi/js": "^7.2.96", |
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.
These should be devDependencies since we don't want them added into the node_modules
of anything downstream like app or rc
src/elements/icon.svelte
Outdated
aria-labelledby="name" | ||
role="img" | ||
> | ||
{#if sizes[size]} |
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.
Why do we need to check for size here?
src/elements/button/button.svelte
Outdated
|
||
let fill = ''; | ||
|
||
switch (variant) { |
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.
We should wrap this in a $
so that it's reactive (e.g. if a user updates the variant
prop we'd want fill to update)
$: {
switch (variant) { ... }
if (isDisabled) { ... }
}
Update icon library to use @mdi/js
Audit current icon set