Make EditableText respect MediaQuery.boldTextOf#122754
Make EditableText respect MediaQuery.boldTextOf#122754dnfield merged 4 commits intoflutter:masterfrom
Conversation
flutter-dashboard
bot
added
a: text input
| /// By default makes text in composing range appear as underlined. | ||
| /// Descendants can override this method to customize appearance of text. | ||
| TextSpan buildTextSpan() { | ||
| final TextStyle style = MediaQuery.boldTextOf(context) |
There was a problem hiding this comment.
There are other codepaths in this file that talk to the platform channel about text style. I'm not clear on whether those need to be updated as well. @LongCatIsLooong or @mdebbar may know
There was a problem hiding this comment.
It looks like SelectableText is already doing this:
flutter/packages/flutter/lib/src/material/selectable_text.dart
Lines 680 to 682 in 7ce6dda
So maybe TextField/CupertinoTextField are better places for this? Usually styles are resolved there iirc.
There was a problem hiding this comment.
Yes, SelectableText does this, but EditableText does not.
If we do it higher up in the stack, couldn't some custom text editing class end up missing out? This seems like the right peer of SelectableText to me.
There was a problem hiding this comment.
Ohh, I see what you're saying now.
Why not do this on EditableText and remove it from SelectableText?
There was a problem hiding this comment.
(I removed the call from SelectableText - I had seen it before but missed the part where SelectableText just returns an EditableText anyway)
There was a problem hiding this comment.
Ah ok. I was worried about not being able to opt-out but people can just add another MediaQuery. But I think TextInputConnection.setTextStyle needs to be updated as well. Also is this supposed to affect things like suffix text or placeholder text in InputDecorator?
There was a problem hiding this comment.
Ah ok those are Text widgets. But I think TextInputConnection.setTextStyle still needs to be updated with the right font weight if boldText is on.
There was a problem hiding this comment.
SG, I will do that and update the tests.
| /// | ||
| /// This makes sure that accessibility settings related to bold text are | ||
| /// respected. | ||
| TextStyle updateStyleForBold() { |
There was a problem hiding this comment.
should the update function be called in didChangeDependencies and cache the value?
There was a problem hiding this comment.
also this should probably be private
There was a problem hiding this comment.
Deleted this method and just inlined it into didChangeDependencies and didUpdateWidget.
mdebbar
left a comment
mdebbar
left a comment
There was a problem hiding this comment.
As long as we continue to send style updates to the engine, this looks good to me from web point of view!
dnfield
added
the
autosubmit
4 participants
Fixes #122750