Override MediaQuery for WidgetsApp #81295

atoka93 · 2021-04-27T14:14:40Z

Description

Motivation described in #81293.

With these changes WidgetsApp will reuse an already existing MediaQuery if there is one.
It also enables moving the definition of MediaQuery up the tree, reusing the same logic used by WidgetsApp in other places by making MediaQueryFromWindow public.

Fixes #81293
Fixes #80931

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt.
All existing and new tests are passing.

goderbauer · 2021-04-28T21:44:47Z

Looks like some tests are failing, can you take a look at those?

goderbauer

This behavior should also be documented somewhere on WidgetApp

packages/flutter/lib/src/widgets/app.dart

atoka93 · 2021-04-29T08:39:35Z

Is the Linux framework_tests_libraries failing caused by the changes?

goderbauer · 2021-05-05T21:41:36Z

Is the Linux framework_tests_libraries failing caused by the changes?

Looked like an unrelated flake. I restarted it to see if it passes now.

goderbauer · 2021-05-05T22:05:03Z

packages/flutter/lib/src/widgets/app.dart

@@ -108,6 +108,9 @@ typedef InitialRouteListFactory = List<Route<dynamic>> Function(String initialRo
 /// It is used by both [MaterialApp] and [CupertinoApp] to implement base
 /// functionality for an app.
 ///
+/// If a [MediaQuery] is available above this widget a new one will not be
+/// created by [WidgetsApp].


Document that it builds a MediaQuery with data derived from WidgetsBinding.window if no MediaQuery is available?

I eventually went with:

/// If a [MediaQuery] is not available above [WidgetsApp] a [MediaQuery] is /// built using [MediaQueryFromWindow].

because it's concise and builds a [MediaQuery] with data derived from [WidgetsBinding.window] is already included under See also: ... *[MediaQueryFromWindow].

I was previously considering:

/// Builds a [MediaQuery] with data derived from [WidgetsBinding.window] if /// [MediaQuery] is not available above [WidgetsApp]. If a [MediaQuery] is /// available a new one will not be created by [WidgetsApp].

Let me know if you think that the second or some other variant would make more sense.

This looks good, thanks for clarifying the docs.

packages/flutter/lib/src/widgets/app.dart

Piinks

Just a few documentation bits - this is a nice change!

packages/flutter/lib/src/widgets/media_query.dart

Piinks · 2021-06-18T19:12:41Z

packages/flutter/lib/src/widgets/media_query.dart

+  /// [MediaQuery] using the latest [WidgetsBinding.window] values.
+  ///
+  /// The [MediaQuery] is wrapped in a separate widget to ensure that only it
+  /// and its dependents are updated when `window` changes instead of rebuilding


Nit:

Suggested change

/// and its dependents are updated when `window` changes instead of rebuilding

/// and its dependents are updated when `window` changes. instead of rebuilding

Did you mean a comma here? The dot didn't make sense for me 🤔
I added a comma for now, let me know

Ah yes, thank you. Pointed out a typo with a typo. 😋

packages/flutter/lib/src/widgets/app.dart

atoka93 · 2021-06-21T18:33:44Z

@Piinks I updated the documentation.
Some checks were cancelled and I don't seem to be able to find a way to re-trigger them.

Piinks · 2021-06-22T23:26:34Z

Hey @atoka93, thanks for updating. Can you rebase your branch with the latest from master? Those checks had an infra issue that has been resolved now.

to enable reusing this functionality

so it follows the _${widgetName}State pattern

Co-authored-by: Michael Goderbauer <goderbauer@google.com>

Co-authored-by: Kate Lovett <katelovett@google.com>

atoka93 · 2021-06-23T04:52:43Z

That did the trick, thank you @Piinks!

fluttergithubbot · 2021-06-23T16:46:02Z

This pull request is not suitable for automatic merging in its current state.

Please get at least one approved review before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

atoka93 · 2021-06-23T17:13:43Z

@Piinks could you approve it? 🙏

Piinks

atoka93 · 2021-06-23T17:58:06Z

Thank you @Piinks! 🚀

This reverts commit b9bf576.

zs-dima · 2021-07-20T22:45:23Z

@Piinks @atoka93
Why this changes had been reverted back ?

flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Apr 27, 2021

google-cla bot added the cla: yes label Apr 27, 2021

atoka93 changed the title ~~Override mediaquery~~ Override MediaQuery for WidgetsApp Apr 27, 2021

goderbauer reviewed Apr 28, 2021

View reviewed changes

packages/flutter/lib/src/widgets/app.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/widgets/app.dart Outdated Show resolved Hide resolved

atoka93 requested a review from goderbauer April 29, 2021 08:39

goderbauer reviewed May 5, 2021

View reviewed changes

flutter-dashboard bot added the f: material design flutter/packages/flutter/material repository. label May 6, 2021

atoka93 requested a review from goderbauer May 6, 2021 10:36

atoka93 requested a review from Hixie June 9, 2021 22:41

Piinks added a: quality A truly polished experience c: new feature Nothing broken; request for a new capability labels Jun 10, 2021

Piinks reviewed Jun 18, 2021

View reviewed changes

atoka93 requested a review from Piinks June 22, 2021 18:36

atoka93 and others added 12 commits June 23, 2021 06:50

Allow overriding the media query

05088c2

Make MediaQueryFromWindow public

b73cfc6

to enable reusing this functionality

Remove unnecessary complexity

ac5f677

Change the class name in the test

0514f47

Adjust the state name

74880dd

so it follows the _${widgetName}State pattern

Document that MediaQuery can be overwritten

7728bf2

Provide more documentation about MediaQueryFromWindow

082aff2

Do not expose the private class in the public API

aa37412

Co-authored-by: Michael Goderbauer <goderbauer@google.com>

Improve the documentation further

dccb557

Correct typo

d532635

Move MediaQueryFromWindow to media_query

f1c36c4

Create MediaQuery.fromWindow

679cad5

atoka93 and others added 5 commits June 23, 2021 06:50

Add MediaQuery.fromWindow tests

fe541df

Make MediaQueryFromWindow private

26b14be

Remove typo

c324a38

Co-authored-by: Kate Lovett <katelovett@google.com>

Add comma

ca270b3

Co-authored-by: Kate Lovett <katelovett@google.com>

Add comma

dad26db

Piinks added the waiting for tree to go green label Jun 23, 2021

fluttergithubbot removed the waiting for tree to go green label Jun 23, 2021

Piinks approved these changes Jun 23, 2021

View reviewed changes

Piinks added the waiting for tree to go green label Jun 23, 2021

fluttergithubbot merged commit b9bf576 into flutter:master Jun 23, 2021

renyou added a commit that referenced this pull request Jun 24, 2021

Revert "Override MediaQuery for WidgetsApp (#81295)"

316b431

This reverts commit b9bf576.

renyou mentioned this pull request Jun 24, 2021

Revert "Override MediaQuery for WidgetsApp" #85223

Merged

renyou added a commit that referenced this pull request Jun 24, 2021

Revert "Override MediaQuery for WidgetsApp (#81295)" (#85223)

9a981b9

This reverts commit b9bf576.

atoka93 mentioned this pull request Jun 25, 2021

WIP - Reland "Override MediaQuery for WidgetsApp (#81295)" #85298

Merged

8 tasks

zs-dima mentioned this pull request Jul 21, 2021

Ability to move app Theme below MaterialApp (Make MediaQuery be available while constructing app Theme) #86783

Open

fluttergithubbot pushed a commit that referenced this pull request Jul 27, 2021

WIP - Reland "Override MediaQuery for WidgetsApp (#81295)" (#85298)

ea1182e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Override MediaQuery for WidgetsApp #81295

Override MediaQuery for WidgetsApp #81295

atoka93 commented Apr 27, 2021 •

edited by Piinks

goderbauer commented Apr 28, 2021

goderbauer left a comment

atoka93 commented Apr 29, 2021

goderbauer commented May 5, 2021

goderbauer May 5, 2021

atoka93 May 6, 2021

Piinks Jun 18, 2021

Piinks left a comment

Piinks Jun 18, 2021

atoka93 Jun 21, 2021

Piinks Jun 22, 2021

atoka93 commented Jun 21, 2021 •

edited

Piinks commented Jun 22, 2021

atoka93 commented Jun 23, 2021

fluttergithubbot commented Jun 23, 2021

atoka93 commented Jun 23, 2021

Piinks left a comment

atoka93 commented Jun 23, 2021

zs-dima commented Jul 20, 2021 •

edited

	/// and its dependents are updated when `window` changes instead of rebuilding
	/// and its dependents are updated when `window` changes. instead of rebuilding

Override MediaQuery for WidgetsApp #81295

Override MediaQuery for WidgetsApp #81295

Conversation

atoka93 commented Apr 27, 2021 • edited by Piinks

Description

Pre-launch Checklist

goderbauer commented Apr 28, 2021

goderbauer left a comment

Choose a reason for hiding this comment

atoka93 commented Apr 29, 2021

goderbauer commented May 5, 2021

goderbauer May 5, 2021

Choose a reason for hiding this comment

atoka93 May 6, 2021

Choose a reason for hiding this comment

Piinks Jun 18, 2021

Choose a reason for hiding this comment

Piinks left a comment

Choose a reason for hiding this comment

Piinks Jun 18, 2021

Choose a reason for hiding this comment

atoka93 Jun 21, 2021

Choose a reason for hiding this comment

Piinks Jun 22, 2021

Choose a reason for hiding this comment

atoka93 commented Jun 21, 2021 • edited

Piinks commented Jun 22, 2021

atoka93 commented Jun 23, 2021

fluttergithubbot commented Jun 23, 2021

atoka93 commented Jun 23, 2021

Piinks left a comment

Choose a reason for hiding this comment

atoka93 commented Jun 23, 2021

zs-dima commented Jul 20, 2021 • edited

atoka93 commented Apr 27, 2021 •

edited by Piinks

atoka93 commented Jun 21, 2021 •

edited

zs-dima commented Jul 20, 2021 •

edited