Skip to content

Community Feedback Requested - New API Design for customRender #1142

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

Closed
Sub6Resources opened this issue Sep 17, 2022 · 3 comments · Fixed by #1176
Closed

Community Feedback Requested - New API Design for customRender #1142

Sub6Resources opened this issue Sep 17, 2022 · 3 comments · Fixed by #1176
Assignees
Labels
enhancement New feature or request extensions
Milestone

Comments

@Sub6Resources
Copy link
Owner

This is an idea for a simplified version of our public-facing API. I've been thinking a lot about the customRender API. I like the matchers, but the interface can be confusing. These suggestions should make interfacing with our widget easier, so basic users don't have to know the internals of how the project works, while still allowing advanced users to get everything they need to done.

@erickok any thoughts or feedback? Any feedback from any community members?

To start, here's what the Html widget would look like in the example with my proposed changes:

Html(
  anchorKey: staticAnchorKey,
  data: htmlData,
  style: { ... }, // No changes here, omitted for the sake of space
  extensions: [
    TagExtension("tex", builder: (ExtensionContext context) {
        return Math.tex(
          context.innerHtml,
          mathStyle: MathStyle.display,
          textStyle: context.style.generateTextStyle(),
          onErrorFallback: (FlutterMathException e) {
            return Text(e.message);
          },
        );
    }),
    TagExtension.inline("bird", inlineSpan: TextSpan(text: "🐦")),
    TagExtension("flutter", builder: (context) {
      return FlutterLogo(
        style: context.attributes['horizontal'] != null
            ? FlutterLogoStyle.horizontal
            : FlutterLogoStyle.markOnly,
        textColor: context.style.color!,
        size: context.style.fontSize!.value * 5,
      );
    }),
    MyCustomTableExtension(), //Read more about this below
    AudioExtension(),
    IframeExtension(),
    MathExtension(),
    SvgExtension(),
    MatcherExtension(
      matcher: (context) {
        return context.attributes['src'] != null && context.attributes['src']!.startsWith("/wiki");
      },
      builder: (context) {
        // Map from "/wiki" to "https://upload.wikimedia.org/wiki". I haven't thought up a clean solution for this network image src filtering yet, but there is lots of demand for it. Any feedback would be welcome!
      }
    ),
    VideoExtension(),
  ],
),

Changes

  • tagsList is no longer required when adding custom widgets. It'll still be there as a parameter for black/white-listing tags, but Html will automatically search through the given extensions and enable support for the custom tags by default. This requires that the Html.tags member be turned into a getter that has that same behavior.
  • customRenders replaced with extensions. This is the major shift. Currently customRenders takes in a Map<bool Function(RenderContext), CustomRender>, which is a bit confusing. extensions will just take in a List<Extension>. More details on the Extension class below.
  • RenderContext in builder replaced by ExtensionContext. ExtensionContext will store and provide direct access to some of the most commonly used attributes, as well as all the extra stuff RenderContext holds.

New Extension Class

This new class encapsulates the "matcher"/"renderer" logic into a container. Keeping it in a class also means that the object can persist and receive data across the various phases of the Html widget's lexing/styling/processing/parsing/rendering/disposing lifecycle. This also makes the Extensions tightly coupled with the tree, rather than a hacky afterthought.

Here's what the basic signature of the Extension class might look like. By default, the only thing that needs to be overwritten by child classes is the matches method. Everything else will work out of the box, with default behavior:

abstract class Extension {

  // Tells the HtmlParser what additional tags to add to the default supported tag list (the user can still override this by setting an explicit tagList on the Html widget)
  List<String> get supportedTags;

  // Whether or not this extension needs to do any work in this context
  bool matches(ExtensionContext context);

  // Converts parsed HTML to a StyledElement. Need to define default behavior, or perhaps defer this step back to the Html widget by default
  StyledElement lex(dom.Node element);

    // Called before styles are applied to the tree. Default behavior: return tree;
  StyledElement beforeStyle(ExtensionContext context, StyledElement tree);

    // Called after styling, but before extra elements/whitespace has been removed, margins collapsed, list characters processed, or relative values calculated. Default behavior: return tree;
  StyledElement beforeProcessing(ExtensionContext context, StyledElement tree);

    //The final step in the chain. Converts the StyledElement tree, with its attached `Style` elements, into an `InlineSpan` tree that includes Widget/TextSpans that can be rendered in a RichText or Text.rich widget. Need to define default behavior, or perhaps defer this step back to the Html widget by default
  InlineSpan parse(ExtensionContext context, StyledElement tree);

    //Called when the Html widget is being destroyed. This would be a very good place to dispose() any controllers or free any resources that the extension uses. Default behavior: do nothing. 
  void onDispose();
}

And then there's the ExtensionContext class, which has the following members available:

class ExtensionContext {

  // Guaranteed to always be present
  dom.Node node;

  // Shortcut getters for `node`
  String elementName; // Returns an empty string if the node is a text content node, comment, or any other non-Element node.
  String innerHtml; // Returns an empty string if there is no inner HTML
  List<dom.Node> nodeChildren; //Returns an empty list if there are no children
  LinkedHashMap<String, String> attributes; // The attributes of the node. Empty Map if none exist.
  String? id; //Null if not present
  List<String> classes; //Empty if element has no classes

  // Guaranteed to be non-null after the lexing step
  StyledElement? styledElement;

  // Guaranteed only when in the `parse` method of an Extension, but it might not necessarily be the nearest BuildContext. Probably should use a `Builder` Widget if you absolutely need the most relevant BuildContext.
  BuildContext? context;

