-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update polaris-icon to v2.0.0 #123
Update polaris-icon to v2.0.0 #123
Conversation
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.
2.0 hotness! 🔥
for (const color of colors) { | ||
this.set('color', color); | ||
|
||
const colorClass = `Polaris-Icon--color${classify(color)}`; | ||
assert.ok(icon.classList.contains(colorClass), `icon with ${color} color applies ${colorClass} class`); | ||
assert.equal(icon.classList.length, 3, `icon with ${color} color does not add other color classes`); | ||
|
||
const colorClassNames = [...icon.classList].filter((className) => className.indexOf('Polaris-Icon--color') === 0); |
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 doesn't bug me being all on one line, but for the record if I did this, you'd have me split it up 😛
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.
Guess I thought that the intent of this line was more important than showing the details explicitly, but you're right, I should play by my own rules 😛
@@ -44,19 +48,30 @@ test('it applies colors correctly', function(assert) { | |||
'purple' | |||
]; | |||
|
|||
assert.expect(2 + colors.length * 3); |
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.
💯
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.
🚀
@@ -19,15 +20,13 @@ export default Component.extend({ | |||
|
|||
layout, | |||
|
|||
/* | |||
* Public attributes. |
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.
🎉
Updates
polaris-icon
to match the v2.0.0 React component by adding the internalisColored
class (required for proper icon rendering now), and setting the newfocusable
andaria-hidden
attributes on the icon's SVG element.