-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Remove _debugWillReattachChildren assertions from _TableElement
#34202
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
Remove _debugWillReattachChildren assertions from _TableElement
#34202
Conversation
goderbauer
left a comment
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.
Did you checkin with @Hixie, who wrote the original code, to figure out if he remembers why the assert was added in the first place? There may have been a good reason for them to be there in the first place.
| @override | ||
| Widget build(BuildContext context) { | ||
| return toggle | ||
| ? const SizedBox() |
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.
nit: indentation is off here (this could probably just all be in one line)
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.
trivial nit: no space between > and [
0d7e69e to
916d094
Compare
Description
Calling
insertChildRenderObjectandremoveChildRenderObjectoutside ofupdateormountin_TableElementshould be allowed.Consider the following minimal reproducible example:
Sequence of events after tapping 'Tap me'
onTapcallback is called,_State.contextis marked dirty.GestureDetectorwas replaced by a widget of a different type,_State.context.updateChildmethod calls_State.context.deactivateChild._TableElementto detach it from_TableElement.renderObject, calling_TableElement.removeChildRenderObject._TableElement.removeChildRenderObjectfails.In the above call path only
removeChildRenderObjectwas called on a_TableElementinstance, pretty much everything else is happening in the descendant elements which we have no knowledge of in the implementation ofTable. So it's probably easier to just remove the asserts than figuring out a way to keep them.Related PR
_debugWillReattachChildrenwasn't brought up in #2961's discussion.Related Issues
Fixes #31473
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?