-
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 resource list to Polaris v3.10.0 #306
Update resource list to Polaris v3.10.0 #306
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.
🎇 🔥 nicely done sir....considering the size of this component 👏
2 really small comments, but overall 👍
@@ -221,6 +222,7 @@ export default Component.extend(context.ConsumerMixin, { | |||
'selectMode', | |||
'element' | |||
); | |||
let { ctrlKey, metaKey } = event; |
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.
should this be event.nativeEvent
?
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.
No, React wraps all event objects in a SyntheticEvent
class, whose nativeEvent
property is the underlying browser event. We don't seem to have to worry about such things in Ember-land 🙌
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.
sweet 🙌
@@ -69,7 +73,7 @@ | |||
</div> | |||
{{/if}} | |||
|
|||
{{#if shortcutActions}} | |||
{{#if (and shortcutActions (not loading))}} | |||
{{#wrapper-element |
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.
why do we use this wrapper-element
here?
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.
I think to handle the click
and gain access to the event so that we can stop it propagating I think, this was from a while ago 😅
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.
kk
Adds support for
alternateTool
topolaris-resource-list
, improves handling of loading state, improves item link handling on click events, and updates the resource list subcomponents with the tweaks from Polaris v3.10.0.