Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Menu buttons MouseRegion callbacks misfired on Android #155023

Closed

Conversation

TahaTesser
Copy link
Member

@TahaTesser TahaTesser commented Sep 11, 2024

Fixes SubmenuButton submenus close automatically only on first activation [Android]

Code sample

expand to view the code sample
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      home: Scaffold(
        body: Center(
          child: MenuAnchor(
            builder: (BuildContext context, MenuController controller, Widget? child) => TextButton(
              onPressed: () {
                controller.isOpen ? controller.close() : controller.open();
              },
              child: const Text('Press here'),
            ),
            menuChildren: <Widget>[
              SubmenuButton(

                menuChildren: <Widget>[
                  MenuItemButton(
                    onPressed: () {},
                    child: const Text('Menu item'),
                  ),
                ],
                child: const Text('Submenu'),
              ),
            ],
          ),
        ),
      ),
    );
  }
}

Before

VID_20240911180435.mp4

After

VID_20240911180401.mp4

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@TahaTesser TahaTesser marked this pull request as draft September 11, 2024 15:08
@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Sep 11, 2024
@TahaTesser TahaTesser marked this pull request as ready for review September 12, 2024 16:45
@TahaTesser
Copy link
Member Author

@gmackall when your engine fix lands I can remove my framework fix and just land the updated framework regression test. Otherwise, this PR is ready to land without a engine to address #154842 right now.

@gmackall
Copy link
Member

I think this looks reasonable to land in the mean time - will want to make sure we revert when flutter/engine#55157 lands

@TahaTesser
Copy link
Member Author

Since flutter/engine#55157 is already up. I'll just wait with a goal is to land the test, not the framework fix.

Copy link
Contributor

@reidbaker reidbaker left a comment

Choose a reason for hiding this comment

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

Regression test approved. Lets land the engine fix instead of the framework fix.


// Check that the menu item is open.
expect(find.text('Menu item'), findsOneWidget);
}, variant: TargetPlatformVariant.only(TargetPlatform.android));
Copy link
Contributor

Choose a reason for hiding this comment

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

The regression was on android but I dont think this is restricted to a capability of android. Consider running this on all target platforms to prevent regressions there as well.

@TahaTesser
Copy link
Member Author

After the engine fix, this framework test became irrelevant as the test can no longer simulate the framework receiving a wrong flow of events. Closing.

@TahaTesser TahaTesser closed this Sep 23, 2024
@TahaTesser TahaTesser deleted the fix_menu_buttons_android branch September 23, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SubmenuButton submenus close automatically only on first activation [Android]
3 participants