Repository navigation
shrink the MediaQuery API surface - #194091
nate-thegrate wants to merge 7 commits into
Conversation
`runApp` always inserts a `MediaQuery`, so most `maybeOf` methods are unnecessary. Nullable fields keep the maybe prefix. That includes the text-style overrides and display corner radii, so displayCornerRadiiOf is deprecated in favor of maybeDisplayCornerRadiiOf.
Also moves content in packages/flutter/test_fixes/widgets/ accordingly.
Update call sites whose null result only meant that no MediaQuery was present. The non-maybe accessors already apply the same fallback, so behavior is unchanged when an ancestor exists.
|
This pull request contains changes to Material or Cupertino, which are currently frozen in this repository. Changes should be made in Please refer to #188444 for instructions. |
github-actions
Bot
added
a: text input
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request deprecates debugCheckHasMediaQuery and the maybe... variant methods of MediaQuery (such as maybeOf and maybeSizeOf), as runApp now always introduces a MediaQuery. It updates documentation, removes obsolete assertions, and adds declarative data fixes to migrate users to the non-nullable ...Of methods. The review feedback suggests adding missing URIs for consistency in the fromView and boldTextOf migrations, and simplifying several ...Of methods in media_query.dart by directly calling _of instead of using unnecessary null-coalescing fallbacks.
| static TextScaler textScalerOf(BuildContext context) => | ||
| maybeTextScalerOf(context) ?? TextScaler.noScaling; | ||
| _maybeOf(context, _MediaQueryAspect.textScaler)?.textScaler ?? TextScaler.noScaling; |
There was a problem hiding this comment.
Since runApp always introduces a MediaQuery and there is no need to handle a missing MediaQuery (as established by the deprecation of the maybe... methods), we can simplify textScalerOf to use _of directly. This avoids the unnecessary null-coalescing fallback and makes the behavior consistent with other ...Of methods (like sizeOf, paddingOf, etc.) which throw if MediaQuery is missing.
static TextScaler textScalerOf(BuildContext context) => _of(context, _MediaQueryAspect.textScaler).textScaler;References
- Suggest simplification and refactoring: Assess whether the code can be made simpler or refactored to enhance readability and maintainability. (link)
| static Brightness platformBrightnessOf(BuildContext context) => | ||
| maybePlatformBrightnessOf(context) ?? Brightness.light; | ||
| _maybeOf(context, _MediaQueryAspect.platformBrightness)?.platformBrightness ?? | ||
| Brightness.light; |
There was a problem hiding this comment.
Since runApp always introduces a MediaQuery and there is no need to handle a missing MediaQuery (as established by the deprecation of the maybe... methods), we can simplify platformBrightnessOf to use _of directly. This avoids the unnecessary null-coalescing fallback and makes the behavior consistent with other ...Of methods (like sizeOf, paddingOf, etc.) which throw if MediaQuery is missing.
static Brightness platformBrightnessOf(BuildContext context) => _of(context, _MediaQueryAspect.platformBrightness).platformBrightness;References
- Suggest simplification and refactoring: Assess whether the code can be made simpler or refactored to enhance readability and maintainability. (link)
| static bool highContrastOf(BuildContext context) => | ||
| _maybeOf(context, _MediaQueryAspect.highContrast)?.highContrast ?? false; |
There was a problem hiding this comment.
Since runApp always introduces a MediaQuery and there is no need to handle a missing MediaQuery (as established by the deprecation of the maybe... methods), we can simplify highContrastOf to use _of directly. This avoids the unnecessary null-coalescing fallback and makes the behavior consistent with other ...Of methods (like sizeOf, paddingOf, etc.) which throw if MediaQuery is missing.
static bool highContrastOf(BuildContext context) => _of(context, _MediaQueryAspect.highContrast).highContrast;References
- Suggest simplification and refactoring: Assess whether the code can be made simpler or refactored to enhance readability and maintainability. (link)
| static bool onOffSwitchLabelsOf(BuildContext context) => | ||
| maybeOnOffSwitchLabelsOf(context) ?? false; | ||
| _maybeOf(context, _MediaQueryAspect.onOffSwitchLabels)?.onOffSwitchLabels ?? false; |
There was a problem hiding this comment.
Since runApp always introduces a MediaQuery and there is no need to handle a missing MediaQuery (as established by the deprecation of the maybe... methods), we can simplify onOffSwitchLabelsOf to use _of directly. This avoids the unnecessary null-coalescing fallback and makes the behavior consistent with other ...Of methods (like sizeOf, paddingOf, etc.) which throw if MediaQuery is missing.
static bool onOffSwitchLabelsOf(BuildContext context) => _of(context, _MediaQueryAspect.onOffSwitchLabels).onOffSwitchLabels;References
- Suggest simplification and refactoring: Assess whether the code can be made simpler or refactored to enhance readability and maintainability. (link)
| static bool boldTextOf(BuildContext context) => | ||
| _maybeOf(context, _MediaQueryAspect.boldText)?.boldText ?? false; |
There was a problem hiding this comment.
Since runApp always introduces a MediaQuery and there is no need to handle a missing MediaQuery (as established by the deprecation of the maybe... methods), we can simplify boldTextOf to use _of directly. This avoids the unnecessary null-coalescing fallback and makes the behavior consistent with other ...Of methods (like sizeOf, paddingOf, etc.) which throw if MediaQuery is missing.
static bool boldTextOf(BuildContext context) => _of(context, _MediaQueryAspect.boldText).boldText;References
- Suggest simplification and refactoring: Assess whether the code can be made simpler or refactored to enhance readability and maintainability. (link)
| static bool supportsAnnounceOf(BuildContext context) => | ||
| _maybeOf(context, _MediaQueryAspect.supportsAnnounce)?.supportsAnnounce ?? false; |
There was a problem hiding this comment.
Since runApp always introduces a MediaQuery and there is no need to handle a missing MediaQuery (as established by the deprecation of the maybe... methods), we can simplify supportsAnnounceOf to use _of directly. This avoids the unnecessary null-coalescing fallback and makes the behavior consistent with other ...Of methods (like sizeOf, paddingOf, etc.) which throw if MediaQuery is missing.
static bool supportsAnnounceOf(BuildContext context) => _of(context, _MediaQueryAspect.supportsAnnounce).supportsAnnounce;References
- Suggest simplification and refactoring: Assess whether the code can be made simpler or refactored to enhance readability and maintainability. (link)
|
I took care not to change any Material or Cupertino package files, but it looks like editing 
Edit: I decided to revert this change, hopefully |
|
Even though flutter/packages/flutter/test/widgets/rich_text_test.dart Lines 293 to 311 in 01d3eec Implementing Gemini's suggestions would break this test, so we'd need to figure out how to fix it. (Given how many different widgets already break without a MediaQuery, perhaps we ought to turn |
github-actions
Bot
removed
p: material_ui
Renzo-Olivares
removed
the
a: text input
|
Closing this PR based on the conversation in #183701. |
This pull request closes #183701 by deprecating several MediaQuery
maybeOfmethods and providing data-driven fixes.Also in this PR:
MediaQuery.displayCornerRadiiOf(see this comment)debugCheckHasMediaQuery(no data-driven fix was added since it isn't possible to automatically remove the method call along with the surroundingassert)