Skip to content
This repository has been archived by the owner on Aug 2, 2024. It is now read-only.

Update icons handling #657

Merged
merged 16 commits into from
Oct 8, 2020
Merged

Conversation

vladucu
Copy link
Member

@vladucu vladucu commented Oct 1, 2020

  1. Updates PolarisIcon to be compatible with latest polaris-react#v5
    version
  2. Updates build so that consuming apps no longer need to manually include
    Polaris icons. New icons can still be added through ember-svg-jar, or they
    can be passed directly as SVG's now.

NOTEs:

  1. Consuming apps should continue to work correctly with these changes,
    assuming previous Polaris icons have been set up correctly as documented
    through ember-svg-jar.
  2. Latest <PolarisIcon> allows passing an SVG as a string directly that will be rendered as an untrusted SVG using an img tag. Example

@vladucu vladucu self-assigned this Oct 1, 2020
@vladucu vladucu added the enhancement New feature or request label Oct 1, 2020
@vladucu vladucu force-pushed the v7-polaris-icons-build-and-component-updates branch from 2678c3b to 50ee873 Compare October 6, 2020 15:55
@vladucu vladucu marked this pull request as ready for review October 8, 2020 10:48
Copy link
Member

@andrewpye andrewpye left a 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
Copy link
Member

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? 🤔

Copy link
Member Author

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

docs/icon.md Outdated Show resolved Hide resolved
docs/migrating-from-v6-to-v7.md Outdated Show resolved Hide resolved
docs/migrating-from-v6-to-v7.md Outdated Show resolved Hide resolved
@@ -3,7 +3,7 @@ import { module, test } from 'qunit';
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';
Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ayyy!

@vladucu vladucu requested a review from andrewpye October 8, 2020 11:32
Copy link
Member

@andrewpye andrewpye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@vladucu vladucu merged commit 9d21893 into v7 Oct 8, 2020
@vladucu vladucu deleted the v7-polaris-icons-build-and-component-updates branch October 8, 2020 11:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants