Skip to content

Add display features to MediaQuery#92906

Merged
fluttergithubbot merged 5 commits intoflutter:masterfrom
andreidiaconu:foldable_support_mediaquery
Dec 21, 2021
Merged

Add display features to MediaQuery#92906
fluttergithubbot merged 5 commits intoflutter:masterfrom
andreidiaconu:foldable_support_mediaquery

Conversation

@andreidiaconu
Copy link
Contributor

@andreidiaconu andreidiaconu commented Nov 2, 2021 •
edited
Loading

This PR adds DisplayFeature to MediaQuery in the form of two new properties: displayFeatures and hinge. This is split from a larger PR for foldable support in order to make it easier to review (#77156).

Issues that will be fixed by this PR:

Depends on:

PRs that depend on this:

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 this PR is test-exempt.
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Nov 2, 2021
@google-cla google-cla bot added the cla: yes label Nov 2, 2021
This was referenced Nov 2, 2021
Merged
Merged
Closed
Closed
@andreidiaconu andreidiaconu marked this pull request as ready for review November 2, 2021 12:45
@andreidiaconu
Copy link
Contributor Author

andreidiaconu commented Nov 2, 2021

Tests will not pass until the engine support lands but since the code is ready to be reviewed, I am marking this as ready for review.

@andreidiaconu andreidiaconu changed the title Add display features to MediaQuery and TestWindow Add display features to MediaQuery Nov 2, 2021
goderbauer
goderbauer reviewed Nov 2, 2021
View reviewed changes
@goderbauer
Copy link
Member

goderbauer commented Nov 2, 2021

/cc @Piinks

goderbauer
goderbauer approved these changes Nov 3, 2021
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once tests pass (w/ engine commit)

Per our policy this will also need a second LGTM to land. I'll find a reviewer.

justinmc
justinmc approved these changes Nov 3, 2021
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Just a docs nit and a comment about confirming List vs. Set.

@Piinks Piinks self-requested a review November 11, 2021 21:57
@Piinks Piinks added c: new feature Nothing broken; request for a new capability a: layout SystemChrome and Framework's Layout Issues labels Nov 11, 2021
@Piinks
Copy link
Contributor

Piinks commented Nov 17, 2021

(Triage) I think this is blocked on

Which are blocked on flutter/plugins#4502

@andreidiaconu if that's not correct, can you update?

@andreidiaconu
Copy link
Contributor Author

andreidiaconu commented Nov 18, 2021

(Triage) I think this is blocked on

Which are blocked on flutter/plugins#4502

@andreidiaconu if that's not correct, can you update?

That is correct.

@andreidiaconu
Copy link
Contributor Author

andreidiaconu commented Nov 22, 2021

@blasten @Piinks I think this can be merged now.

@Piinks
Copy link
Contributor

Piinks commented Dec 9, 2021

Hey @andreidiaconu can you update this with the latest from master? The other changes have rolled in, so the Google testing shard should pass with an update.

@andreidiaconu
Copy link
Contributor Author

andreidiaconu commented Dec 9, 2021

@Piinks Yes & done 👍

@Piinks Piinks mentioned this pull request Dec 9, 2021
Closed
@Piinks
Copy link
Contributor

Piinks commented Dec 9, 2021

Thanks for rebasing, I am looking into this customer test failure, I am not sure if it is caused by your change. I linked an infra issue above that is tracking this.
Since it does not look related, can you try rebasing again? Previously the customer tests were not broken, so you may have just inadvertently picked up a bad state.

@Piinks
Copy link
Contributor

Piinks commented Dec 9, 2021

So, I have checked the customer test out locally and ran it with your changes here and they all passed. I think an update will finally allow this to land. :) Thanks for your patience.

@goderbauer
Copy link
Member

goderbauer commented Dec 15, 2021

I re-ran the failing test. If that doesn't make it pass, you'll probably have to rebase this to the latest master.

@Piinks
Copy link
Contributor

Piinks commented Dec 16, 2021

It is still failing after re-run, can you rebase once more?

@andreidiaconu
Copy link
Contributor Author

andreidiaconu commented Dec 17, 2021 •
edited
Loading

Rebased Merged latest master

@Piinks
Copy link
Contributor

Piinks commented Dec 20, 2021

@Hixie can you take a look at this rainbowmonkey failure?
I've seen it pop up on other PRs, but it usually goes away after rebasing. It does not appear to be related to this change.

@nt4f04uNd
Copy link
Member

nt4f04uNd commented Dec 21, 2021 •
edited
Loading

I recall that I once had it and it was solved when I used git rebase instead, you might try it

@Hixie
Copy link
Contributor

Hixie commented Dec 21, 2021

That was an unrelated issue due to rolling Dart, you should be able to resolve it by updating your PR to top of tree (i.e. rebasing as @nt4f04uNd says). You just got unlucky about what commit you started from.

@andreidiaconu andreidiaconu force-pushed the foldable_support_mediaquery branch from ecda5de to bc73916 Compare December 21, 2021 10:39
@andreidiaconu
Copy link
Contributor Author

andreidiaconu commented Dec 21, 2021

Rebased this time around.

@Piinks
Copy link
Contributor

Piinks commented Dec 21, 2021

That did it! Thanks all! #teamwork 😉

@fluttergithubbot fluttergithubbot merged commit a35e7aa into flutter:master Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@justinmc justinmc justinmc approved these changes

@goderbauer goderbauer goderbauer approved these changes

@Piinks Piinks Awaiting requested review from Piinks

Assignees

No one assigned

Labels

a: layout SystemChrome and Framework's Layout Issues a: tests "flutter test", flutter_test, or one of our tests c: new feature Nothing broken; request for a new capability framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@andreidiaconu @goderbauer @Piinks @nt4f04uNd @Hixie @justinmc @fluttergithubbot