Honouring the app's theme

[edwardaux]

Firstly, just wanted to say this is a cracking library! This is by far the best library in this space. Thank you!

One thing that I've noticed is that the default styling doesn't seem to follow the app's theme (although #18 seems to suggest that it does). Also, something that seems related is how it scales the text based on the user's device accessibility settings. Consider this super-simple app:

import 'package:flutter/material.dart';
import 'package:flutter_html/flutter_html.dart';

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        appBar: AppBar(title: Text('')),
        body: SingleChildScrollView(
          child: Column(
            children: [
              Text('Heading2', style: Theme.of(context).textTheme.headline2),
              Text('Body', style: Theme.of(context).textTheme.bodyText2),
              Html(data: '<h2>Heading2</h2>Body'),
              Html(data: '<h2>Heading2</h2><p>Body</p>'),
            ],
          ),
        ),
      ),
    );
  }
}

This results in the following:

Standard Size	Small Text	Largish

• For the standard scenario, headings are different sizes, but at least the text seems to be the same as the app's theme
• With small text enabled, headings are heaps smaller. Text inside and outside a <p> tag are scaled at different sizes (not sure why)
• Once you get to the really large text, things get way out of proportion. This screenshot is at the 3rd-largest text. At this point, headings barely even fit on a screen, text outside a <p> tag take's about 25% of the screen height, and text inside a<p> tag is about 80% of the screen height

If I explicitly assign a style like this:

Html(
  data: '<h2>Heading2</h2><p>Body</p>',
  style: {
    "h2": Style.fromTextStyle(Theme.of(context).textTheme.headline2),
  },
)

it looks a bit better initially (in that the styles seem to match), however a) I would have thought this would be the default behaviour, and b) the scaling still doesn't match the standard text components.

I'm happy to look at creating a PR to resolve this if you can point me in the right direction.

Thanks again for a great library 🎉

[ryan-berger]

@edwardaux My buddy @Sub6Resources left me with this library before taking off on a 2 year humanitarian trip, so if it was expected to be released in 1.0.0 and you are finding cases it definitely isn't, then it must definitely not be, and must have been a mistake to close the issue.

Is the theming problem solely based around text, or are there other problems? Will this be addressed in your other PR for accessibility and scaling?

[edwardaux]

Sadly, now that I know a bit more about things, I don't think they're related (well, any mention I make of "scaling" is related to #308, but the default theming is a separate problem. I'll have a poke around and see what I can see.

[ryan-berger]

@edwardaux Is this still an issue? Or is it part of #308?

[erickok]

Actual bugs are probably resolved by #308. As for following the app theme, I am not sure we should. HTML has well-defined standards that we rely upon (like a browser) would, instead of adopting the app theme. I think with the currect styling support we should have enough tools for a developer to easily set up a theme for Html that matches her/his's app. Let us know if you disagree.

[edwardaux]

FWIW, I do disagree - not that that means you're obliged to make any changes of course 😄

My thinking here is purely about the cognitive load of getting up and running with this library. The way I see things are that this library can be used in two main ways:

• A means to render HTML content inline with the rest of the widgets on the screen. In this case, I feel the expectation of consumers of this library would be that the content would be styled in the same way as the rest of the app.
• A full-blown web browser. In this case, I can see that it is quite possible to desire totally different theming... although, having said that, I can just as easily see that having the same styling as the rest of the app would also be desirable in order to maintain a consistent L&F across the app.

Perhaps you could consider creating a style that represents the equivalent of the app's current theme and make that as a readily discoverable static variable in the Style class? By creating this variable, you're removing the need for your consumers to have to figure out what all the various bits they'd need to set in a style.

Personally, I'd advocate for this to be the "default" style, but I imagine there may be others that disagree.

[erickok]

I do think in the first way, where this library is rendering just bits of content (and not be a web browser). It's tricky... because your argument makes sense, but I also want to prevent that for people that use standard-compatible styling to get drastically different results than people that don't. But we shoudl discuss on this, because you make lots of sense.

[tneotia]

@edwardaux what would be some things that you'd like to see "inherited" from the app theme? Just trying to get a better idea of your use case - I assume things like font family, colors, etc? Currently the app uses Theme.of(context).textTheme.bodyText2 as the default.

I wouldn't be opposed to including a "defaultTheme" parameter on the Html widget to override this, but I don't know if @erickok or @ryan-berger would approve of this.

[edwardaux]

Yeah, that's pretty much the idea I had. It has been a while since I've dabbled in this space, but if I look at the code that I'm using at the moment, this is how I construct my Html widget:

Html(
  data: html,
  shrinkWrap: true,
  style: {
    'h1': Style.fromTextStyle(theme.textTheme.headline1),
    'h2': Style.fromTextStyle(theme.textTheme.headline2),
    'h3': Style.fromTextStyle(theme.textTheme.headline3),
    'h4': Style.fromTextStyle(theme.textTheme.headline4),
    'h5': Style.fromTextStyle(theme.textTheme.headline5),
    'h6': Style.fromTextStyle(theme.textTheme.headline6),
    'body': Style.fromTextStyle(theme.textTheme.bodyText2).copyWith(margin: EdgeInsets.zero),
  }
)

In my case, my HTML was super simple so this is all I needed, but I imagine there's a few other properties from the theme that could be populated initially.

Don't get me wrong, it isn't that hard for devs to use something like the above, but as a newcomer to this library, it was something I was surprised wasn't the default behaviour.

[tneotia]

I see. I guess the only thing is that this would likely be quite a bit of hardcoding, right? Since you have to essentially manually set headline1 vs headline6 for h1, for example. I definitely see the use case though, since this allows you to be more flexible with your theming.

Again, I don't know how feasible this would be now that I look at it again since (I think) it requires some hardcoding, and if theming changes between Flutter versions, it could potentially break the widget or deprecate parts of the code. If there's a better way to do this then let me know! I am definitely not an expert in Flutter/Dart so I could be missing something.

[tneotia]

In my case, my HTML was super simple so this is all I needed, but I imagine there's a few other properties from the theme that could be populated initially.

I looked through the various properties in ThemeData and there wasn't anything else that I felt could be populated initially besides what you have there. If you have any ideas feel free to drop them here! PR created above :)