-
Notifications
You must be signed in to change notification settings - Fork 4
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
Resolve issue where rebuilds were over-eager. #2
Conversation
…rtino without extra import Reduce rebuilds only to active tab when switching tabs
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.
Thanks for checking out the PR!
Anything you have a problem with or unclear about the review content, just ask!
_alreadyLoaded[widget.index] = true; | ||
_children[widget.index] = widget.children[widget.index]; | ||
super.didUpdateWidget(oldWidget); |
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.
super.didUpdateWidget(oldWidget);
should be called first.
Implementations of this method should start with a call to the inherited method, as in super.didUpdateWidget(oldWidget).
ref: https://api.flutter.dev/flutter/widgets/State/didUpdateWidget.html
@@ -45,26 +45,33 @@ class LazyLoadIndexedStack extends StatefulWidget { | |||
class _LazyLoadIndexedStackState extends State<LazyLoadIndexedStack> { | |||
late final List<bool> _alreadyLoaded; | |||
|
|||
late List<Widget> _children; | |||
|
|||
final stackKey = GlobalKey(); |
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.
I'd prefer a variable name of _stackKey
, which is private.
@@ -45,26 +45,33 @@ class LazyLoadIndexedStack extends StatefulWidget { | |||
class _LazyLoadIndexedStackState extends State<LazyLoadIndexedStack> { | |||
late final List<bool> _alreadyLoaded; |
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.
Thanks to this modification, the variable _alreadyLoaded
is no longer needed, so I thought the whole code could look something like this, but guess what?
I named the function _loadedChildren
as _initialChildren
.
class _LazyLoadIndexedStackState extends State<LazyLoadIndexedStack> {
late List<Widget> _children;
final _stackKey = GlobalKey();
@override
void initState() {
super.initState();
_children = _initialChildren();
}
@override
void didUpdateWidget(final LazyLoadIndexedStack oldWidget) {
super.didUpdateWidget(oldWidget);
_children[widget.index] = widget.children[widget.index];
}
@override
Widget build(final BuildContext context) {
return IndexedStack(
key: _stackKey,
index: widget.index,
children: _children,
alignment: widget.alignment,
textDirection: widget.textDirection,
sizing: widget.sizing,
);
}
List<Widget> _initialChildren() {
return widget.children.asMap().entries.map((entry) {
final index = entry.key;
final childWidget = entry.value;
return index == widget.index ? childWidget : widget.unloadWidget;
}).toList();
}
}
final indexStackKey = GlobalKey(); | ||
|
||
final page1Key = GlobalKey(); | ||
final page2Key = GlobalKey(); | ||
final page3Key = GlobalKey(); | ||
|
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.
I would like to keep the code in Example to the minimum necessary.
If this code is not needed, would it be possible to have it removed along with the print debug(line 7375, 8789, 100~102)?
Resolves Issue #1 By using a static list of children and updating only the entry that changed, instead of reloading the children on every update.
Also uses widgets import instead of material, to allow this to be used in a pure Cupertino app.