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

SelectableText widget supports WidgetSpan #38474

Closed
chunhtai opened this issue Aug 13, 2019 · 32 comments · Fixed by #92295
Closed

SelectableText widget supports WidgetSpan #38474

chunhtai opened this issue Aug 13, 2019 · 32 comments · Fixed by #92295
Labels
a: quality A truly polished experience c: new feature Nothing broken; request for a new capability customer: crowd Affects or could affect many people, though not necessarily a specific customer. f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Comments

@chunhtai
Copy link
Contributor

chunhtai commented Aug 13, 2019

Currently SelectableText.rich can only support textspan. We would like to be able to support different type of inlinespan eventually as Text.rich did.

This might require some discussions on how should we deal with copying or pasting a widgetspan.

@chunhtai chunhtai added c: new feature Nothing broken; request for a new capability framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. a: quality A truly polished experience labels Aug 13, 2019
@zmtzawqlp
Copy link
Contributor

zmtzawqlp commented Nov 10, 2019

_textPainter.getOffsetForCaret return Offset.zero for WidgetSpan. _textPainter.getBoxesForSelection is right for WidgetSpan, so I do a workaround for get caret position with selection boxs.

but at version is high than 1.7.8 , _textPainter.getBoxesForSelection is also breaking. selection function about WidgetSpan are all breaking for https://github.com/fluttercandies/extended_text and https://github.com/fluttercandies/extended_text_field.

i look forward that it will be fixed asap.

@AlexV525
Copy link
Member

AlexV525 commented Dec 9, 2019

Is there any updates?

@zmtzawqlp
Copy link
Contributor

getPositionForOffset always return 0, after 1.7.8

@AxesandGrinds
Copy link

It's 2020 April, any update on this?

@chunhtai
Copy link
Contributor Author

chunhtai commented Apr 22, 2020

I would like to gather some feedbacks on what does it mean to copy and paste a widget span.

If possible, please share your use cases that require widget span selection.

@AlexV525
Copy link
Member

If possible, please share your use cases that require widget span selection.

We've discussed this case in #35869 (comment) . Please check it out.

@chunhtai
Copy link
Contributor Author

@AlexVincent525 Thank you for sharing that. That is a good example, and I will keep that in mind. The reason i am still looking for examples is that I can implement a more generic fix that can fit all use cases.

@goderbauer
Copy link
Member

@GaryQian When you designed the WidgetSpan did this use case of what does it mean to copy text with a widget span ever come up? Any thoughts on this issue?

@GaryQian
Copy link
Contributor

I believe we had discussed this, pur initial thoughts was to just return the toString of the widget span, which would then require the user to parse the data back into a WidgetSpan object. Since we don't really know what to expect from the nested widget, this would allow people to serialize their own widgets via toString.

Was not aware of the getPositionForOffset bug, will look into it.

@chunhtai
Copy link
Contributor Author

chunhtai commented Apr 29, 2020

I took a look at different platforms how they deal with copying none text item.

Our current API only support text format, so i think it makes sense to use string for now. I was imagining surfacing api to WidgetSpan so that developer can set the string they want when creating widgetspan instead of relying on widget.toString.

@snoe
Copy link

snoe commented Jun 17, 2020

@chunhtai Does there exist a workaround for this issue? Or is there an update?

My use case is a minimal markdown viewer with special widgets for fixed-width, block quote paragraphs, and lists

