Small fonts on macOS
Fixes #309 Added a text rescaler on macOS only.
The root cause is the usage of hardcoded fontSize in many different TextStyle in the ThemeData and in the BottomNavigationBar.
I applied a simple textScaler for macOS only, avoiding impacting too many places, breaking the mobile versions.
"flutter pub deps" added many new dependendcies. Not sure if all are required by my change or just some other unsynced stuff.
I am quite new to Flutter, so I am probably wrong.
I considered both approaches (ThemeData and MediaQuery). My understanding is that MediaQuery is a very light class which rarely changes. The global overhead should be negligible, but to be sure we need a well done profiling phase, which is probably too early, considering the initial stage of the app.
ThemeData requires a lot more testing to tune each single widget, so for a negligible effort (if I am not wrong) I went for the MediaQuery
Querying using MediaQuery.of will cause your widget to rebuild automatically whenever any field of the MediaQueryData changes (e.g., if the user rotates their device). Therefore, unless you are concerned with the entire MediaQueryData object changing, prefer using the specific methods (for example: MediaQuery.sizeOf and MediaQuery.paddingOf), as it will rebuild more efficiently.
This came from the official documentation, so IMO is not that "small class", but the most important part is that it will refresh the whole widget tree.
Consider for example a user that on macOS, resize the window, this will trigger the rebuild of the WHOLE app, for each logical pixel that changes of the screen, another example is when on mobile device the user change the orientation or changes the screen brightness, and so on.
I didn't manage to test it yet, but seems scary to me just by looking at the code.
Ofc this is just my opinion, I leave the final word to @mikev-cw and @theperu
Just tried the devtools to profile the app, on macos repeating multiple controlled resizing, testing without builder/MediaQuery and with it. I cannot spot any relevant difference in terms of:
- user perceivable lag
- UI avg ms/frame
- Raster avg ms/frame
- number of widget builds
- number and duration of janks
If I am using the profiler in the wrong way, please point out a better procedure to measure the impact of the change.
What @fres-sudo is pointing out is correct.
Also, textScaleFactor has been deprecated for a while now, textScaler should be used instead.
On a general note, I think 8px text, even on Android, is too small and generally against accessibility guidelines.
If we take in consideration the WCAG standard, even if there isn't a minimum size, it is recommended to use at least a 12px font size.
Part of my day to day job is ensuring the widgets we build are accessible (not only on font size, but also color contrast, keyboard navigation, screen readers and other assistive devices) and we use 12px only for legal copies, the minimum font size is 16px.
Thanks for the confirmation @bongio94 About the font styling, I usually find it easy and consistent to stick with the Material Typography Guidelines.
Btw, this seems a bit out of scope for this PR, if @theperu and @mikev-cw agree, we can close this and create a separate issue for the typography refactoring.
@fres-sudo @bongio94 thanks for your inputs.
Regarding the highlighted issues:
- Generated files: removed
- 8px fonts: I agree that font size in the navigation bar are too small, but in this PR I wanted to touch only macOS stuff and avoid impacting the app no mobile.
- Solution with MediaQuery: I am aware and agree that the MediaQuery could have an impact on redraws. So far, I am not able to spot any measurable degradation: please help me here to improve the profiling if needed. For the time being, I think that this approach is an acceptable trade-off compared to writing a custom macOS theme which requires a lot more work and testing and the need to open the app to macOS users too.