[Ember]: Use native onclick #1608
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Feb 17, 2024
|
Thanks. I'll try to post results tomorrow. |
knownasilya
reviewed
Feb 21, 2024
| @@ -116,8 +116,8 @@ export default class MyTable extends Component { | |||
| {{#each this.data key="id" as |item|}} | |||
| <tr class={{if (eq item.id this.selected) 'danger'}}> | |||
| <td class="col-md-1">{{item.id}}</td> | |||
| <td class="col-md-4"><a {{on 'click' (fn this.select item)}}>{{item.label}}</a></td> | |||
| <td class="col-md-1"><a {{on 'click' (fn this.remove item)}}><span class="glyphicon glyphicon-remove" aria-hidden="true"></span></a></td> | |||
| <td class="col-md-4"><a onclick={{fn this.select item}}>{{item.label}}</a></td> | |||
There was a problem hiding this comment.
@NullVoxPopuli didn't know you could do this..
There was a problem hiding this comment.
ye, it's very nice -- I made a little decision tree to help with deciding between native event handlers and "event handlers with cleanup (on)": emberjs/rfcs#1007 (comment)
There was a problem hiding this comment.
When is cleanup necessary? Like components that get removed from view? Or ssr?
There was a problem hiding this comment.
cleanup is needed when:
- dynamic modifier is used (via a condition)
- or when the handler could change (the function itself)
|
Meanwhile chrome 122 was released. So it'll be included in the chrome 122 results soon. |
|
Now they're available: https://krausest.github.io/js-framework-benchmark/2024/table_chrome_122.0.6261.69.html |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's idiomatic to use
{{on}}for event listening, and{{on}}is the only way to have conditionally applied / dynamic event listeners, but for performance sensitive situations, we can do less by using the platform, and we don't need any cleanup behavior, so nativeonclickshould be sufficient, and I think I'm going to push for this being default in the ember learning materialsThe improvement to speed is noticeable
There was a concern mentioned by a community member about
onclickonly being able to be set once, but here is how this works in a real app, and it appears to update as one would expect.Results: