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.

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

Disable context menu #128365

Merged
merged 3 commits into from Jun 7, 2023
Merged

Disable context menu #128365

merged 3 commits into from Jun 7, 2023

Conversation

gspencergoog
Copy link
Contributor

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.

@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 repository. 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. team Infra upgrades, team productivity, code health, technical debt. See also team: labels. tool Affects the "flutter" command-line tool. See also t: labels. labels Jun 6, 2023
@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 repository. 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

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

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

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

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
34 checks passed
@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
36f73cf64 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
36f73cf64 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 8a5c22e282db to 6e254a3f9fb7 (12 revisions) (#4165)
4515495 
flutter/flutter@8a5c22e...6e254a3

2023-06-08 chillers@google.com [labeler] Set sync labels to false to stop removing labels (flutter/flutter#128446)
2023-06-08 jacksongardner@google.com Update Chrome version for testing (flutter/flutter#128447)
2023-06-08 zanderso@users.noreply.github.com Revert "Redo make inspector weakly referencing the inspected objects." (flutter/flutter#128506)
2023-06-08 sstrickl@google.com Use `--target-os` for appropriate precompiled targets. (flutter/flutter#127567)
2023-06-08 polinach@google.com Redo make inspector weakly referencing the inspected objects. (flutter/flutter#128471)
2023-06-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1089ce6874cf to a5f7d5d75ff2 (11 revisions) (flutter/flutter#128473)
2023-06-07 gspencergoog@users.noreply.github.com Disable context menu (flutter/flutter#128365)
2023-06-07 47866232+chunhtai@users.noreply.github.com Adds vmservices to retrieve android applink settings (flutter/flutter#125998)
2023-06-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4f4486b00be2 to 1089ce6874cf (20 revisions) (flutter/flutter#128460)
2023-06-07 leroux_bruno@yahoo.fr Fix typos 'wether' -> 'whether' (flutter/flutter#128392)
2023-06-07 aam@google.com Roll engine, patch expression evaluation (flutter/flutter#128255)
2023-06-07 engine-flutter-autoroll@skia.org 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 rmistry@google.com,stuartmorgan@google.com 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
Open
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
@engine-flutter-autoroll
36f73cf64 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
36f73cf64 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
36f73cf64 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
36f73cf64 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

@HansMuller HansMuller HansMuller approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos team Infra upgrades, team productivity, code health, technical debt. See also team: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@gspencergoog @HansMuller