Skip to content

fix(google-maps): update right-click event to use contextmenu for Adv… #30523

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SippieCup
Copy link

'rightclick' never triggers on advanced marker elements. nor within on gmp-click, as right-click events have been disabled for some reason on advanced markers within JS Maps.

This assumes that contextmenu is being opened via right clicking of a mouse, which is 99% of use cases. We can rename the emitter as well if deemed necessary to truly fit context, but I wanted to maintain backwards compatibility with and uniformity between the two.

@SippieCup SippieCup requested a review from a team as a code owner February 19, 2025 19:56
@SippieCup SippieCup requested review from mmalerba and wagnermaciel and removed request for a team February 19, 2025 19:56
@mmalerba mmalerba requested a review from crisbeto February 19, 2025 20:18
@SippieCup
Copy link
Author

SippieCup commented Feb 22, 2025

Thinking about this more, I believe google maps natively block the default context menu within the component, am I correct in that belief? Or should I be adding event.preventDefault() to any listener function to be safe?

I don't understand google-maps & map-advanced-marker interactions enough to make the change within map-advanced-marker itself without some feedback.

@mmalerba
Copy link
Contributor

I am not sure about that. We have a page to test it in our dev app (pnpm dev-app) you should be able to set your google maps key in an environment variable called GOOGLE_MAPS_KEY and run that. It would be good to add some manual test of this feature to that dev-app as part of this PR

Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

updating review status to reflect request above

@SippieCup SippieCup force-pushed the fix-right-click-advanced-markers branch from cee6bce to 3c2a592 Compare August 13, 2025 03:25
@Copilot Copilot AI review requested due to automatic review settings August 13, 2025 03:25
@SippieCup SippieCup force-pushed the fix-right-click-advanced-markers branch from 3c2a592 to 6186b1a Compare August 13, 2025 04:42
By rewriting the prototype of the advancedmarker and inject functionality for right clicking there.We have a cleaner and more native event management within the component and fix issues surrounding different ways contextmenu is supported across browsers.
@SippieCup SippieCup force-pushed the fix-right-click-advanced-markers branch from 6186b1a to f7cdfc6 Compare August 13, 2025 04:43
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants