[Resolved] Custom image loading API

[erickok]

We should think of a good API to customize image loading. As it stands we lack many features that users seem to want. Here is a partial list of those wishes:

• Added a custom function for creating image widgets #183 Custom image loading
• Added image loader to view loader while image is loading #278 Make Image support placeholder #156 Set your own image loader/placeholder
• add headers for NetworkImage #496 Set requests headers
• adding ability to disable using width and height attributes in img tag #235 Ignoring width/height attributes
• Allow to suppress onImageTap for linked images. #301 Suppress taps on certain images
• is local image file supported #478 Added the option to display downloaded images using the prefix 'file:' #304 Load images locally using file: (related to asset support for <img> #162)
• Handled relative path in image URLs: #146 Rewrite the urls before resolving images

Clearly we need to extend the possibilities. Let's evaluate what is currently possible with a customRenderer already and see if we can provide some API by which users can do custom things without having to override the whole default image loading strategy.

[tneotia]

I'm going to begin working on this.

My idea is to create a new parameter customImage: that accepts a Map<String, CustomImage>. This CustomImage class will have parameters for headers (headers: Map<String, String>, suppress taps (supressTaps: bool), prefix image paths (prefixPath: String), ignoring with/height (ignoreWidth: bool and ignoreHeight: bool), and setting image loader (imageLoader: Widget).

The reason for Map<String, CustomImage> is so the user can specify these parameters for only certain images, by either providing an exact url or just a certain domain. The user could also customize an image from www.domain.com/somePath and provide a different customization for www.domain.com (all other images) by putting the more exact paths first - I plan to query this map using a firstWhere(), so it will take the first item in the map where the src of the image contains that key. Edit: If the user wants to select all images they can simply use "." as their key, since every image url is going to have a "." in it. (Again this would be placed as the last item in the map in case they provide more exact urls).

This solution takes care of the 2nd - 5th and 7th items on that list. I think the custom image loading needs a separate parameter which I can add in my commit using the PR from #183 (customImageWidget:), and I can also add the load images locally from #304. Plus, this solution would be able to customize all the same loading parameters for the local images, since users can make the key in the customImage: map start with "file:".

Edit: PR pushed at #503 for testing

What are your thoughts @erickok?

[ryan-berger]

Wow! That was quick @tneotia! I'm going to take a look at #503 today and read through it to figure out what I think about it.

Personally, as Matt was first handing the project over to me and catching me up, I realized really quick I really don't like how we have the customRender API set up, and want to steer clear of creating new similar APIs especially with customImage. Personally, I like passing along the ability to the user to decide whether or not a string is a match, and using some sort of "registration" model where the user "registers" an instance of HTMLImageProvider, or something of the sort, that has a couple of methods that lets flutter_html figure out whether or not it is a match. That way it isn't blob matching, string matching, regex matching, and we can just let people use whatever logic they'd like to decide whether or not they should render the URI.

The only problem I can see with this is conflicting matchers, although this will happen with just about any implementation. Personally, I think if we go the route of class and instance registration, we can figure out a better way to prioritize which provider is used outside of a "first come first serve" with maybe some sort of priority being apart of the HTMLImageProvider so that if there are multiple matches we can decide which one to render based on priority.

Let me know what you think @tneotia @erickok

[tneotia]

I see what you're saying but I don't really understand it 😂

I think what you're trying to say is that you'd rather have the user decide how they set the parameters for an image. Currently I have it setup to match urls, but you want to open that up so that, let's say, the user could match by attributes instead. I like that idea but it's not something I think I could implement, it seems too complicated for me. Maybe someone else can use what I have as a baseline and try and do what you said?

The only concern I have is if it would be complex for the user to use this functionality in their project, since I don't know how a use case would look like for your implementation.

[ryan-berger]

@tneotia Let me write out my own post with a proposal and so you can see it better, personally I think it is much more simple than a blob/regex/string matcher, but that might just be my opinion. You'll see it come through in a top level post really soon

[tneotia]

Sure, will watch out for it. Do you want me to try and implement it (if I think it's something I can do)?

[ryan-berger]

Intro:

I've personally been thinking about this one for a while, and it looks like we have a proposal from @tneotia along with a basic implementation in a PR, but I would like to propose a different solution.

Please excuse my likely bad psuedo-code Dart. I'm a Go developer and typically don't dive too deep in using Dart/Flutter

Proposal:

In order to better accommodate needs of people who would like to create their own image URI resolvers and renderers, I propose that we use a "registration" model for creating URI resolvers, which would involve extension of the Html widget API, along with a new abstract class or interface named something like URIResolver in order to allow users to register their URIResolvers with our library so we can make use of them.

The URIResolver class

The URIResolver abstract class or interface will be extremely simple. There are two parts, and and an optional third, which I'll get to in a moment. I propose it looks something like this:

abstract class URIResolver {
   bool isMatch(string URI);
   Widget render(string URI);
}

We can implement all manners of resolvers, and include a few in the library itself, for example:

class RemoteResolver extends URIResolver {
  @override
  bool isMatch(string URI) {
    return URI.contains('http://') || URI.contains('https://');
  }

  @override
  Widget render(string URI) {
    return NetworkImage(url: URI);
  }
}

Works great for network images. Obviously doesn't cover all cases, but we can get into how that can be done in other parts of the API in just a moment.

Instead of blob matching, we can now allow the user to write their own match function so they can make it as arbitrary as they like, and we could even allow it to be asynchronous if they need to check the file system, or even make a network call to see whether or not the image is something that they can render. This makes the library much more extensible in my opinion

Priority

In order to allow the user's resolvers to take priority over our own resolvers, any registered resolvers will be executed first before our own resolvers are.

API changes to the Html class

I think we clearly will need some sort of field such as resolvers on the Html class that is of type List<ImageResolver> but its behavior and whether or not it is exported is something that should be discussed. Here are a few of my ideas for possible solutions:

Builder API

We could possibly use a couple of Dart's built in language features to have some sort of setter in order to build the list. For example:

Html()
  ..resolvers = [RemoteResolver()]

The register method

We could also have some sort of registerResolver method on the Html widget that allows you to register your resolver:

final html = Html();
html.registerResolver(myResolver);

Constructor parameter

This is the simplest, most self-explanatory, and I believe that it is likely the best, however the constructor parameter shouldn't be a direct property access:

Html(resolvers: [myResolver]);

Conclusion

Personally I find this method much more extensible than other ways of dealing with this sort of URI resolution. In fact, I find it so extensible and elegant that it is something I've considered porting the customRender to as well. Many of our bugs with the customRender API come out of handling cases when you don't want it rendered, or you want to render the default widget in some cases or your custom one in others, and I feel like this solves this without having to handle so many cases. It is very extensible, and is a more complete of a solution.

[gamelaster]

Hi guys,
I'm very looking forward to this feature, although, I needed to solve this problem right now. So I made my custom renderer which is pretty easy:

"asset-img": (RenderContext context, Widget child, attributes, _) {
  return Image(image: AssetImage(attributes['src']));
}

I hope someone who need this feature ASAP will be this useful.

Thank you guys for your great work on this library.

[erickok]

@gamelaster any feedback in the proposed api?

[gamelaster]

@erickok I checked your proposal, and I think it's good and best way. The user of this library should be able to define custom behavior (what is already made thanks to custom renders, which basically "saved my situation", since I wouldn't like to modify library just for one modification). Base64 support is really great, but I think by default, there should be loader from assets available, I think it's second most used way of loading of images.

[tneotia]

@gamelaster if I understand your use case correctly I don't think you need a custom render, the package supports AssetImage natively. Simply prefix your src with asset: and it will load it correctly.

[gamelaster]

@tneotia Oh! I didn't knew about this, it wasn't listed anywhere. Maybe it will be good to add it into the example.
Thanks