  // Guaranteed to always be present. Useful for calling callbacks on the `Html` widget like `onImageTap`.
  HtmlParser parser;
}

These classes haven't actually been written or implemented, so things may be subject to change as specific implementation requirements open or close different doors.

Benefit: Clearer Path Towards First and Third-Party Extensions

This new approach would make the modular design a little more intuitive.

Each of our first-party packages would now just need to override the Extension class, and including them is a bit more intuitive than, say, svgAssetUriMatcher(): svgAssetImageRender(), etc. They would still be able to support custom options, error callbacks, etc, in their constructors.

AudioExtension( //Or do we prefer AudioTagExtension() or AudioHtmlExtension()?
  //Options for this extension. Pass in a controller, error handling, etc.
),
IframeExtension(),
MathExtension(),
SvgExtension(), //This would include matchers for svg tag and data/network/asset uri's. We might consider providing some options to turn certain features on or off
TableExtension(),

In addition, this opens the door more widely for third-party packages to extend flutter_html's functionality in a way that is easy to use and doesn't affect existing users.

For example, a third-party could write and publish support for:

YoutubeExtension(),
MarkdownExtension(),
TexExtension(),
ImageDisplaysFullscreenOnTapExtension(),
LazyLoadingHtmlExtension(), //Which we would look at and seriously consider adding to the main project, with the permission of the extension owner, of course
JavascriptExtension(), //Someone is feeling extremely ambitious and really doesn't want to just use webview for some reason :)

Included Extension Helper Classes

Since the average use case isn't worth creating a whole class to override Extension for, flutter_html will provide a couple helper class constructors for basic use cases, as used in the example above. Here's what their signatures might look like:

TagExtension(String tagName, {Widget? child, Widget Function(ExtensionContext)? builder}); //Takes either a widget or a builder

TagExtension.inline(String tagName, {InlineSpan? child, InlineSpan Function(ExtensionContext)? builder)); //Takes either an InlineSpan or a builder

MatcherExtension({required bool Function(ExtensionContext) matcher, required Widget Function(ExtensionContext) builder}), // Similar to the current "matcher", "renderer" API.

MatcherExtension.inline({required bool Function(ExtensionContext) matcher, required InlineSpan Function(ExtensionContext) builder}),

Hopefully it's fairly obvious how these would be implemented!

Example Extension:

Here's a somewhat silly example of what an extension subclass might look like and how it would be used by the user (I'm coding this in the GitHub text editor, so forgive any typos or errors 😅):

class NumberOfElementsExtension extends Extension {

  final String tagToCount;
  bool _counting = false;
  int _counter = 0;

  NumberOfElementsExtension({
    this.tagToCount = "*",
  });

  @override
  List<String> supportedTags => ["start-count", "end-count", "display-count"];

  @override
  bool matches(ExtensionContext context) {
    if(_counting) {
      if(context.elementName == "end-count" && context.styledElement == null) {
        _counting = false;
      }
      if(context.elementName == tagToCount || tagToCount == "*") {
        _counter++;
      }
    } else {
      if(context.elementName == "start-count" && context.styledElement == null) {
        _counting = true;
      }
    }

    // The only element this extension actually renders is "display-count"
    return context.elementName == "display-count";
  }

  @override
  InlineSpan parse(ExtensionContext context, StyledElement tree) {
     //There's a lot we could do here (process children, update styles, etc.), but we'll just show the simplest case. If we want a block-level element where styling is handled for us, it is recommended to wrap our widget in CssBoxWidget.

     return TextSpan(
       text: "There are $_counter elements!",
       style: tree.style.generateTextStyle(),
     );
   }
}

And, here's the usage in an app:

    Html(
      htmlData: """
      <h1>One...</h1>
      <h2>Two...</h2>
      <h3>Three...</h3>
      <start-count></start-count>
      <div>
        Hello, world!
        <!-- It would count comments, too! -->
      </div>
      <div>
        Goodbye!
      </div>
      <end-count></end-count>
      <div>
        <display-count></display-count>
     </div>
      """,
      extensions: [
        NumberOfElementsExtension(),
      ],
    ),

Which would output something along the lines of

One...

Two...

Three...

Hello, world!
Goodbye!
There are 5 elements!
@Sub6Resources Sub6Resources added enhancement New feature or request extensions labels Sep 17, 2022
@Sub6Resources Sub6Resources added this to the 3.0.0 milestone Sep 17, 2022
@Sub6Resources Sub6Resources self-assigned this Sep 17, 2022
@Sub6Resources Sub6Resources pinned this issue Sep 17, 2022
@erickok
Copy link
Contributor

erickok commented Sep 19, 2022

I think this is a fantastic improvement over the current state. It extends what is possible while making the API easier to understand. We could apply the same idea to our image renders as well, though it remains to be seen if we can merge the two entirely into one 'extension' feature.

@Sub6Resources
Copy link
Owner Author

Perhaps some sort of ImageExtension helper class (and perhaps a few helper subclasses) might make that possible. I'll look the current implementation over and see what could be done

@TDuffinNTU
Copy link

Maybe I'm a bit pedantic but the naming of "Extension" might be confusing to some, especially since extension is already a reserved keyword in dart, and might clash with other classes since it's such a common term. Intellisense may also add to confusion if you don't explicitly check where the import is coming from.

HtmlExtensions or something that better differentiates it might be an improvement imo :)

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

Successfully merging a pull request may close this issue.

3 participants