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

feat: menuOpenEvent, menuSelectEvent, and location for context menu items #8891

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 2 commits into from
Apr 14, 2025
Merged

feat: menuOpenEvent, menuSelectEvent, and location for context menu items #8891

merged 2 commits into from
Apr 14, 2025

Conversation

maribethb
Copy link
Contributor

This PR was already reviewed as #8877 and #8890. This PR just merges the action-menu branch into rc/v12.0.0.

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
feat: support menuOpenEvent, menuSelectEvent, location for context me…
d1dc38f 
…nu items (#8877)

* feat: support menuOpenEvent, menuSelectEvent, location for context menu items

* feat: show context menu based on location

* fix: rtl
@maribethb
fix: update dependencies so adv compilation works (#8890)
e0009e2
@maribethb maribethb requested a review from a team as a code owner April 14, 2025 19:18
@maribethb maribethb requested a review from gonfunko April 14, 2025 19:18
@maribethb maribethb added the PR: feature Adds a feature label Apr 14, 2025
@maribethb maribethb mentioned this pull request Apr 14, 2025
Merged
@rachel-fenichel rachel-fenichel removed the request for review from gonfunko April 14, 2025 19:34
@maribethb maribethb merged commit 547fed4 into rc/v12.0.0 Apr 14, 2025
11 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rachel-fenichel rachel-fenichel rachel-fenichel approved these changes

Assignees

@rachel-fenichel rachel-fenichel

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

Successfully merging this pull request may close these issues.
3 participants
@maribethb @rachel-fenichel @gonfunko
0