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

breaking: Remove SelectableHtml #1153

Closed
wants to merge 4 commits into from
Closed

Conversation

Sub6Resources
Copy link
Owner

@Sub6Resources Sub6Resources commented Oct 1, 2022

This should resolve a few issues regarding the SelectableHtml/Html feature parity. Users can achieve a much cleaner result by using a SelectionArea widget to wrap Html.

Fixes #1137
Closes #717 Closes #1134 Closes #1105

@Sub6Resources
Copy link
Owner Author

@erickok have you had a chance to look at this PR?

@erickok
Copy link
Collaborator

erickok commented Oct 21, 2022

To be honest, since we're still trying to get a 3.0 ready, breaking the API big time, I'd get rid of the limited SelectableHtml entirely. Deprecations are nice in minor releases and are developer friendly, but we have a clean, superior and 'free' alternative.

@Sub6Resources
Copy link
Owner Author

That works for me! I'll get this updated sometime today

@Sub6Resources Sub6Resources changed the title feat: Deprecate SelectableHtml and add full html tag support for SelectionArea breaking: Remove SelectableHtml Oct 22, 2022
@codecov
Copy link

codecov bot commented Oct 22, 2022

Codecov Report

Base: 51.40% // Head: 51.07% // Decreases project coverage by -0.32% ⚠️

Coverage data is based on head (b067c71) compared to base (c75e0df).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1153      +/-   ##
==========================================
- Coverage   51.40%   51.07%   -0.33%     
==========================================
  Files          19       19              
  Lines        2640     2561      -79     
==========================================
- Hits         1357     1308      -49     
+ Misses       1283     1253      -30     
Impacted Files Coverage Δ
lib/custom_render.dart 53.02% <ø> (+2.05%) ⬆️
lib/flutter_html.dart 65.30% <ø> (-4.02%) ⬇️
lib/html_parser.dart 84.98% <ø> (+1.01%) ⬆️
lib/src/css_box_widget.dart 63.17% <100.00%> (-1.00%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Sub6Resources
Copy link
Owner Author

@erickok. It's all removed!

Copy link
Collaborator

@erickok erickok left a comment

Choose a reason for hiding this comment

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

Just a README update is needed.

@@ -148,19 +148,11 @@ If you would like to modify or sanitize the HTML before rendering it, then `Html

#### Selectable Text

The package also has two constructors for selectable text support - `SelectableHtml()` and `SelectableHtml.fromDom()`.
Note: These constructors are deprecated. It is preferred that you use Flutter's new `SelectionArea` instead (just wrap the `Html` widget in a `SelectionArea`).
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be updated now to reflect it's removal rather than deprecation.

@Sub6Resources
Copy link
Owner Author

As I've been (finally) working at the migration from customRender to Extension, I ended up touching a lot of the same code as this pull request. As such, I decided to just include all the changes from this PR into the other branch manually rather than resolving massive merge conflicts after the fact. That said, I'm going to close this PR, with the changes continuing on into #1176. I'll push the changes to that PR in a few hours.

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