Remove remaining uses of childToSlot#64273
Remove remaining uses of childToSlot#64273tvolkert merged 1 commit intoflutter:masterfrom tvolkert:childToSlot
childToSlot#64273Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Adding @goderbauer for context of the change (since he reviewed the earlier changes) Adding @HansMuller and @justinmc for the specific Material changes. |
There was a problem hiding this comment.
This name sounds like a list rather than a map (slotToChild sounds like a map)
There was a problem hiding this comment.
I'll update it back.
HansMuller
left a comment
There was a problem hiding this comment.
LGTM modulo (my humble opinion) renaming the slotToChild properties.
justinmc
left a comment
There was a problem hiding this comment.
LGTM, just a naming question 👍
There was a problem hiding this comment.
Why is this children here but slottedChildren in the cupertino file above?
There was a problem hiding this comment.
In the Cupertino file above, the element contains both an indexed list of children and a map of what it was referring to as "slotted children", so I made the map name match what it was referred to in the comments. However, it sounds like it's more confusing, so I'll rename all occurrences back to slotToChild
There was a problem hiding this comment.
Nit: I know you're just changing the variable name, but couldn't this be:
| assert(children.values.contains(child)); | |
| assert(children.containsValue(child)); |
Here and in a couple other places below.
There was a problem hiding this comment.
Yep - I'll update them all. I'm not used to having a containsValue() function on a map 🙂
The remaining uses of this pattern were all due to wanting to have the child's slot when `Element.forgetChild()` was called. However, when that method is called, the child's `slot` value is still valid in the context of the parent, so the uses can just use `child.slot`. This is the final round of cleanup from the fallout of #63269
The remaining uses of this pattern were all due to wanting to have the child's slot when `Element.forgetChild()` was called. However, when that method is called, the child's `slot` value is still valid in the context of the parent, so the uses can just use `child.slot`. This is the final round of cleanup from the fallout of flutter#63269
The remaining uses of this pattern were all due to wanting to have the child's slot when `Element.forgetChild()` was called. However, when that method is called, the child's `slot` value is still valid in the context of the parent, so the uses can just use `child.slot`. This is the final round of cleanup from the fallout of flutter#63269
Description
The remaining uses of this pattern were all due to wanting to have
the child's slot when
Element.forgetChild()was called. However,when that method is called, the child's
slotvalue is still validin the context of the parent, so the uses can just use
child.slot.Related Issues
This is the final round of cleanup from the fallout of #63269
Tests
Test exemption: this PR just removes code (and renames a few variables) - the existing tests passing validates the change.
Checklist
///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read [Handling breaking changes].