Optimize StandardTableView in Fluent, Material and Native styles #3425
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
|
|
bjorn
commented
Sep 7, 2023
|
@bjorn thank you very much for the PR. Would you like to add this optimization also for the |
By using a ListView instead of a ScrollView + VerticalLayout, we trigger the optimization where only visible children are being instantiated. For the Fluent style, this was making the rows more compact, so I've added additional padding to the TableViewCell to compensate. I don't know where the padding used to come from. I didn't touch Cupertino style since it's currently work-in-progress. This optimization can be done there as well. For the Native style, this optimization isn't as straight-forward because it uses a NativeScrollView + Flickable directly rather than a the ScrollView.
bjorn
force-pushed
the
standardtableview-listview
branch
from
0a74464
to
7a68c7d
Compare
|
@FloVanGH Alright, done. As for
Would still be nice if anybody had an idea why my change destroyed the original padding. |
Using the ListView makes it possible to only instantiate the visible items, which makes the StandardTableView a lot faster for larger models (already very noticeably in the gallery example). When using a ListView, StandardTableView lost access to the NativeScrollView.native-padding-top/left properties, which are now exposed through the ScrollView item. It could also no longer position the Flickable below the header (or in doing so, would take the scrollbar along with it), so a property was added to ScrollView for that purpose as well.
bjorn
force-pushed
the
standardtableview-listview
branch
from
7a68c7d
to
3096b06
Compare
bjorn
changed the title
|
So, the |
This avoids exposing additional properties on the ScrollView for just the native style.
FloVanGH
requested changes
Sep 7, 2023
Added minimum size, preferred size and stretch factors (they were not present before introducing InternalScrollView either, but are added now for consistency with the other styles).
FloVanGH
approved these changes
Sep 8, 2023
|
Looks good to me, thank you very much! |
ogoffart
added
the
candidate-for-bugfix-release
tronical
pushed a commit
that referenced
this pull request
Sep 15, 2023
* Optimize StandardTableView in Fluent and Material styles By using a ListView instead of a ScrollView + VerticalLayout, we trigger the optimization where only visible children are being instantiated. For the Fluent style, this was making the rows more compact, so I've added additional padding to the TableViewCell to compensate. I don't know where the padding used to come from. I didn't touch Cupertino style since it's currently work-in-progress. This optimization can be done there as well. For the Native style, this optimization isn't as straight-forward because it uses a NativeScrollView + Flickable directly rather than a the ScrollView. * Optimize StandardTableView for Native style Using the ListView makes it possible to only instantiate the visible items, which makes the StandardTableView a lot faster for larger models (already very noticeably in the gallery example). When using a ListView, StandardTableView lost access to the NativeScrollView.native-padding-top/left properties, which are now exposed through the ScrollView item. It could also no longer position the Flickable below the header (or in doing so, would take the scrollbar along with it), so a property was added to ScrollView for that purpose as well. * Introduced InternalScrollView This avoids exposing additional properties on the ScrollView for just the native style. * Fix missing default values for Native style ScrollView Added minimum size, preferred size and stretch factors (they were not present before introducing InternalScrollView either, but are added now for consistency with the other styles). (cherry picked from commit 74b37df)
tronical
removed
the
candidate-for-bugfix-release
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.
By using a
ListViewinstead of aScrollView+VerticalLayout, we trigger the optimization where only visible children are being instantiated. In my application it makes a very noticeable difference.For some reason, changing to
ListViewmade the existing vertical padding disappear. I don't know where this padding came from before, but I've restored it by adding padding to theTableViewCell.If this optimization is correct I can look into doing the same for other styles.