-
Notifications
You must be signed in to change notification settings - Fork 7
Update icons handling #657
Conversation
This converts `PolarisIcon` to a glimmer component. Now Polaris icons are automatically working without consuming apps needing to manually copy/paste SVG assets.
2678c3b to
50ee873
Compare
andrewpye
left a comment
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.
This is great stuff @vladucu! 🙌 🔥
Left a couple of suggestions for grammar tweaks in the docs, and I was also wondering if removing the SVG handling mixin for removing fills etc. could impact any other components (either in ember-polaris or in downstream usages) 🤔
| @@ -0,0 +1,25 @@ | |||
| <span | |||
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.
Out of curiosity, why isn't this in addon/templates/components? 🤔
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.
This co-locates the template and component file. Change done through codemod & RFC here.
This will be needed for all components in order to support Octane-only consuming apps.
Btw, sorry, wanted to, but completely forgot to mention this in the description.
PS: These can also be nested, but for now went with the default mode. We can change later probably if we feel it's better for us
| import { setupRenderingTest } from 'ember-qunit'; | ||
| import { findAll, click, render } from '@ember/test-helpers'; | ||
| import buildNestedSelector from '../../helpers/build-nested-selector'; | ||
| import MockSvgJarComponent from '../../mocks/components/svg-jar'; |
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.
Can we remove the mock SVG jar component now?
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.
ayyy!
Co-authored-by: Andy Pye <andrewpye@users.noreply.github.com>
andrewpye
left a comment
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.
🚀
PolarisIconto be compatible with latestpolaris-react#v5version
Polaris icons. New icons can still be added through
ember-svg-jar, or theycan be passed directly as SVG's now.
NOTEs:
assuming previous Polaris icons have been set up correctly as documented
through
ember-svg-jar.<PolarisIcon>allows passing an SVG as a string directly that will be rendered as an untrusted SVG using animgtag. Example