Skip to content

Disable context menu #128365

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.

Sign up for GitHub

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

Jump to bottom
Merged
auto-submit merged 3 commits into from
Jun 7, 2023
Merged

Disable context menu #128365

auto-submit merged 3 commits into from
Jun 7, 2023

Conversation

@gspencergoog
Copy link
Contributor

@gspencergoog gspencergoog commented Jun 6, 2023

Description

Changes the context menu example for MenuAnchor so that it uses right-click, or (on macOS and iOS only) ctrl-left-click, for the context menu. Also disables the browser context menu on web platforms.

Tests

  • Updated test to reflect new triggers.

@gspencergoog gspencergoog requested a review from HansMuller June 6, 2023 23:54
@flutter-dashboard flutter-dashboard bot added a: tests "flutter test", flutter_test, or one of our tests a: text input Entering text in a text field or keyboard related problems d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos documentation engine flutter/engine related. See also e: labels. f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Jun 6, 2023
@gspencergoog gspencergoog force-pushed the disable_context_menu branch from 5198da1 to ab24799 Compare June 6, 2023 23:55
@github-actions github-actions bot removed framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. tool Affects the "flutter" command-line tool. See also t: labels. a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. engine flutter/engine related. See also e: labels. a: tests "flutter test", flutter_test, or one of our tests labels Jun 6, 2023
@gspencergoog gspencergoog marked this pull request as ready for review June 6, 2023 23:56
@HansMuller
Copy link
Contributor

HansMuller commented Jun 7, 2023

This is great, makes the demo work just like you'd expect. Should MenuAnchor take care of disabling web context menus (perhaps optionally) itself?

@gspencergoog
Copy link
Contributor Author

gspencergoog commented Jun 7, 2023

Should MenuAnchor take care of disabling web context menus (perhaps optionally) itself?

No, because we can't know what they will bind to opening the menu. It doesn't make sense to disable it unless it's using the same key/button binding as the browser menu.

@HansMuller
Copy link
Contributor

HansMuller commented Jun 7, 2023

Good point. Still, using the same event as the browser to trigger opening the menu is likely to be a common case, so we're making writing portable code a bit more tedious. What if MenuAnchor set up itself up as a right-button context menu - with the platform-specific triggering event tweaks from your example - and disabled the browser menu by default. And we allowed developers to easily defeat the default behavior?

@gspencergoog
Copy link
Contributor Author

gspencergoog commented Jun 7, 2023

We could make a constructor for MenuAnchor that did that for you, perhaps, but MenuAnchor is a general purpose menu class, not just for context menus. Could be that you just want it to open a menu on a dropdown, and you don't want it disabling the browser context menu in that case.

I think including it in an example does at least help surface it, although it would be better in the docs, and maybe a special constructor for context menus.

HansMuller
HansMuller approved these changes Jun 7, 2023
View reviewed changes
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

LGTM

@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 7, 2023
@auto-submit auto-submit bot merged commit 36f73cf into flutter:master Jun 7, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 8, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 8, 2023
@engine-flutter-autoroll
36f73cf Disable context menu (flutter/flutter#128365)
5e8a9b0
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 8, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 8, 2023
@engine-flutter-autoroll
36f73cf Disable context menu (flutter/flutter#128365)
0eede1b
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jun 8, 2023
@engine-flutter-autoroll
Roll Flutter from 8a5c22e to 6e254a3 (12 revisions) (#4165)
4515495 
flutter/flutter@8a5c22e...6e254a3

2023-06-08 [email protected] [labeler] Set sync labels to false to stop removing labels (flutter/flutter#128446)
2023-06-08 [email protected] Update Chrome version for testing (flutter/flutter#128447)
2023-06-08 [email protected] Revert "Redo make inspector weakly referencing the inspected objects." (flutter/flutter#128506)
2023-06-08 [email protected] Use `--target-os` for appropriate precompiled targets. (flutter/flutter#127567)
2023-06-08 [email protected] Redo make inspector weakly referencing the inspected objects. (flutter/flutter#128471)
2023-06-07 [email protected] Roll Flutter Engine from 1089ce6874cf to a5f7d5d75ff2 (11 revisions) (flutter/flutter#128473)
2023-06-07 [email protected] Disable context menu (flutter/flutter#128365)
2023-06-07 [email protected] Adds vmservices to retrieve android applink settings (flutter/flutter#125998)
2023-06-07 [email protected] Roll Flutter Engine from 4f4486b00be2 to 1089ce6874cf (20 revisions) (flutter/flutter#128460)
2023-06-07 [email protected] Fix typos 'wether' -> 'whether' (flutter/flutter#128392)
2023-06-07 [email protected] Roll engine, patch expression evaluation (flutter/flutter#128255)
2023-06-07 [email protected] Roll Packages from da72219 to a84b2c2 (1 revision) (flutter/flutter#128444)

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
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@HansMuller HansMuller mentioned this pull request Jun 9, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
@engine-flutter-autoroll
36f73cf Disable context menu (flutter/flutter#128365)
a8eb56a
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@engine-flutter-autoroll
36f73cf Disable context menu (flutter/flutter#128365)
79f1aa1
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@engine-flutter-autoroll
36f73cf Disable context menu (flutter/flutter#128365)
3ef72b3
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
@engine-flutter-autoroll
36f73cf Disable context menu (flutter/flutter#128365)
f5a8e50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@HansMuller HansMuller HansMuller approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App c: contributor-productivity Team-specific productivity, code health, technical debt. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gspencergoog @HansMuller