[Performance improvements] Refactor SettingsListItem #186
Conversation
lib/pages/settings_list_item.dart
Outdated
|
|
||
| final LinkedHashMap<T, DisplayOption> optionsMap; | ||
|
|
||
| /// For ease of use. Correspond to the keys and values of [optionsMap]. |
There was a problem hiding this comment.
I feel like breaking the map into these 2 Iterables doesn't add much value. I suggest removing them.
If you think they should remain, then please make them private.
There was a problem hiding this comment.
Doing data structure operations felt kind of wrong in the build method, although that's purely based on feeling. I'll see to moving this to the _State class
There was a problem hiding this comment.
Did a bit of reading on these things and because it's implemented as a hash map + doubly linked list, obtaining the keys/values is inexpensive.
lib/pages/settings_list_item.dart
Outdated
| return AnimatedBuilder( | ||
| animation: _controller.view, | ||
| builder: _buildHeaderWithChildren, | ||
| child: Container( | ||
| constraints: const BoxConstraints(maxHeight: 380), |
There was a problem hiding this comment.
So is this the real crux of the fix? Just capping the height?
There was a problem hiding this comment.
Yes, I believe most of the gains are gotten through this
There was a problem hiding this comment.
since 380 / 8 = 47.5, and if this is to show exactly 8 items, should it maybe be something like 384? (maybe the height of the item is 48?)
There was a problem hiding this comment.
Good catch, this makes it fit perfectly
* Limit Settings list item height * Renaming * Make optionsMap keys & values private * Set correct height Former-commit-id: b631481
* Add snackbars demo Former-commit-id: 6410ee6
This PR refactor SettingsListItem for performance.
Closes #46
Web startup performance