Skip to content

Conversation

@PhilippeCollin
Copy link
Contributor

@PhilippeCollin PhilippeCollin commented Mar 10, 2025

Description

Add an eslint rule to disallow contextmenu event listeners. Overriding the default right click menu is generally unexpected and results in worse accessibility.

At Shopify, we believe that this should almost never been done, except in some full-screen applications, like the Online Store Theme Editor, Figma, Google Docs, etc...

@PhilippeCollin PhilippeCollin force-pushed the pcollin/no_contextmenu branch 4 times, most recently from b3ecc9b to 1534a97 Compare March 11, 2025 13:36
Copy link

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

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

Code LGTM! Thanks for adding such extensive tests.

@BPScott
Copy link
Member

BPScott commented Mar 11, 2025

You can comment /snapit on this PR to create a snapshot package release that you can then test within some repo to make sure it is good and you're happy with it.

@PhilippeCollin
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @PhilippeCollin! Your snapshot has been published to npm.

Test the snapshot by updating your package.json with the newly published version:

"@shopify/eslint-plugin": "0.0.0-snapshot-20250311145226"

@PhilippeCollin PhilippeCollin force-pushed the pcollin/no_contextmenu branch from 1534a97 to 66fb520 Compare March 11, 2025 21:07
@PhilippeCollin
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @PhilippeCollin! Your snapshot has been published to npm.

Test the snapshot by updating your package.json with the newly published version:

"@shopify/eslint-plugin": "0.0.0-snapshot-20250311210802"

@PhilippeCollin
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @PhilippeCollin! Your snapshot has been published to npm.

Test the snapshot by updating your package.json with the newly published version:

"@shopify/eslint-plugin": "0.0.0-snapshot-20250311213039"

@PhilippeCollin
Copy link
Contributor Author

Thanks for the suggestion @BPScott, I added the new rule to the core config so that it's enabled by default and tested that it works as expected in a repo ✅

@mxstbr
Copy link

mxstbr commented Mar 12, 2025

Awesomeee. Let's shipit!

@PhilippeCollin PhilippeCollin merged commit dd68594 into main Mar 12, 2025
4 checks passed
@PhilippeCollin PhilippeCollin deleted the pcollin/no_contextmenu branch March 12, 2025 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants