-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[DataGridPremium] Fix aggregation with sorting #19892
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
Open
lauri865
wants to merge
60
commits into
mui:master
Choose a base branch
from
lauri865:aggregation-sort-fix
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+507
−373
Open
Changes from 5 commits
Commits
Show all changes
60 commits
Select commit
Hold shift + click to select a range
8797b6c
fix aggregation with sorting
lauri865 ff426d6
fix aggregation reason
lauri865 8851650
tentative changes
lauri865 8916698
refactors
lauri865 8e542b8
use store effect equality check
lauri865 86d7e49
wip
lauri865 f2d411b
test opts
lauri865 067a960
fixes
lauri865 2653c29
wip
lauri865 d136a1a
lint
lauri865 b568d28
fix
lauri865 7eb1f6a
Merge branch 'master' into aggregation-sort-fix
lauri865 742b378
fix
lauri865 900eeec
refactor: virtualizer implemented in subcomponent
lauri865 a03a136
fix
lauri865 62b16f0
fixes
lauri865 30a1e05
console
lauri865 8f2537b
fixes
lauri865 352b5c3
fix
lauri865 0a5f3f0
fix
lauri865 0d345b5
tst
lauri865 4cb956b
dim
lauri865 ac62a97
Merge branch 'master' into aggregation-sort-fix
lauri865 87db6a2
virt
lauri865 1bf65c2
fix
lauri865 402f14d
lint
lauri865 57c9815
update
lauri865 1613657
remove unnecessary event handler
lauri865 a936867
log
lauri865 29fbafb
fix
lauri865 fdd510e
add an event for when aggregation finishes
lauri865 4bffff5
fix
lauri865 11ee56a
oops
lauri865 f50fde4
Merge branch 'master' into aggregation-sort-fix
lauri865 9fbeade
fix
lauri865 d46a0b3
Merge branch 'aggregation-sort-fix' of https://github.com/lauri865/mu…
lauri865 12feebd
remove hack for now
lauri865 1ab86c4
fix
lauri865 bc96ff9
fix
lauri865 b43dfdc
fix
lauri865 5199dfd
deduplicate row spanning calls
lauri865 9262c19
fixes
lauri865 806bc9a
fix test
lauri865 15cc4fb
fix
lauri865 7d1b7e0
fix keyboard test in the browser
lauri865 970ea94
fix
lauri865 25b27ad
test
lauri865 3b45c7b
test
lauri865 dae6c7f
empty
cherniavskii 217d960
lint
romgrk 1424219
fix tests
lauri865 3b070fa
reduce state updates
lauri865 f09246d
fix
lauri865 d5619fb
reduce calls to sorting / filtering
lauri865 12c7e97
update call count
lauri865 beb0a57
fix
lauri865 610a270
update call counts
lauri865 d60b398
lint
lauri865 e8a5d85
fix
lauri865 249c567
fix
lauri865 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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 file contains hidden or 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 file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
OK, so IIUC instead of calling
applyAggregation('sort')
on sort model change, sorted columns are now included in the first chunk, correct?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.
It's two different things.
applyAggregation('sort') needs to run only when there are sort dependent aggregations (rank, median, first/last, etc.).
But Sort columns were included in the initial chunk due to what I wrote in the next comment. We want them to be included in the first chunk (=these columns affect "viewport" due to potentially changing the row order, regardless of whether they are in the renderContext or not) so that we don't end up with a visual flicker.