Add option to specify default text style in MaterialApp#12537
Closed
tvolkert wants to merge 1 commit intoflutter:masterfrom
tvolkert:textstyle
Closed
Add option to specify default text style in MaterialApp#12537tvolkert wants to merge 1 commit intoflutter:masterfrom tvolkert:textstyle
tvolkert wants to merge 1 commit intoflutter:masterfrom
tvolkert:textstyle
Conversation
Currently, MaterialApp inserts a `DefaultTextStyle` in its hierarchy
with a style of `_errorTextStyle`. This means that any material app
with a `Text` widget that:
(a) doesn't have another `DefaultTextStyle` widget in its ancestry
between it and the `MaterialApp`, and
(b) specifies an explicit style with `inherit: true`
... is susceptible to having its styles inherit from the app-level
error styles. In this case, it's forced to insert a `DefaultTextStyle`
as a *child* of the `MaterialApp`, which is a little counter-intuitive.
This is exacerbated by the change in #12249, which changed the default
platform text themes in `typography.dart` to `inherit: true`. While this
change was correct, it makes apps more susceptible to the behavior above.
Consider the case of an app using a `Text` widget and explicitly setting
a `style` on that widget of a platform default style. In that case, they're
*explicitly* saying "I want this text to have style X" (e.g.
`Typography.white.body1`), and because the default platform text styles
don't specify decorations, those get inherited from the top-level error
text styles.
This change adds the ability to give `MaterialApp` a `defaultTextStyle`,
and only to use `_errorTextStyles` if such a default is unspecified.
Contributor
|
This is the wrong solution. It's invalid to have text in a material app that isn't in a Material, and the Material introduces the right styles. |
Contributor
|
The right fix is probably to make the default text style be one that actually throws an exception when used, saying that you have to use a Material. |
Contributor
Author
|
ACK. Closing this, and I fixed the issue that was affecting the internal tests by fixing the tests to use Material correctly. thx. |
Contributor
|
Having said that, #12548 is similar and interesting... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, MaterialApp inserts a
DefaultTextStylein its hierarchywith a style of
_errorTextStyle. This means that any material appwith a
Textwidget that:(a) doesn't have another
DefaultTextStylewidget in its ancestrybetween it and the
MaterialApp, and(b) specifies an explicit style with
inherit: true... is susceptible to having its styles inherit from the app-level
error styles. In this case, it's forced to insert a
DefaultTextStyleas a child of the
MaterialApp, which is a little counter-intuitive.This is exacerbated by the change in #12249, which changed the default
platform text themes in
typography.darttoinherit: true. While thischange was correct, it makes apps more susceptible to the behavior above.
Consider the case of an app using a
Textwidget and explicitly settinga
styleon that widget of a platform default style. In that case, they'reexplicitly saying "I want this text to have style X" (e.g.
Typography.white.body1), and because the default platform text stylesdon't specify decorations, those get inherited from the top-level error
text styles.
This change adds the ability to give
MaterialAppadefaultTextStyle,and only to use
_errorTextStylesif such a default is unspecified.