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

Handled relative path in image URLs: #146

Closed
wants to merge 27 commits into from

Conversation

arnold-parge
Copy link

Many web content has relative images paths, which cannot be resolved in flutter_html.
Hence, I have made few changes as below:

  • added prefixNetworkImageRelativePath in ImageProperties.
  • HtmlRichTextParser: prefixed node.attributes['src'] with imageProperties.prefixNetworkImageRelativePath

- added prefixNetworkImageRelativePath in ImageProperties.
- HtmlRichTextParser: prefixed node.attributes['src'] with imageProperties.prefixNetworkImageRelativePath
@The-Redhat
Copy link
Contributor

I think you should implement this feature also for the deprecated HtmlOldParser.

@arnold-parge
Copy link
Author

I think you should implement this feature also for the deprecated HtmlOldParser.

But HtmlOldParser does not have ImageProperties. Also it is deprecated.

@The-Redhat
Copy link
Contributor

Oh sorry you're right. Forgott that

@Sub6Resources Sub6Resources self-requested a review August 26, 2019 17:43
@Sub6Resources Sub6Resources added enhancement New feature or request high-priority labels Aug 26, 2019
@Sub6Resources Sub6Resources self-assigned this Aug 26, 2019
Copy link
Owner

@Sub6Resources Sub6Resources left a comment

Choose a reason for hiding this comment

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

Added my review. Just a couple small things and then we should be good to go!

lib/rich_text_parser.dart Outdated Show resolved Hide resolved
@@ -30,5 +31,6 @@ class ImageProperties {
this.centerSlice,
this.matchTextDirection = false,
this.filterQuality = FilterQuality.low,
this.prefixNetworkImageRelativePath = ''
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add an assert to the constructor that ensures that this is never null?

Copy link
Author

Choose a reason for hiding this comment

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

constructor ImageProperties is const, so it cannot have a body.

lib/image_properties.dart Outdated Show resolved Hide resolved
Sub6Resources and others added 5 commits September 13, 2019 17:19
- added interpolation to prepend image source with prefixImagePath
- added prefixNetworkImageRelativePath in ImageProperties.
- HtmlRichTextParser: prefixed node.attributes['src'] with imageProperties.prefixNetworkImageRelativePath
@Sub6Resources
Copy link
Owner

Do you mind squashing your commits so this is a bit easier to review?

@andy1xx8
Copy link
Contributor

andy1xx8 commented Mar 28, 2020

I dont think this is what flutter html should handle.
As developer have to resolve relative paths on their own first.
Before u give it to flutter html. Let's Flutter Html does his job only.

@tneotia tneotia mentioned this pull request Jan 17, 2021
@tneotia
Copy link
Collaborator

tneotia commented Feb 4, 2021

Hi there, thanks for this PR! It looks like this code applies to the old parser and thus is incompatible with the latest changes on the branch. As a result, we may close this PR.
However, with #505, we have finally added this capability into the library with a custom image rendering API (much better than the current customRender). You can see the discussion at #497 to see how it works. If you have any suggestions, please let @erickok know there. Again thanks for contributing! :)

erickok added a commit to vrtdev/flutter_html that referenced this pull request Feb 8, 2021
@erickok
Copy link
Collaborator

erickok commented Feb 8, 2021

See #532 for an example on how to use relative paths with the new custom image render API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request high-priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants