Skip to content

[do not merge] dirty selection model for angular and react #799

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

Closed
wants to merge 1 commit into from

Conversation

krausest
Copy link
Owner

This is currently not intended to be actually merged.
This implements the dirty model (selected is part of the row model) for discussion in #772


select(item: Data, event: Event) {
event.preventDefault();
for (let i=0;i<this.data.length;i++) {
Copy link

Choose a reason for hiding this comment

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

If you check if each row === selected, like the main branch does, then changing the value of selected will cause all rows to need to be checked by change detection. When using @for and track by item.id, then changing the id of the two rows that you are changing here should be a 500 times faster solution. For more detail read on. Best wishes and very impressive benchmark. Although this one number appears wrong for angular.

Each iteration of the loop is checking length, but it can be set once.
Only one item in the list has clazz = 'danger', so looping all items is not the optimal solution.
Simply store the index of the selected item in a class property; use this value to find one row to set to ''.
this.selectedRowIndex = this.data.indexOf(item);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants