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

Add support for selectable text via SelectableText.rich #682

Merged
merged 9 commits into from
May 31, 2021

Conversation

tneotia
Copy link
Collaborator

@tneotia tneotia commented May 16, 2021

Title.

Fixes #169.

Builds off work done in #679.

There are a few caveats due to Flutter #38474:

  1. No support for customRender, customImageRender, onImageError, onImageTap, onMathError, and navigationDelegateForIframe. (Support for customRender may be added in the future - once the updated customRender changes are merged because then we can support CustomRender(textSpan: ...) or something similar).

  2. You cannot whitelist tags, you must use blacklistedElements to remove any tags that shouldn't be rendered. This is to make sure unsupported tags are not accidentally whitelisted, causing errors in the widget code.

  3. The list of tags that can be rendered is significantly reduced. Key omissions include no support for images/video/audio, table, and ul/ol because they all require widgets and WidgetSpans.

  4. Styling support is significantly reduced. Only text-related styling works (e.g. bold or italic), while container related styling (e.g. borders or padding/margin) do not work because we can't use the ContainerSpan class (it needs an enclosing WidgetSpan).

  5. Due to the above, the margins between elements no longer appear. As a result, the HTML content will not have proper spacing between elements like <h1>. The default margin for <body> is removed as well.

I think this PR will satisfy the majority of simple use-cases, but proper support can't happen until that linked issue is resolved.

lib/flutter_html.dart Outdated Show resolved Hide resolved
@erickok
Copy link
Collaborator

erickok commented May 17, 2021

If we were to add support for 'selectable html' I would most definitely do so as a separate widget, under its own name, and not as a named constructor to Html. Because it is so different and so limited, giving even an impression of the selectable variant to be on par with the default widget is opening up a storm of raised issues, I suspend. It would also be in line with the naming of Text.rich versus SelectableText.rich.

Also, are we sure a widget that can have selectable text but only cover basic text use-cases is enough to satisfy any of our users? Is it enough to warrant this feature right now, kwoing we also have to then maintain and support it?

@tneotia
Copy link
Collaborator Author

tneotia commented May 17, 2021

Also, are we sure a widget that can have selectable text but only cover basic text use-cases is enough to satisfy any of our users? Is it enough to warrant this feature right now, kwoing we also have to then maintain and support it?

My thinking is that it may not be enough but it's at least something. That flutter issue has been open for 2 years and there hasn't been any traction on it, so there's no telling when it will be fixed and thus allow full support with selectable content. Plus, the issue on this repo has the most upvotes and is the oldest feature request.

I do see the flip side though, where it's another thing to maintain that isn't as feature-rich and might bring about a slew of its own issues. I'll leave the decision up to you, if you think this is warranted then I can fix it up a bit to make it merge-ready or we can close this.

@arianneorpilla
Copy link

arianneorpilla commented May 17, 2021

Hi, I coincidentally started working on a branch of my language learning application just a few hours ago to allow it to be used as an EPUB reader using the epub_view package, which uses this library as a dependency in order to render the HTML elements.

When I got it working, I found out the text could not be selected and it really got me down until I looked at the pull requests and found this one and got up to speed. I can select the text now! I am absolutely interested in this feature and will put it to good use, thank you for your work.

@nandakav
Copy link

nandakav commented May 18, 2021

I am waiting for this issue. Please fix it. A separate API is also fine. There is no selectable HTML widget in flutter. There is only one markdown widget but it has so many issues wherein * and _ gets dumped inadvertenly as the conversion from HTML to Markdown can have some bad after effects.
For example:
<strong>N</strong><strong>iranjana Das</strong> gets converted to **N****iranjana Das** which gets rendered as N****iranjana Das. The problem is that is the name of a person and it looks like we are beaping his name. I cant release my product with this and formatting the HTML'S is going to be a huge task as we have more than 10k content in our application.

We have an existing android app which gives this functionality so to disable it will also be uncool in a newer version due to this flutter issue.

@tneotia
Copy link
Collaborator Author

tneotia commented May 18, 2021

Because this PR has some traction I've gone ahead and added the SelectableHtml class like @erickok said and made sure to document the differences in the interface itself as @DFelten said.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
lib/html_parser.dart Outdated Show resolved Hide resolved
lib/html_parser.dart Outdated Show resolved Hide resolved
lib/html_parser.dart Outdated Show resolved Hide resolved
@erickok erickok merged commit 8d2e91b into Sub6Resources:master May 31, 2021
@amanokerim
Copy link

Hurray!!! 🎉🎉🎉

All developer friend, who waited for this for a long time, we can import from master:

flutter_html:
    git:
      url: git://github.com/Sub6Resources/flutter_html.git
      ref: master

and, use this like:

SelectableHtml(
  data: menuButton.content ?? "",
  style: {
    "body": Style(
      margin: EdgeInsets.zero,
      color: Theme.of(context).primaryColor,
    ),
  },
  onLinkTap: (link, renderContext, map, element) async {
    if (link != null && link.isNotEmpty) {
      await launch(link);
    }
  },
)

THANK YOU SUPPORTERS OF THIS LIBRARY< WE LOVE YOU!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use SelectableText - Selectable content
6 participants