SelectableText.rich(TextSpan(
 [WidgetSpan(FittedBox(Text("fixed-width"),...)), 
  WidgetSpan(DecoratedBox(Text("blockquote"),...)), 
  TextSpan("PlainText")])

@zmtzawqlp
Copy link
Contributor

@chunhtai Does there exist a workaround for this issue? Or is there an update?

My use case is a minimal markdown viewer with special widgets for fixed-width, block quote paragraphs, and lists

SelectableText.rich(TextSpan(
 [WidgetSpan(FittedBox(Text("fixed-width"),...)), 
  WidgetSpan(DecoratedBox(Text("blockquote"),...)), 
  TextSpan("PlainText")])

you can use https://github.com/fluttercandies/extended_text for now, it works on 1.17.0

@asjqkkkk
Copy link

Is there any plan to support this feature?

@sabetAI
Copy link

sabetAI commented May 25, 2021

Updates?

@GaryQian
Copy link
Contributor

I am currently working on bringing WidgetSpan support to more areas of the framework. Currently, Editables/TextFields have the priority, but this can come after.

@GaryQian
Copy link
Contributor

GaryQian commented Jun 16, 2021

SelectableText is actually built on top of EditableText internally, which means that the core WidgetSpan support added for #30688 which landed in #83537 may have also allowed SelectableText to support WidgetSpans. I will check if this is indeed true.

If anyone has a particular use case, please try passing WidgetSpans to the SelectableText.rich() constructor and see if it behaves as you expect.

@GaryQian
Copy link
Contributor

I will have to remove the assert that prevents non-TextSpan spans from being passed, but otherwise, widgets are rendering as expected. Will still have to verify selection behavior.

@daohoangson
Copy link
Contributor

I will have to remove the assert that prevents non-TextSpan spans from being passed, but otherwise, widgets are rendering as expected. Will still have to verify selection behavior.

This sounds encouraging. Does this mean Flutter master channel as of its current state doesn't allow passing WidgetSpans but if one goes into Flutter source code and remove the assert then it should work?

@GaryQian
Copy link
Contributor

@daohoangson Yes, if you remove the assert, widgetspans will start rendering. My more formal PR will include tests and whatnot as well as additional validation, but if you want to try it out, removing the assert at SelectableText.dart:564 should work as a temporary way to try it.

@daohoangson
Copy link
Contributor

It works! I ran into some problem with complicated WidgetSpan tree though, will keep an eye on this and file separate issues later. Thank you for your great work @GaryQian

@GaryQian
Copy link
Contributor

@GaryQian
Copy link
Contributor

flutter/engine#27010 Adds support for placeholders to represent n-length codepoints in libtxt to enable intuitive selection and word bounds.

@lrorpilla
Copy link

lrorpilla commented Jul 10, 2021

If anyone has a particular use case, please try passing WidgetSpans to the SelectableText.rich() constructor and see if it behaves as you expect.

@GaryQian, I just want to thank you for your work. I managed to update my app to have the functionality I want, I knew about this problem and that it was going to be a roadblock to achieve this sort of layout (I need it for recursive lookups as you can see), this is already excellent for my use case and I'm just thankful you got to fix this just about a week after I started working on this. Just used your fork, removed the assert and worked like a dream.

My only nitpicks are that the widget spans seemingly selectable showing up as OBJ symbols in the copied text, also had trouble trying to use SelectableText.rich in an AlertDialog inside a SingleChildScrollView without triggering another assert where Text.rich worked just fine but those aren't really any significant misbehaviors for my use case for me to hold this back and I'm happy to release this to my users.

Thanks again and looking forward to seeing this to stable.

@Nickkorol
Copy link

@daohoangson Yes, if you remove the assert, widgetspans will start rendering. My more formal PR will include tests and whatnot as well as additional validation, but if you want to try it out, removing the assert at SelectableText.dart:564 should work as a temporary way to try it.

@GaryQian I'm trying to implement the same functionality, but I'm getting another error after removing the assert you mentioned:

The following assertion was thrown during performLayout():
'package:flutter/src/widgets/widget_span.dart': Failed assertion: line 105 pos 12: 'dimensions != null': is not true.

Maybe anybody faced the same problem and fixed something else in the source code to fix this?

The simpliest code resulting to this error is below

SelectableText.rich(TextSpan(children: [
          TextSpan(text: "1231231231"),
          WidgetSpan(
              child: Container(
            width: 10,
            height: 10,
            color: Colors.red,
          )),
          TextSpan(text: "dfgdfgdsfgfd"),
        ]))

I'm using flutter version 2.2.3

@DipakSkywave
Copy link

@daohoangson Yes, if you remove the assert, widgetspans will start rendering. My more formal PR will include tests and whatnot as well as additional validation, but if you want to try it out, removing the assert at SelectableText.dart:564 should work as a temporary way to try it.

@GaryQian I'm trying to implement the same functionality, but I'm getting another error after removing the assert you mentioned:

The following assertion was thrown during performLayout():
'package:flutter/src/widgets/widget_span.dart': Failed assertion: line 105 pos 12: 'dimensions != null': is not true.

Maybe anybody faced the same problem and fixed something else in the source code to fix this?

The simpliest code resulting to this error is below

SelectableText.rich(TextSpan(children: [
          TextSpan(text: "1231231231"),
          WidgetSpan(
              child: Container(
            width: 10,
            height: 10,
            color: Colors.red,
          )),
          TextSpan(text: "dfgdfgdsfgfd"),
        ]))

I'm using flutter version 2.2.3

@Nickkorol can you solve this problem?

If, yes then how to solve this problem. Please send this code

Thanks

@Alcly
Copy link

Alcly commented Aug 24, 2021

@daohoangson Yes, if you remove the assert, widgetspans will start rendering. My more formal PR will include tests and whatnot as well as additional validation, but if you want to try it out, removing the assert at SelectableText.dart:564 should work as a temporary way to try it.

@GaryQian I'm trying to implement the same functionality, but I'm getting another error after removing the assert you mentioned:

The following assertion was thrown during performLayout():
'package:flutter/src/widgets/widget_span.dart': Failed assertion: line 105 pos 12: 'dimensions != null': is not true.

Maybe anybody faced the same problem and fixed something else in the source code to fix this?
The simpliest code resulting to this error is below

SelectableText.rich(TextSpan(children: [
          TextSpan(text: "1231231231"),
          WidgetSpan(
              child: Container(
            width: 10,
            height: 10,
            color: Colors.red,
          )),
          TextSpan(text: "dfgdfgdsfgfd"),
        ]))

I'm using flutter version 2.2.3

@Nickkorol can you solve this problem?

If, yes then how to solve this problem. Please send this code

Thanks

It works when I check out to the Master branch...

@Nickkorol
Copy link

After flutter v2.5.0 release, it works on stable branch too (removing the assert at selectable_text.dart:582 is still required).

@daohoangson
Copy link
Contributor

Things seem to work reasonable well. Are you planning to remove the assert anytime soon?

@justinmc
Copy link
Contributor

justinmc commented Oct 7, 2021

Any reason not to remove that assert @chunhtai @GaryQian?

@DipakSkywave

This comment has been minimized.

@GaryQian
Copy link
Contributor

Let's remove the assert. Since we technically can render it with no problem, let's no longer gate it from developers.

The behavior around clipboards and selection is not yet fully rich, but that is a minor problem that we can fill out in time. Behavior currently assumes a single codepoint object replacement character when converting to UTF-8 formats such as in clipboard or selections. I would not fully depend on this particular behavior yet though.

Lmk if anyone has any opposing views of removing the assert and gradually adding richer support for it later.

@github-actions
Copy link

github-actions bot commented Nov 9, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: quality A truly polished experience c: new feature Nothing broken; request for a new capability customer: crowd Affects or could affect many people, though not necessarily a specific customer. f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging a pull request may close this issue.