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

Fix application of fontFamily to child nodes #131

Closed
wants to merge 2 commits into from

Conversation

amake
Copy link
Contributor

@amake amake commented Jul 18, 2019

Currently, applying a different fontFamily to a nested node doesn't work:

Html(
      data: 'Hello, <em>world</em>',
      customTextStyle: (node, style) {
        if (node is dom.Element) {
          switch (node.localName) {
            case 'em':
              return style.copyWith(fontFamily: 'DM_Serif_Display'); // or whatever
          }
        }
        return style;
      },
    )

One would expect world to be shown in the specified font, but it isn't. (It is correctly displayed in italics, though.)

The cause is, apparently, this: flutter/flutter#35992

The workaround is easy, but yucky. I personally can't wait for the core issue to be fixed and the fix to make its way to the stable channel, so I will just use my fork, but I understand if you'd prefer to wait for this to be fixed upstream rather than merge this.

@Sub6Resources
Copy link
Owner

Thanks for the PR. I'm going to wait a week or two to see if there's any progress towards resolving that Flutter issue you mentioned. If there isn't I'll consider merging this pr.

Workaround for bug where fontFamily styling is not applied to child spans if the
root span is given a style. See:
flutter/flutter#35992
Due to workaround, two widgets are created instead of one
@amake
Copy link
Contributor Author

amake commented Aug 1, 2019

What are your thoughts on merging this? My concerns are:

  1. The real fix will take a long time to get out
  2. I have a feature PR I'd like to open that I currently have implemented on top of this fix, so I am blocked on your decision at the moment. If you'd like to pass on this fix, I will redo it on master.

I'm happy either way, so please let me know.

@amake
Copy link
Contributor Author

amake commented Aug 9, 2019

The core fix appears to have made it to the beta channel now (1.8.3): flutter/flutter@c15cfdd

That's actually good enough for me. Shall I close this PR?

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

Successfully merging this pull request may close these issues.

2 participants