Fixes inputDecoration sibling explicit child not included in semantic… #170079
Conversation
github-actions
bot
added
a: text input
There was a problem hiding this comment.
I took a deep dive into semantics to try to review this PR well, thanks @chunhtai for giving me a crash course. This makes sense that children like prefix/suffix of InputDecorator were dropped before and this includes them, but can you link the PR that caused this regression? Just for my own interest. Otherwise LGTM 👍
| @@ -4739,8 +4739,8 @@ class _SemanticsConfigurationProvider { | |||
| /// | |||
| /// This is typically use to recalculate certain properties when mutating | |||
| /// [effective] since [effective] may contain stale data from previous update. | |||
| /// Examples are [SemanticsConfiguration.isBlockingUserActions] or | |||
| /// [SemanticsConfiguration.elevation]. Otherwise, use [effective] instead. | |||
| /// Examples are [SemanticsConfiguration.isBlockingUserActions] or. Otherwise, | |||
There was a problem hiding this comment.
Looks like this sentence got chopped up weirdly, should it be "An example is ..." and remove the "or"?
| for (final _RenderObjectSemantics explicitSiblingChild in siblingMergeGroups | ||
| .expand<_SemanticsFragment>((List<_SemanticsFragment> group) => group) | ||
| .whereType<_RenderObjectSemantics>() | ||
| .expand( | ||
| (_RenderObjectSemantics siblingChild) => | ||
| siblingChild.shouldFormSemanticsNode | ||
| ? <_RenderObjectSemantics>[siblingChild] | ||
| : siblingChild._children, | ||
| )) { |
There was a problem hiding this comment.
I asked @chunhtai offline about possible performance impact here, seems to be minimal as the existence of explicitSiblingChildren is rare.
|
The previous pr was a big refactor that rewrote the entire semantics compilation #161195 before the refactor the explicit children under the sibling merge group are built and attached to the render object directly, which is still incorrect |
chunhtai
added
the
autosubmit
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Thanks for the diagram and link! |
flutter-dashboard
bot
removed
the
autosubmit
…s tree
fixes #169499
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.