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

Update polaris-spinner to v3.10.0 #308

Merged
merged 5 commits into from
Apr 2, 2019

Conversation

tomnez
Copy link
Contributor

@tomnez tomnez commented Apr 1, 2019

No description provided.

@tomnez tomnez force-pushed the update/spinner-v3.10.0 branch from 0d1f710 to 26a3a9e Compare April 1, 2019 15:36
@tomnez tomnez requested review from vladucu and andrewpye April 1, 2019 15:44
@tomnez tomnez self-assigned this Apr 1, 2019
}}
aria-label={{accessibilityLabel}}
data-test-polaris-spinner
>
Copy link
Member

Choose a reason for hiding this comment

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

Does this change mean we can remove the SvgHandling mixin from polaris-spinner?

Copy link
Member

Choose a reason for hiding this comment

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

yasss, let's remove that :D

'Integration | Component | polaris spinner',
{
integration: true,
module('Integration | Component | polaris spinner', function(hooks) {
Copy link
Member

Choose a reason for hiding this comment

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

🙌 Thanks for doing this! 🙇

@andrewpye
Copy link
Member

Copy link
Member

@vladucu vladucu left a comment

Choose a reason for hiding this comment

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

Some minor comments and what already mentioned....but looks good otherwise
🚀

return `polaris/spinner-${this.get('normalizedSize')}`;
spinnerSVG: computed('normalizedSize', function() {
let size = this.get('normalizedSize') === 'large' ? 'large' : 'small';
return `/@smile-io/ember-polaris/illustrations/spinner-${size}.svg`;
Copy link
Member

Choose a reason for hiding this comment

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

let's make a map with explicit image sizes, ex

const spinnerSVGSources = {
  small: '/@smile-io/ember-polaris/illustrations/spinner-small.svg'
  large: /@smile-io/ember-polaris/illustrations/spinner-large.svg
}

This is to avoid issues with fingerprinting

}}
aria-label={{accessibilityLabel}}
data-test-polaris-spinner
>
Copy link
Member

Choose a reason for hiding this comment

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

yasss, let's remove that :D

@tomnez tomnez merged commit c7b953a into update/polaris-v3.10.0 Apr 2, 2019
@delete-merged-branch delete-merged-branch bot deleted the update/spinner-v3.10.0 branch April 2, 2019 21:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants