-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Reland "Text inline widgets, TextSpan rework (#30069)" with improved backwards compatibility #34051
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
Conversation
|
I still haven't completed the roll, please hold off on this until the roll succeeds. |
|
Yep, won't be landing this until your roll is all done. |
| /// | ||
| /// This getter does not include the contents of its children. | ||
| @override | ||
| String get text => _text; |
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.
This change and below should not be necessary. final fields are equivalent to a getter
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.
Ahh thanks, good to know!
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.
Will it override the getter without a @override? Update: it apparently will.
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.
@override is an analyzer hint, but it is required for our lints
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.
... I think!
jonahwilliams
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.
LGTM
This is another attempt at landing Text inline widgets (original PR with comments can be found here: https://github.com/flutter/flutter/pull/
This version adds the following backwards compatibility deprecated API:
These deprecated APIs should allow InlineSpan to behave like the old TextSpans for instances of InlineSpan that are also TextSpans.
getSpanForPositionnow returns an InlineSpan, which may still break if you try to store the returned InlineSpan into a TextSpan.These three deprecated APIs should be removed after the next stable release.
Diff from previous version: https://github.com/flutter/flutter/pull/34051/files/db2ed0acdbea7243917603e348b8b01585a7e590..3b08022762ea94b6fdf9570f00931bdafefac550
Previous version had LGTM.