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

Accessibility and scaling #308

Closed
edwardaux opened this issue May 29, 2020 · 33 comments · Fixed by #333
Closed

Accessibility and scaling #308

edwardaux opened this issue May 29, 2020 · 33 comments · Fixed by #333

Comments

@edwardaux
Copy link
Contributor

edwardaux commented May 29, 2020

When applying the system accessibility size settings, the text in a Html widget gets scaled way more than a normal Text widget. Consider the following code:

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

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

class HtmlAccessibilityApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        appBar: AppBar(title: Text('')),
        body: Column(
          children: [
            Text('Normal Text'),
            Html(data: 'Html Text'),
          ],
        ),
      ),
    );
  }
}

When you scale the text using iOS accessibility settings, you see the following:

Simulator Screen Shot - iPhone 11 - 2020-05-30 at 08 48 24

I mentioned this in #303 but I thought it might be worth having it's own ticket (they may end up being related).

@edwardaux
Copy link
Contributor Author

edwardaux commented May 29, 2020

I've done some investigation here, and it looks like it has something to with how Flutter handles rich text. For example, as a hack, if I temporarily change this library's StyledText class to use a Text instead of Text.rich then scaling works OK (ie. modifying the line of code below).:

child: Text.rich(

Similarly, if I slightly modify the above program to add a third row containing:

RichText(text: TextSpan(style: TextStyle(color: Colors.black), text: 'Rich Text')),

the rich text row doesn't scale at all (see below):
Simulator Screen Shot - iPhone 11 - 2020-05-30 at 08 53 50

@pacifio
Copy link

pacifio commented May 31, 2020

Same thing is happening with me
For smaller fonts the view gets smaller
For bigger views , fonts get bigger

@edwardaux
Copy link
Contributor Author

Did a bit more digging on this today and found this helpful comment from one of the key flutter folk: flutter/flutter#14675 (comment)

So, it looks like trying to render a StyledText object using Text.rich() is not going to result in consistently scaled content with the rest of the app. From that comment, it seems to me that creating a Text would fit more closely into the way the flutter text widgets interact.

Clearly, though, there is some tension in this case though, because of the ability in this library to provide "css" overrides. However, it feels like we should still be able to apply the style settings and then allow flutter to apply the platform's scaling on top of that. Still investigating...

@edwardaux
Copy link
Contributor Author

Alright. I know what's going on here... the underlying problem is that the current scale factor gets applied multiple times because the tree recursively creates Text.rich() objects and each time the current scale factor gets applied again.

I've got a hacky fix and will push up a PR shortly once I've tidied it up.

@ryan-berger
Copy link
Collaborator

@edwardaux Thank you very much for your contribution. I've been quite busy recently and haven't been able to take a look at this

@mileusna
Copy link

mileusna commented Jun 5, 2020

I need this ASAP. 😜
Not only that the fontSize is inconsistent due to nesting, it looks like that the width is messed also and content doesn't even fit into screen when system scale factor is >1.0. Even if I manually set style width to screen width, content can't fit the screen.

@mileusna
Copy link

mileusna commented Jun 5, 2020

I've done some investigation here, and it looks like it has something to with how Flutter handles rich text.

In Flutter, Text use null as default for textScaleFactor while RichText use 1.0 as default value for textScaleFactor

https://api.flutter.dev/flutter/widgets/Text-class.html
https://api.flutter.dev/flutter/widgets/RichText-class.html

@edwardaux
Copy link
Contributor Author

I think we are getting caught up in a flutter text defect.

I have an isolated case captured here https://stackoverflow.com/questions/62189595/scaling-nested-richtext-widgets-for-accessibility (I was hoping someone might have some ideas) but I’m about to raise a ticket in the flutter repo.

@mileusna
Copy link

mileusna commented Jun 6, 2020

@edwardaux @ryan-berger
I remember I have tested this a lot in my app and it worked fine. So I start to wander what have changed since then?

I have downgraded Flutter to 1.12.13+hotfix.6 and checkout older version of my app (since I upgraded lots of code and dependencies for Flutter 1.17 since then) and Voila, it works!

First screenshot is Flutter 1.12.13+hotfix.6 with flutter_html v1.0.0-pre.1 and second screenshot is the Flutter 1.17+ with flutter_html v1.0.0.

I have then recompiled my old code keeping flutter_html 1.0.0-pre.1 with flutter 1.17.3, and fonts are messed up again.

I'm not sure does this help our case, but it looks like they have changed something in RichText from 1.12 to 1.17 ☹️

Flutter 1.12.13+hotfix.6
Screenshot_20200606-130443

Flutter 1.17
Screenshot_20200606-123041

@ryan-berger
Copy link
Collaborator

@edwardaux @mileusna Yeah definitely sounds like a Flutter issue. I'm quite busy this week as I am starting a new job and moving, but if I find some time I can take a look at the changelogs and see if I can find anything. @edwardaux If you think that it is for sure an issue that can't be fixed through the update to the RichText API, by all means open a flutter ticket, and if a fix comes out we can try to make a shim so we can have it before the next release.

@edwardaux
Copy link
Contributor Author

Ah, well spotted @mileusna! I ha a very quick look at the diff between those two specific releases, but nothing jumped out at me. I'll try to find some time tomorrow to step through the releases and find the one where it breaks.

@RigertLusha
Copy link

I have the same issue that text wrapped in Html() its not resposive in IOS user text scale.
Any solution ?

@ryan-berger
Copy link
Collaborator

ryan-berger commented Jun 11, 2020

@edwardaux I took a look at the changelog, and there was a change directly related to RichTextif you want to take a look. I'm not sure how it fits in to all of this, but, it looks to be the only change made to RichText since 1.12, so I'm sure that it is the cause of our issues.

Here's that link to the PR in question: flutter/flutter#48346

@edwardaux
Copy link
Contributor Author

@mileusna @ryan-berger I finally had a chance to poke around using older versions of flutter today. I have a pretty isolated test case (totally independent of flutter_html) that I have been using for my investigations. Interestingly, it is also broken in 1.12.13+hotfix.6.

If I compare flutter_html 1.0.0-pre.1 and flutter_html 1.0.0, I can see there's definitely been some changes in there in this general space, so I'm not quite sure why it used to work.

Anyway, given that I can easily reproduce the same symptoms using Flutter primitive widgets, I've raised flutter/flutter#59316 to see what the Flutter-folk say.

@TheStarkster
Copy link

i was becoming fan of this package but unfortunately now i am using webview and rendering local html string

@mileusna
Copy link

i was becoming fan of this package but unfortunately now i am using webview and rendering local html string

WebView can't display large HTML, so I'm still a fan of this widget. Hopefully, this bug with RichText will sort out.

@ghecreagheorghe
Copy link

ghecreagheorghe commented Jun 15, 2020

I created local copy of html_view and add textScaleFactor to Text.rich in html_parser. That fix the issue.

Text.rich(textSpan,
          style: style.generateTextStyle(), textAlign: style.textAlign, textDirection: style.direction, textScaleFactor: 1.0)

@edwardaux
Copy link
Contributor Author

@ryan-berger So I've heard back from the Flutter team about the underlying problem.

It looks like the expected behaviour if you nest RichText is for the scaling to multiply, so we have to stop doing that. As a few folk have suggested, hard-coding the textScaleFactor to 1.0 when creating the Text.rich() widget will solve this.

Right now, though, this is an immediate fix that will fix the scaling. However, it does introduce a wrapping problem... which is a bug in the current Flutter SDK (see flutter/flutter#59711). As far as I can see, there's no real workaround until that fix lands.

Given that the current scaling bug doesn't wrap properly either, it is probably still worth fixing the scale factor in this library so that it is ready when that fix lands.

I'll submit a PR shortly.

@erickok
Copy link
Collaborator

erickok commented Jun 18, 2020

Instead of 1.0 you'd better fix it to MediaQuery's textScaleFactor though. Or would that break again?

@edwardaux
Copy link
Contributor Author

all of the inner scale factors should be set to 1.0, but then a new wrapper around everything needs to be added that contains the MediaQuery.of(context).textScaleFactor

@raLaaaa
Copy link

raLaaaa commented Jun 29, 2020

Any updates on this already?

@ThuAbLKA
Copy link

ThuAbLKA commented Jul 1, 2020

#308 (comment)

this actually works until there is a proper fix

@saptha
Copy link

saptha commented Jul 1, 2020

Hi @ThuAbLKA / @ghecreagheorghe,
I can't understand how to implement your workaround.
Can you kindly share a sample code snippet?
My HTML comes with images as well.

Thanks
Saptha

@ThuAbLKA
Copy link

ThuAbLKA commented Jul 3, 2020

Hi @saptha ,

  • one way of doing it is changing the local file (questionable approach).
    • You can go to the definition of "Html" and change the line mentioned in the comment.
  • The better way is, fork this repository, make the changes in the forked repo and add that as a dependency in your pubsec.yaml

dagovalsusa added a commit to dagovalsusa/flutter_html that referenced this issue Jul 7, 2020
@saptha
Copy link

saptha commented Jul 15, 2020

Thanks @ThuAbLKA for the explanation. I followed it and got it worked.

@Doflatango
Copy link

If you are not care about the font scale from the ROM settings, you can use the workaround below:

just wrap your widget in a MediaQuery which will force reset the textScaleFactor to 1.0

MediaQuery(
    data: MediaQuery.of(context).copyWith(textScaleFactor: 1.0),
    child: child,
);

Absolutely, this is not a resolution for this plugin.

@kkizlaitis
Copy link

I see that the related pull request has relanded successfully in Flutter framework: flutter/flutter#63118

Interesting when it will make its way to stable... Does anyone know how to monitor that?

@ueman
Copy link

ueman commented Sep 23, 2020

I would guess flutter/flutter#63118 lands in Flutter 1.22.0

@joelbrostrom
Copy link

Shouldn't this Issue be left open until the issue is fixed and verified?

I searched open issues before opening my own copy of this issue since I couldn't find any.
Perhaps open this and close the clones so that all paths lead here?

Exited to see Flutter 1.22.0 land!

@ueman
Copy link

ueman commented Oct 3, 2020

This is fixed with Flutter 1.22

@kkizlaitis
Copy link

I can confirm, works for me too with 1.22! 🎉

@timmyrosen
Copy link

I'm having this same issue again on flutter_html 3.0.0-alpha.6

@theSharpestTool
Copy link

I'm having this same issue again on flutter_html 3.0.0-alpha.6

Same issue for me too with Flutter 3.3.10

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 a pull request may close this issue.