Fix: Submenu anchor misaligned with child panel in web (Resolved #151081) #151294

Vi-debug · 2024-07-04T04:04:33Z

Fix: Submenu anchor misaligned with child panel in web (Resolved #151081)

The issue comes from different in calculating the position of the menu in web and mobile.
The calculation is currently base on function getPositionForChild() inside _MenuLayout in menu_anchor.dart, which base on the anchorRect
The calculation of anchorRect is from upperLeft and bottomRight
upperLeft is result of localToGlobal() function, which take the point arguments to be the base line. Right now, point is refer to Offset.zero, but it should not be Offset.zero since we having densityAdjustment, which is different between web and mobile
Change point from Offset.zero to Offset(dx, -dy) should fix the error. Use dx instead of -dx since dx already be recalculated refer to the above comment on densityAdjustment.
Before:

After:

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].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

flutter-dashboard · 2024-07-04T04:04:35Z

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 "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

google-cla · 2024-07-04T04:04:36Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

nate-thegrate

You can see the output from failing tests by going to
details → View more details on flutter-dashboard → stdout [raw]

I'll copy-paste the failing tests from Linux framework_tests_libraries (click to expand). Can you update these tests to reflect the new behavior?

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: [
            Rect:Rect.fromLTRB(161.0, 0.0, 639.0, 40.0),
            Rect:Rect.fromLTRB(265.0, 40.0, 467.0, 160.0),
            Rect:Rect.fromLTRB(467.0, 72.0, 707.0, 232.0)
          ]
  Actual: [
            Rect:Rect.fromLTRB(161.0, 0.0, 639.0, 40.0),
            Rect:Rect.fromLTRB(265.0, 40.0, 467.0, 160.0),
            Rect:Rect.fromLTRB(467.0, 80.0, 707.0, 240.0)
          ]
   Which: at location [2] is Rect:<Rect.fromLTRB(467.0, 80.0, 707.0, 240.0)> instead of
Rect:<Rect.fromLTRB(467.0, 72.0, 707.0, 232.0)>

When the exception was thrown, this was the stack:
#4      main.<anonymous closure>.<anonymous closure> (…/flutter/test/material/menu_anchor_test.dart:3179:7)
<asynchronous suspension>
#5      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:189:15)
<asynchronous suspension>
#6      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1032:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///b/s/w/ir/x/w/flutter/packages/flutter/test/material/menu_anchor_test.dart line 3179
The test description was:
  submenus account for compact menu density in LTR
════════════════════════════════════════════════════════════════════════════════════════════════════

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: [
            Rect:Rect.fromLTRB(161.0, 0.0, 639.0, 40.0),
            Rect:Rect.fromLTRB(333.0, 40.0, 535.0, 160.0),
            Rect:Rect.fromLTRB(93.0, 72.0, 333.0, 232.0)
          ]
  Actual: [
            Rect:Rect.fromLTRB(161.0, 0.0, 639.0, 40.0),
            Rect:Rect.fromLTRB(333.0, 40.0, 535.0, 160.0),
            Rect:Rect.fromLTRB(93.0, 80.0, 333.0, 240.0)
          ]
   Which: at location [2] is Rect:<Rect.fromLTRB(93.0, 80.0, 333.0, 240.0)> instead of
Rect:<Rect.fromLTRB(93.0, 72.0, 333.0, 232.0)>

When the exception was thrown, this was the stack:
#4      main.<anonymous closure>.<anonymous closure> (…/flutter/test/material/menu_anchor_test.dart:3195:7)
<asynchronous suspension>
#5      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:189:15)
<asynchronous suspension>
#6      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1032:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///b/s/w/ir/x/w/flutter/packages/flutter/test/material/menu_anchor_test.dart line 3195
The test description was:
  submenus account for compact menu density in RTL
════════════════════════════════════════════════════════════════════════════════════════════════════

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: Rect:<Rect.fromLTRB(249.0, 80.0, 483.0, 136.0)>
  Actual: Rect:<Rect.fromLTRB(257.0, 80.0, 491.0, 136.0)>

When the exception was thrown, this was the stack:
#4      main.<anonymous closure> (…/flutter/test/material/menu_anchor_test.dart:257:5)
<asynchronous suspension>
#5      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:189:15)
<asynchronous suspension>
#6      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1032:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///b/s/w/ir/x/w/flutter/packages/flutter/test/material/menu_anchor_test.dart line 257
The test description was:
  Menu responds to density changes
════════════════════════════════════════════════════════════════════════════════════════════════════

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: Rect:<Rect.fromLTRB(366.0, 68.0, 559.0, 82.0)>
  Actual: Rect:<Rect.fromLTRB(372.0, 68.0, 565.0, 82.0)>

When the exception was thrown, this was the stack:
#4      main.<anonymous closure>.<anonymous closure> (…/flutter/test/material/menu_style_test.dart:281:7)
<asynchronous suspension>
#5      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:189:15)
<asynchronous suspension>
#6      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1032:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///b/s/w/ir/x/w/flutter/packages/flutter/test/material/menu_style_test.dart line 281
The test description was:
  visual density
════════════════════════════════════════════════════════════════════════════════════════════════════

packages/flutter/lib/src/material/menu_anchor.dart

nate-thegrate · 2024-07-07T00:52:30Z

packages/flutter/lib/src/material/menu_anchor.dart

+    final BuildContext anchorContext = anchor._anchorKey.currentContext!;
+    final RenderBox overlay = Overlay.of(anchorContext).context.findRenderObject()! as RenderBox;
+    final RenderBox anchorBox = anchorContext.findRenderObject()! as RenderBox;
+    final Offset upperLeft = anchorBox.localToGlobal(Offset(dx, -dy), ancestor: overlay);


Nice job with the fix here!

Thanks. Hope to be merge

nate-thegrate

LGTM, thanks for making those test updates!

We'll be ready to merge once we get 1 more review 🙂

packages/flutter/test/material/menu_anchor_test.dart

…ter#151081)

…obile (Resolved flutter#151081)

…ce change

gspencergoog

…ved #151081) (flutter/flutter#151294)

@tool

Roll Flutter from 5103d75 to 58068d8 (42 revisions) flutter/flutter@5103d75...58068d8 2024-07-12 [email protected] Reland: Move all Linux Moto G4 tests to mokey in staging (flutter/flutter#151654) 2024-07-12 [email protected] Update obsolete comment in InputDecorator test (flutter/flutter#151651) 2024-07-12 [email protected] Fix `TabBar` tab indicator stretch effect (flutter/flutter#150868) 2024-07-12 [email protected] Remove workaround for a bug in dart2wasm (flutter/flutter#151603) 2024-07-12 [email protected] [native_assets] Stop running link hooks in JIT mode (flutter/flutter#151534) 2024-07-12 [email protected] Roll `Switch.adaptive` changes into `CupertinoSwitch` (flutter/flutter#149465) 2024-07-11 [email protected] Unmark java11 tests as bringup:true (flutter/flutter#151612) 2024-07-11 [email protected] Add link to design document archive (flutter/flutter#151489) 2024-07-11 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Move all Linux Moto G4 tests to mokey in staging (#151608)" (flutter/flutter#151620) 2024-07-11 [email protected] Move all Linux Moto G4 tests to mokey in staging (flutter/flutter#151608) 2024-07-11 [email protected] docimports for API samples (flutter/flutter#151606) 2024-07-11 [email protected] docimports for flutter_goldens, flutter_localizations, flutter_web_plugins, fuchsia_remote_debug_protocol, integration_test (flutter/flutter#151271) 2024-07-11 [email protected] Re-enable debug canvaskit e2e tests. (flutter/flutter#151565) 2024-07-11 [email protected] Fix: Submenu anchor misaligned with child panel in web (Resolved #151081) (flutter/flutter#151294) 2024-07-11 [email protected] Replaced {@tool snippet} with {@tool dartpad} in CupertinoTabController (flutter/flutter#151272) 2024-07-11 [email protected] feat: Support overriding native endorsed plugins (flutter/flutter#137040) 2024-07-11 [email protected] expose keyboardType in DropdownMenu #150894 (flutter/flutter#150896) 2024-07-11 [email protected] docimports for flutter_driver (flutter/flutter#151267) 2024-07-11 [email protected] Add `TimeOfDay` comparison methods (flutter/flutter#151233) 2024-07-11 [email protected] Roll Flutter Engine from 6534fbf3c2c1 to 36dccf7bb25c (2 revisions) (flutter/flutter#151577) 2024-07-11 [email protected] Roll Flutter Engine from 1c23c8f64076 to 6534fbf3c2c1 (3 revisions) (flutter/flutter#151572) 2024-07-11 [email protected] Use correct locale for `CupertinoDatePicker` weekday (flutter/flutter#151494) 2024-07-10 [email protected] doc imports for enum values (flutter/flutter#151548) 2024-07-10 [email protected] Reland "Upgrade template Gradle, App AGP, Module AGP, and Kotlin versions, and tests"... but no longer upgrade module AGP version (flutter/flutter#151433) 2024-07-10 [email protected] Roll Packages from 14341d1 to ea35fc6 (16 revisions) (flutter/flutter#151556) 2024-07-10 [email protected] [CupertinoActionSheet] Fix padding and font size of buttons (flutter/flutter#151199) 2024-07-10 [email protected] Roll Flutter Engine from db2b45bea2c0 to 1c23c8f64076 (2 revisions) (flutter/flutter#151550) 2024-07-10 [email protected] Add docImports for flutter_test references (flutter/flutter#151175) 2024-07-10 [email protected] Mention not @-mentioning people in commit messages in tree hygiene (flutter/flutter#151487) 2024-07-10 [email protected] Roll Flutter Engine from 371db85f33d7 to db2b45bea2c0 (8 revisions) (flutter/flutter#151522) 2024-07-10 [email protected] fix heading level absorption, diagnostics; add tests and an a11y use-case (flutter/flutter#151421) 2024-07-10 [email protected] Roll Flutter Engine from 9d943eb2b37a to 371db85f33d7 (3 revisions) (flutter/flutter#151505) 2024-07-10 [email protected] Roll Flutter Engine from d3269d5855a7 to 9d943eb2b37a (5 revisions) (flutter/flutter#151495) 2024-07-09 [email protected] Update doc of `SemanticsProperties.identifier` (flutter/flutter#149915) 2024-07-09 [email protected] Clean up leaky test. (flutter/flutter#151131) 2024-07-09 [email protected] Roll pub packages (flutter/flutter#151492) 2024-07-09 [email protected] testAdd tests for stepper.controls_builder.0.dart (flutter/flutter#150669) 2024-07-09 [email protected] Add Semantics Property `linkUrl` (flutter/flutter#150639) 2024-07-09 [email protected] Roll Flutter Engine from 4a2ac0e51a8f to d3269d5855a7 (1 revision) (flutter/flutter#151488) 2024-07-09 [email protected] Link engine docs on AS setup in flutter/flutter docs on engine contributor setup (flutter/flutter#151481) 2024-07-09 [email protected] Roll Flutter Engine from 69075e7e87d4 to 4a2ac0e51a8f (21 revisions) (flutter/flutter#151482) 2024-07-09 [email protected] Fix Material 3 `Dialog` default background color (flutter/flutter#151400) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages ...

…ved #151081) (flutter/flutter#151294)

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jul 4, 2024

Vi-debug mentioned this pull request Jul 4, 2024

[material/menu_anchor.dart] Submenu anchor misaligned with child panel #151081

Closed

Vi-debug force-pushed the 151081-fix-submenu-anchor-misaligned-with-child-panel branch 2 times, most recently from 10ae588 to 5ac0182 Compare July 5, 2024 02:56

nate-thegrate reviewed Jul 7, 2024

View reviewed changes

Vi-debug force-pushed the 151081-fix-submenu-anchor-misaligned-with-child-panel branch from 9881208 to d9fc22e Compare July 7, 2024 05:29

nate-thegrate approved these changes Jul 7, 2024

View reviewed changes

Vi-debug force-pushed the 151081-fix-submenu-anchor-misaligned-with-child-panel branch from d9fc22e to c4b454d Compare July 8, 2024 01:41

bleroux reviewed Jul 8, 2024

View reviewed changes

packages/flutter/test/material/menu_anchor_test.dart Outdated Show resolved Hide resolved

bleroux requested a review from gspencergoog July 8, 2024 06:53

Vi-debug force-pushed the 151081-fix-submenu-anchor-misaligned-with-child-panel branch 2 times, most recently from 28c9a31 to 315d629 Compare July 8, 2024 09:12

Vi-debug added 6 commits July 10, 2024 09:11

Fix: Submenu anchor misaligned with child panel in web (Resolved flut…

8bc194c

…ter#151081)

Fix: Change upperLeft offset to calculate correctly on both web and m…

2762fea

…obile (Resolved flutter#151081)

Fix: Recalculate the upleft offset of the achorBox

356629b

Fix: Should be dx since dx already be recalculated

87cf79c

Fix: Update test case in menu anchor and menu style, rollback the spa…

b555f43

…ce change

Fix: Add comma

d1a7cd8

Vi-debug force-pushed the 151081-fix-submenu-anchor-misaligned-with-child-panel branch from 315d629 to d1a7cd8 Compare July 10, 2024 02:11

Piinks requested a review from dkwingsmt July 11, 2024 18:12

Piinks mentioned this pull request Jul 11, 2024

Fix: Vertical offset between submenu item and menu item button #151365

Closed

9 tasks

gspencergoog approved these changes Jul 11, 2024

View reviewed changes

gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 11, 2024

auto-submit bot merged commit bc3d2fe into flutter:master Jul 11, 2024

engine-flutter-autoroll mentioned this pull request Jul 12, 2024

Roll Flutter from 5103d7574361 to bf6c455d4a7a (41 revisions) flutter/packages#7109

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 12, 2024

bc3d2fe Fix: Submenu anchor misaligned with child panel in web (Resol…

2e64049

…ved #151081) (flutter/flutter#151294)

engine-flutter-autoroll mentioned this pull request Jul 12, 2024

Roll Flutter from 5103d7574361 to 58068d8ea4ec (42 revisions) flutter/packages#7110

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 12, 2024

bc3d2fe Fix: Submenu anchor misaligned with child panel in web (Resol…

dd6f6af

…ved #151081) (flutter/flutter#151294)

engine-flutter-autoroll mentioned this pull request Jul 12, 2024

Roll Flutter from 5103d7574361 to e47c83729075 (43 revisions) flutter/packages#7111

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 12, 2024

bc3d2fe Fix: Submenu anchor misaligned with child panel in web (Resol…

920a904

…ved #151081) (flutter/flutter#151294)

engine-flutter-autoroll mentioned this pull request Jul 12, 2024

Roll Flutter from 5103d7574361 to 68f375fe3871 (44 revisions) flutter/packages#7112

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 12, 2024

bc3d2fe Fix: Submenu anchor misaligned with child panel in web (Resol…

2a67d9e

…ved #151081) (flutter/flutter#151294)

engine-flutter-autoroll mentioned this pull request Jul 12, 2024

Roll Flutter from 5103d7574361 to 607190448b1c (45 revisions) flutter/packages#7113

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 12, 2024

bc3d2fe Fix: Submenu anchor misaligned with child panel in web (Resol…

5cbe0e8

…ved #151081) (flutter/flutter#151294)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix: Submenu anchor misaligned with child panel in web (Resolved #151081) #151294

Fix: Submenu anchor misaligned with child panel in web (Resolved #151081) #151294

Uh oh!

Vi-debug commented Jul 4, 2024 •

edited

Loading

Uh oh!

flutter-dashboard bot commented Jul 4, 2024

Uh oh!

google-cla bot commented Jul 4, 2024

Uh oh!

nate-thegrate left a comment •

edited

Loading

Uh oh!

Uh oh!

nate-thegrate Jul 7, 2024

Uh oh!

Vi-debug Jul 7, 2024

Uh oh!

nate-thegrate left a comment

Uh oh!

Uh oh!

gspencergoog left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fix: Submenu anchor misaligned with child panel in web (Resolved #151081) #151294

Fix: Submenu anchor misaligned with child panel in web (Resolved #151081) #151294

Uh oh!

Conversation

Vi-debug commented Jul 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Jul 4, 2024

Uh oh!

google-cla bot commented Jul 4, 2024

Uh oh!

nate-thegrate left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nate-thegrate Jul 7, 2024

Choose a reason for hiding this comment

Uh oh!

Vi-debug Jul 7, 2024

Choose a reason for hiding this comment

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gspencergoog left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Vi-debug commented Jul 4, 2024 •

edited

Loading

nate-thegrate left a comment •

edited

Loading