Repository navigation
Add a isSystemTextScaler matcher - #160120
Conversation
github-actions
Bot
added
a: tests
| } | ||
|
|
||
| @override | ||
| double scaleFontSize(double unscaledFontSize) => textScaleFactor * unscaledFontSize; |
There was a problem hiding this comment.
Can you add a test for this?
| /// | ||
| /// If `withScaleFactor` is specified and non-null, this matcher also asserts | ||
| /// that the [TextScaler]'s' `textScaleFactor` equals `withScaleFactor`. | ||
| Matcher isSystemTextScaler({ double? withScaleFactor }) { |
There was a problem hiding this comment.
What is a non-system text scaler? Can you add something to the docs that describes the difference between a system text scaler and a non-system one?
And for my own understanding: Why should I care in my tests whether the text scaler is system or not?
There was a problem hiding this comment.
A "system" one is one that directly reflects the user's preference while the developer could introduce a MediaQuery with a "non-system" scaler (with a fixed 1.0 scaling factor, for example). This is useful (I guess) to make sure the text scaler from a certain BuildContext is using the user's preference instead of using some fixed scales.
There was a problem hiding this comment.
Wanna add that context to the doc? I think it would be useful to specify what cases would fail this check (i.e. when developers introduce their own text scaler in a widget)
| /// | ||
| /// If `withScaleFactor` is specified and non-null, this matcher also asserts | ||
| /// that the [TextScaler]'s' `textScaleFactor` equals `withScaleFactor`. | ||
| Matcher isSystemTextScaler({ double? withScaleFactor }) { |
There was a problem hiding this comment.
Wanna add that context to the doc? I think it would be useful to specify what cases would fail this check (i.e. when developers introduce their own text scaler in a widget)
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
@LongCatIsLooong Heads up on this old PR if you plan to return to it after break. |
LongCatIsLooong
added
the
autosubmit
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/160120, because - The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label. |
LongCatIsLooong
added
the
autosubmit
flutter-dashboard
Bot
removed
the
autosubmit
3 participants
This is for #159999. That PR breaks registered tests so a new matcher is added for soft transition & making it slightly easier to write tests that verify nothing is shadowing the system text scaler in the widget tree.
If this approach sounds plausible & gets merged, I'm going to:
PlatformDispatcher#159999 as ready for review.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.