8000 feat: support menuOpenEvent, menuSelectEvent, location for context menu items by maribethb · Pull Request #8877 · google/blockly · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: support menuOpenEvent, menuSelectEvent, location for context menu items #8877

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
merged 3 commits into from
Apr 11, 2025
Merged

feat: support menuOpenEvent, menuSelectEvent, location for context menu items #8877

merged 3 commits into from
Apr 11, 2025

Conversation

maribethb
Copy link
Contributor

The basics

The details

Resolves

Fixes #8844 and fixes #8843

Proposed Changes

  • Updates many of the context menu-related functions to take Event and not just PointerEvent
  • Passes the events that opened a menu and selected an option down to the option callbacks
  • Calculates a location correctly for the existing uses of the context menu (on blocks, comments, and workspaces). The logic for the block location comes from the keyboard experiment (fakeEventForBlock) while for the comments it comes from the location of the comment. For workspace it is not currently implemented. It needs to be implemented based on the cursor location on the workspace, but since that's a bit in flux I just left it out for now.
  • Should be backwards compatible as in the contextmenu.show method, if no location is passed, I calculate one based off the PointerEvent that would have to be passed in if there wasn't a location.

Reason for Changes

  • It should be possible for the context menu to be opened via keyboard, so the opening event will not always be a PointerEvent.
  • Since the location of the menu if opened via keyboard event is determined by the thing that owns the menu, the menu itself can't calculate its location anymore. The owner of the menu is responsible for calculating its location, and the logic for doing so varies based on type of thing, as you can see from the logic here.
  • Some context menu events need to behave differently based on how the menu was opened.

Test Coverage

Manually tested by invoking showContextMenu on blocks, workspaces, and comments and made sure the menu opened even when not passed a pointer event, and that the location was expected. Also ensured the events were passed down to the callback correctly.

Documentation

Yes, but there's another issue for that.

Additional Information

@maribethb maribethb requested a review from a team as a code owner April 10, 2025 23:25
@maribethb maribethb requested a review from BenHenning April 10, 2025 23:25
@maribethb maribethb added the PR: feature Adds a feature label Apr 11, 2025
@maribethb maribethb mentioned this pull request Apr 11, 2025
Merged
@gonfunko gonfunko requested review from gonfunko and removed request for BenHenning April 11, 2025 15:53
@gonfunko gonfunko self-assigned this Apr 11, 2025
gonfunko
gonfunko approved these changes Apr 11, 2025
View reviewed changes
@maribethb
Copy link
Contributor Author

Something I did in this PR causes field_dropdown.ts to be compiled before field.ts for some reason. I have no idea why though or how closure decides in what order to do things. I don't see any circular dependencies here?

@maribethb maribethb changed the base branch from rc/v12.0.0 to action-menu April 11, 2025 22:08
@maribethb
Copy link
Contributor Author

merging this into a side branch so I can pause bashing at this test failure to work on subsequent issues

@maribethb maribethb merged commit d1dc38f into google:action-menu Apr 11, 2025
4 of 8 checks passed
This was referenced Apr 14, 2025
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gonfunko gonfunko gonfunko approved these changes

Assignees

@gonfunko gonfunko

Labels
PR: feature Adds a feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@maribethb @gonfunko
0