[H] Created a variant of InheritedWidget specifically for Listenables #23393
Conversation
Hixie
force-pushed
the
inherited_notifier
branch
from
266755a to
a427837
Compare
There was a problem hiding this comment.
Now it calls updated which calls notifyClients by default.
There was a problem hiding this comment.
I suppose updateShouldNotify is now a bit of a misnomer, since notifyClients is only called by default. I'm not suggesting that the method should be called updateShouldCallUpdated though.
There was a problem hiding this comment.
I think from the perspective of people using the API, it's still conceptually more useful to think of it as a method that says whether to send the notifications.
There was a problem hiding this comment.
Doing this test in updated is subtly different (since calls to notifyClients used to be gated by updateShouldNotify) Hopefully no one will notice.
There was a problem hiding this comment.
Yeah, I wrestled with this quite a bit. I hope this is a subtle enough change to be fine.
There was a problem hiding this comment.
ChangeNotified => ChangeNotifier
There was a problem hiding this comment.
This is a little confusing because Listenable isn't (necessarily) a Notifier and the developer might be wondering what "sends notifications" means.
There was a problem hiding this comment.
I'll update the docs for Listenable to make the terminology clearer.
There was a problem hiding this comment.
Needs an assert(child != null)
There was a problem hiding this comment.
Does this need to document the fact that we're comparing the notifiers with ==?
Hixie
changed the title
Hixie
force-pushed
the
inherited_notifier
branch
from
a427837 to
c864d7f
Compare
|
Updated, PTAL if you're interested (the updates are in a new commit for ease of review). Will land on green unless I hear otherwise. |
Hixie
force-pushed
the
inherited_notifier
branch
from
c864d7f to
49a44f9
Compare
Hixie
force-pushed
the
inherited_notifier
branch
from
49a44f9 to
a5b6d65
Compare
…flutter#23393) * Created a variant of InheritedWidget specifically for Listenables * Add more documentation per review comments
* Revert "[H] Created a variant of InheritedWidget specifically for Listenables (flutter#23393)" This reverts commit 9313285. * Revert "Fix a race condition in vmservice_test.dart (flutter#23529)" This reverts commit 5e7b0a3.
3 participants
cc @HansMuller