Add InlineSpan.visitDirectChildren
#125656
Conversation
flutter-dashboard
bot
added
a: text input
goderbauer
left a comment
goderbauer
left a comment
There was a problem hiding this comment.
It's too bad that the semantics of InlineSpan.visitChildren is different from most other visitChildren, which do not automatically traverse... Oh well.
| /// `visitor` callback returns `false` on an immediate child. This method | ||
| /// itself returns a `bool` indicating whether the visitor callback returned | ||
| /// `true` on all immediate children. | ||
| bool visitDirectChildren(InlineSpanVisitor visitor); |
There was a problem hiding this comment.
Can you link the docs of visitDirectChildren and visitChildren which a one-sentence summary of what the difference is?
| return 1.0; | ||
| } | ||
| } | ||
| return switch ((textAlign, textDirection)) { |
There was a problem hiding this comment.
nice usage of new language features!
| builder.pushStyle(style!.getTextStyle(textScaleFactor: textScaleFactor)); | ||
| } | ||
| if (text != null) { | ||
| if (text case final String text) { |
There was a problem hiding this comment.
IMO, text != null was much clearer here before and easier to read (even if it means to do an extra null check, if you really want to avoid that, I would go with a local variable instead)
There was a problem hiding this comment.
Same below in other places in this PR.
There was a problem hiding this comment.
I wonder what makes the optional binding form harder to read? Was it the lack of syntax highlighting on github?
or would it be cleaner if it's with a ? or when the type annotation gets omitted?
if (text case final text?) {or
if (text case final nonNullText?) {There was a problem hiding this comment.
oh the keywords are highlighted.
| return (text == null || visitor(this)) | ||
| && !(children?.any((InlineSpan descendant) => !descendant.visitChildren(visitor)) ?? false); |
There was a problem hiding this comment.
The old code is way more readable here and it also made the intend clearer.
There was a problem hiding this comment.
(the first two ifs in the old code could probably be merged, though)
| bool visitDirectChildren(InlineSpanVisitor visitor) => !(children?.any((InlineSpan child) => !visitor(child)) ?? false); | ||
|
|
There was a problem hiding this comment.
Same, this doesn't seem to be the most readable way of expressing the logic.
LongCatIsLooong
force-pushed
the
TextSpanVisitChildren-fix
branch
from
5ad4d0b to
a662a8c
Compare
LongCatIsLooong
added
the
autosubmit
2 participants
I'd like to find out the
fontSizeof aPlaceholderSpan, and currently there doesn't seem to be a way to doTextStylecascadinginheritance in the framework:InlineSpan.visitChildrentraverses the entireInlineSpantree using a preorder traversal, and nodes that don't have "content" will be skipped (https://master-api.flutter.dev/flutter/painting/InlineSpan/visitChildren.html):which makes it impossible to do
TextStyleinheritance in the framework:InlineSpans with a non-nullTextStylebut has no content will be skippedvisitChildrendoesn't directly expose the hierarchy, it only gives information about the flattened tree.This doesn't look like a breaking change, most internal customers are extending
WidgetSpanwhich has a concrete implementation of the new method.Alternatively I could create a fake
ui.ParagraphBuilderand record theui.TextStyleat the top of the stack whenaddPlaceholderis called. Butui.TextStyleproperties are not exposed to the framework.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.