It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@Piinks, both examples are based on examples/catalog/basic_app_bar.dart and you can find them here.

The difference you pointed wasn't introduced by this PR because the position is calculated here, way before those widgets, but you couldn't notice so much before since the PopupMenuButton was not avoiding the status bar.

@Piinks, both examples are based on examples/catalog/basic_app_bar.dart and you can find them here.

The difference you pointed wasn't introduced by this PR because the position is calculated here, way before those widgets, but you couldn't notice so much before since the PopupMenuButton was not avoiding the status bar.

Thanks for clarifying and providing the code! @shihaohong went ahead (and saved us a bunch of time! 🎉 ) checking this against the tests it would undergo during the roll to the dev channel (See Rolling to the dev channel), and it looks like this will be a breaking change. Breaking Changes require a few more steps in order to land, which are laid out in that wiki link. This change will probably need to be phased, with an initial opt-in flag to prevent breaking developers.

It will also need tests. It looks like the current framework tests don't check the alignment, so this is awesome that our coverage will be better here! Check out the wiki links I provided and let me know if you need any guidance on proceeding with this change. :)

@arthurdenner would you like to move forward with this change?

Sorry, but unfortunately I can't - forgot to reply here, thanks for the ping. Breaking changes usually take some time and I'm not so familiar with the inner bits of the framework. Last time I tried to implement a breaking change here, it took many days and the PR ended up being closed. 😕

I'd rather leave the changes to someone more experienced with the inner bits and with more time to deal with the interaction. I'd just kindly ask you to update the labels on the linked issue because it's currently tagged as easy fix and that may mislead someone else.

Hi @Piinks, I would like to proceed with this change based on this pull request.
I wrote a design doc, and now writing a test for this change.

I'm thinking of comparing the status bar height and the first popupMenuItem's y axis. (If the top left y axis of first popupMenuItem is greater than the status bar height, we can say that they are not overlapping.)
However, I'm struggling with getting the status bar height - we can easily get it when running the emulator, but seems like there is no status bar on the testing environment. 😓
Can I get some help with writing test about this change?

Hi @Piinks, I would like to proceed with this change based on this pull request.
I wrote a design doc, and now writing a test for this change.

I'm thinking of comparing the status bar height and the first popupMenuItem's y axis. (If the top left y axis of first popupMenuItem is greater than the status bar height, we can say that they are not overlapping.)
However, I'm struggling with getting the status bar height - we can easily get it when running the emulator, but seems like there is no status bar on the testing environment. 😓
Can I get some help with writing test about this change?

Hey @JuYeong0413 thanks for working on a solution for this!
The status bar would be represented in MediaQuery.data as a MediaQueryData.viewInsets IIRC, but it may be MediaQueryData.viewPadding. I would check to see what MediaQueryData is informing the layout of your widgets. In order to test for this, you can create your own MediaQuery in your test and mimic the MediaQueryData you see when executing on your emulator.

I recently wrote a test like this to check safe area, here I included MediaQueryData.viewPadding in my test to simulate a notch on the bottom: https://github.com/flutter/flutter/pull/64268/files

Let me know if this helps in your test writing! Thanks! :)

Thank you for your kind guidance @Piinks!
I have created MediaQuery.data for mimicking status bar and bottom notch, but I'm getting the same results before&after wrapping PopupMenu with SafeArea. 😞
Executing the same widgets on the emulator works as expected, so I think there's some problem with MediaQueryData or getTopLeft/getBottomLeft of PopupMenu in my test code. (BTW, using MediaQueryData.viewInsets and MediaQueryData.viewPadding both give me the same result when representing the status bar... I think MediaQueryData.viewInsets may be correct.)

Here are the tests I wrote:

1. few items in AppBar(overlapping with status bar)
2. many items covering the screen vertically(overlapping with status bar&bottom notch)

I've tried for several days, but cannot figure out why these tests are giving the same result although the PopupMenu is wrapped with SafeArea. 🤔
Could you please help me with this?

@JuYeong0413 do you have an open PR with these changes? It may be easier to discuss there. Feel free to ping me on a PR for review. :)