-
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 avatar to Polaris v3.10.0 #286
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.
Looks good to me, one small comment only
class="Polaris-Avatar__Image" | ||
src={{finalSource}} | ||
alt="" | ||
role="presentation" | ||
onload={{action "handleLoad"}} |
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.
just double-checking these are the right events here in Ember world
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.
Well, I initially went with {{action "handleLoad" on="load"}}
element modifiers and tried tinkering with that, but apparently that approach doesn't work here so this is what I landed on. The updated tests cover this functionality and with the exception of a timing issue in the tests for Ember 2.16/2.18, they pass without problem.
Updates
polaris-avatar
to match Polaris v3.10.0. I haven't added the newisServer
functionality since we don't need it at this point. Also dropped the old tests in favour of porting the new ones added inpolaris-react
since we initially builtpolaris-avatar
.