-
Notifications
You must be signed in to change notification settings - Fork 11
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
[Reverted][Experimental] Add icon components #483
Conversation
const component = `// THIS FILE IS GENERATED. ANY CHANGES TO THIS FILE WILL BE LOST. | ||
import type { TOC } from '@ember/component/template-only'; | ||
|
||
export interface ${componentName}IconSignature { |
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.
Since the interface is static, this could live separately and be imported into each file, which would simplify the generated file slightly.
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.
Yea, but since all the files are generated it feels a bit strange to store a common type file somewhere, from which they would import. I don't really see an added value, other than DRYing up the generated code a tiny bit, but we would edit the generation script if we would want to make changes anyways, so no real benefit there either. I would prefer to keep it like this for simplicity sake.
I can poke at it if you want. Where would you put the type? We don't really have a "shared" type file in the project yet, AFAIK.
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.
It was just a similar strange feeling with generating something that is static, but I don't see a strong reason to pick one feeling over the other.
2c33e84
to
0585c9c
Compare
- set `mergeAmbiguousCharacters` to true since some of the icons contain numbers in the name: circle-step-1 > CircleStep_1Icon vs CircleStep1Icon - rename some variables - shorten some code
I need to double check that this runs when we want, but we were already using it for the symbolset, so I guess it does.
It seems the prepare script has some issues and we can't always depend on it for donig the right thing. We'll just run the build script when needed.
c27be7f
to
6cf23ae
Compare
I forgot to squash merge.. my bad.. |
Reverted this change and reopened as #487 |
This adds icon components for each .svg file in the
/public/icons
folder. We don't register them in the global component namespace, to use them you need to import them in your JS files and pass them through the template context. This a lot more intuitive in the .gjs world, but it still works in loose-mode as well, but it is more verbose.This PR isn't very useful by itself, but together with #481 it can be used to replace the string based icon setup.
Warning
This setup is still in an experimental state. We want to try it out in a couple of specific places before considering this ready to use by all the Appuniversum users. We might still break the setup in future minor versions until we announce it as stable.
Usage examples
(These examples assume #481 is merged)
Polaris
Octane
Part of the solution for #482