Skip to content
This repository has been archived by the owner on Feb 4, 2022. It is now read-only.

Add central TextStyle #366

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

999eagle
Copy link
Member

This PR adds a default TextStyle and some helper methods to the ThemeService and replaces (I hope) all uses of TextStyle(...) with one of the helper methods from the ThemeService. I have also added the free font DejaVu Sans as default fallback, but feel free to change that font.

The default font and font fallbacks can now be changed in a single central location (well, actually in lib/main.dart and lib/services/theme.dart). With this change we don't have to rely on the OS supplying Unicode characters and can make sure that even older devices without up to date Unicode fonts can render user names and posts containing newer Unicode characters.

@uiboy
Copy link
Member

uiboy commented Aug 17, 2019

🤔 hmm, maybe Im missing something but I still think we should create a OBTextStyle widget here for this use case. Which will accept all the optional params that the flutter TextStyle does, and just pass them along setting the default fontfamily that you need for desktop/mobile along the way.
We do this to handle theming for many existing widgets, for eg. check OBText which uses the current primary theme color and renders some text.

We also have a OBSecondaryText widget to reflect that the text should take the current active themes secondary color.

This widget wrapper is IMO better than having to pull this information from the themeService each time and merging with the other custom styles.

@lifenautjoe thoughts?

@999eagle
Copy link
Member Author

Yeah, I've implemented this for those two widgets as well, but the basic Text is used very often instead of the OBText and OBSecondaryText widgets, so I wanted to keep it that way. If you want, I can change this of course to use the custom widgets instead of flutter's Text widget.

@999eagle
Copy link
Member Author

@uiboy @lifenautjoe any update on how to proceed?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants