Skip to content
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

Text inline widgets, TextSpan rework #30069

Closed
wants to merge 101 commits into from
Closed

Text inline widgets, TextSpan rework #30069

wants to merge 101 commits into from

Conversation

@GaryQian
Copy link
Contributor

@GaryQian GaryQian commented Mar 27, 2019

This PR implements the capability of embedding widgets inline into paragraphs.

New TextSpan inheritance structure is the following:

InlineSpan --> TextSpan
         |
         ----> PlaceholderSpan ---> WidgetSpan

Wrap widgets to embed in WidgetSpan(widget: <yourwidgethere>)

The TextSpan changes make this a breaking change, although existing code passing <TextSpan>[] into RichText or TextPainter should still work as expected. Only code directly modifying a InlineSpan tree will be affected.

Fix/Addition for #2022

Dependent on the engine/LibTxt side PR: flutter/engine#8207

Work continued in #33946

@jonahwilliams
Copy link
Member

@jonahwilliams jonahwilliams commented Mar 29, 2019

@GaryQian lets make that widgets placed as inline text widgets can also contribute semantics

@GaryQian
Copy link
Contributor Author

@GaryQian GaryQian commented Mar 30, 2019

Have checked that semantics are being correctly built up and passed through, although I will verify with some people who are more familiar with semantics if it is indeed the full behavior

packages/flutter/lib/src/painting/inline_span.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/painting/inline_span.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/painting/inline_span.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/painting/inline_span.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/painting/inline_span.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/rendering/paragraph.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/rendering/paragraph.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/rendering/paragraph.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/rendering/paragraph.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/widgets/widget_span.dart Outdated Show resolved Hide resolved
Copy link
Member

@goderbauer goderbauer left a comment

LGTM

@@ -415,11 +415,8 @@ class RenderParagraph extends RenderBox
assert(() {
final Offset manualPosition = (position - textParentData.offset) / textParentData.scale;
// Compare the two offsets ignoring floating point error.
Copy link
Member

@goderbauer goderbauer May 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove this comment? It's rather obvious what's happening below, I think.

@GaryQian
Copy link
Contributor Author

@GaryQian GaryQian commented Jun 3, 2019

Due to versioning calculation, #33794 squashes this PR on top of master. Further changes can be seen there.

@GaryQian
Copy link
Contributor Author

@GaryQian GaryQian commented Jun 4, 2019

Closing this, will be landing the other version.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants