Add Directionality.maybeOf#69117
Conversation
| final ThemeData theme = Theme.of(context)!; | ||
| final ChipThemeData chipTheme = ChipTheme.of(context); | ||
| final TextDirection? textDirection = Directionality.of(context); | ||
| final TextDirection? textDirection = Directionality.maybeOf(context); |
There was a problem hiding this comment.
Here and below: should this really be nullable? I understand it was, but it might help to have a comment here about why we're allowing this to be null instead of using .of.
There was a problem hiding this comment.
Ultimately, they are passed to EdgeInsetsGeometry.resolve, which is fine with a null text direction as long as the EdgeInsetsGeometry is not text direction aware. Similar with the other cases.
We actually have tests for these. If you make it non-null here, they will fail.
| /// ```dart | ||
| /// TextDirection textDirection = Directionality.of(context); | ||
| /// ``` | ||
| // TODO(goderbauer): Make this non-null when customers have upgraded to Directionality.maybeOf. |
There was a problem hiding this comment.
Should we start encouraging people to migrate by adding an assert here with a message saying to use maybeOf if the resolved direction is null?
There was a problem hiding this comment.
That could be kinda spammy. I think we usually don't do that.
There was a problem hiding this comment.
Won't that effectively be what users gets though when we actually make this non-null? But probably without the benefit of a message printing next to the error that they really should use the other variant...
There was a problem hiding this comment.
We can make the error message link to TextDirection.maybeOf.
There was a problem hiding this comment.
How does that work for compile time errors?
There was a problem hiding this comment.
What do you mean? The assert added will be a runtime error.
There was a problem hiding this comment.
We will basically just call https://master-api.flutter.dev/flutter/widgets/debugCheckHasDirectionality.html at the beginning of this method.
There was a problem hiding this comment.
Discussed offline - most apps will get directionality from material/cupertinoapp, probably not worth worrying about outside of tests.
goderbauer
added
framework
2 participants
Description
Adds
Directionality.maybeOf(and uses it in the framework) in preparation of makingDirectionality.ofnon-nullable. Making it non-nullable is not part of this PR since we first need to migrate some customers (flutter_svg) to the newmaybeOfAPI before we can do that.Checklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.