-
Notifications
You must be signed in to change notification settings - Fork 66
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
Migration of textScaleFactor to textScaler #830
Conversation
One thing to note is that we need to use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, thanks for taking this on! And nice cleanup too. The ScalableText
definitely makes this transition easier.
Things look good on Android. The base size is unchanged, which is good. 😊
I'll keep this as a draft and wait until this deprecation change reaches the upstream |
…/text-scale-factor-migration
The deprecation change has made its way to the stable branch, so I'm opening this up for review |
* Migration of textScaleFactor to textScaler (#830) * migration of to be deprecated textScalingFactor, and a lot of linter fixes * added ScalableText to missing instance * linting * Added option to collapse post body (#888) * added option to collapse a post in the post view * added ability to tap on post preview to expand post, linting * moved changelog to proper place
Pull Request Description
This PR is mainly under-the-hood changes to migrate the deprecated
textScaleFactor
to usetextScaler
. For more information about the deprecation, see here: https://docs.flutter.dev/release/breaking-changes/deprecate-textscalefactorTo accomplish this, I've created a new widget
ScalableText
which works similar to the regularText
widget, except it accepts afontScale
parameter. ThefontScale
parameter is defined here. The main reason for creating this new widget is to hopefully simplify the logic for calculating the finalfontSize
attribute required.Previously, to apply a
textScaleFactor
, all we had to do was add the parameter to anyText
widgetTo have the equivalent functionality as before using
TextScaler
, the following changes had to be made. Note that the scaling is dependent on thetextTheme
we choose (e.g.,bodyMedium
,titleSmall
, etc.)By using the new
ScalableText
widget, that migration code changes to the followingThe
ScalableText
takes any existing styling that was initially applied, and appends the appropriatefontSize
based on thefontScale
that is passed in. This should help simplify things in the future when we want to add more resizable text elements. This approach also considers the system's font size like the previous implementation.In addition, I've fixed up some linting warnings throughout the whole codebase to clean up warnings and errors. Since some of the code was adjusted for this migration, we should double check and see if the text scaling is roughly the same as it was before, and adjust accordingly if not.
@micahmo could you run through this and see if you notice any changes to the text size from the current
develop
build vs this change? I'll keep it as a draft until more checks are done to confirm that the general text sizes have not changed much!Issue Being Fixed
Issue Number: N/A
Screenshots / Recordings
Checklist
semanticLabel
s where applicable for accessibility?