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.
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
feat: virtual list item selection #8318
base: main
Are you sure you want to change the base?
feat: virtual list item selection #8318
Changes from 38 commits
627689b
02e00f0
1287b6c
5273a0a
9892d7a
374e690
57f6dfd
8033ace
2516e39
e33511e
a549207
734d6e1
3d78d3f
a9fd9b0
8e7e837
2acdbf6
26af05c
e408af5
a407c4a
1854b36
c626bcb
7e2eb5b
d16de3d
5a9e6f9
6c8734a
33833c7
7c5e2ec
272c046
2c4dae6
24001d4
3fbfd1f
19ebae1
b61a645
28ded0e
275aac1
993af45
a0bc08a
c4802cf
a499cbd
20854b0
9b37d4d
3a5f2d4
bb503ee
d67dd5b
3b5bda3
86b695f
7901b01
70dbd70
46369a8
16ed88c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
You probably meant
...super.__getItemModel ? super.__getItemModel(index) : {}
? I used this approach earlier but it kind of fealt awkward to return an incomplete model{ selected: this.__isSelected(...) }
from a function named__getItemModel
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.
Yes, sorry for the mistake.
Yeah, I agree, it would be confusing. This problem seems related to the concern I mentioned in #8318 (comment). The current order in which mixins extend each other is a bit confusing in my opinion. I would expect that there would be a base mixin that defines the default model, with SelectionMixin extending this base mixin to add its own functionality, rather than the other way around.
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.
Done.
super.updateElement
need to be called at the end of the overridden function to have the element up-to-date by the time the renderer gets called. That's why I also needed to extract__updateElementRole
as a separate function.