Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_js_analyze): False positives for interactive elements in useKeyWithClickEvents #3644

Merged
merged 1 commit into from
Nov 10, 2022

Conversation

MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented Nov 10, 2022

Interactive elements like button, a, input etc. don't need key events as they are supported by the browser.

This is a simplified implementation and the rule should also consider if the element is hidden for screen readers but that's out of the scope of this fix. See #3640

Test Plan

I added a new test that verifies that button no longer gets flagged.

Versioning

This reduces the number of lint warnings and is thus, safe to land

@MichaReiser MichaReiser requested a review from a team November 10, 2022 10:09
@netlify
Copy link

netlify bot commented Nov 10, 2022

Deploy Preview for docs-rometools ready!

Name Link
🔨 Latest commit 7e8dfa2
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/636cde812eeb38000ca88c96
😎 Deploy Preview https://deploy-preview-3644--docs-rometools.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@MichaReiser MichaReiser added this to the 10.0.1 milestone Nov 10, 2022
@MichaReiser MichaReiser added the A-Linter Area: linter label Nov 10, 2022
@MichaReiser MichaReiser force-pushed the fix/useKeyWithClickEvents-interactive-elements branch from 4a579de to 73ba8eb Compare November 10, 2022 10:11
@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45879 45879 0
Passed 44939 44939 0
Failed 940 940 0
Panics 0 0 0
Coverage 97.95% 97.95% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 1621 1621 0
Failed 4325 4325 0
Panics 0 0 0
Coverage 27.26% 27.26% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12395 12395 0
Failed 3862 3862 0
Panics 0 0 0
Coverage 76.24% 76.24% 0.00%

@MichaReiser MichaReiser force-pushed the fix/useKeyWithClickEvents-interactive-elements branch from 73ba8eb to 7e8dfa2 Compare November 10, 2022 11:20
@MichaReiser MichaReiser merged commit a617093 into main Nov 10, 2022
@MichaReiser MichaReiser deleted the fix/useKeyWithClickEvents-interactive-elements branch November 10, 2022 12:08
jeysal added a commit to jeysal/rometools that referenced this pull request Nov 10, 2022
* upstream/main:
  fix(npm/js-api): Import type from @rometools/backend-jsonrpc (rome#3647)
  doc(npm/js-api): Add experimental warning to README
  fix(npm/js-api): Lazy load backend implementations (rome#3652)
  feat(rome_lsp): stop the daemon after the last client disconnects (rome#3643)
  fix(npm/js_api): Ensure JS API build contains `dist` folder (rome#3648)
  fix(rome_cli): ensures the service only connects to compatible versions (rome#3642)
  feat(playground): add settings button and group IR (rome#3559)
  release: v10.0.1 (rome#3649)
  fix(rome_js_analyze): False positives for interactive elements in `useKeyWithClickEvents` (rome#3644)
  fix(rome_js_semantic): support for object and array bindings when exporting (rome#3620)
  doc(editors): Clarify Rome discovery (rome#3639)
  fix(rome_js_analyze): improve the detection of `ReactDOM.render` calls in `noRenderReturnValue` (rome#3626)
  chore(npm): add license (rome#3629)
  Add rustdocs index
  Add environment
  Change rust docs to use Netlify
  fix(rome_js_analyze): useValidAnchor considering all possible expressions (rome#3599)
  fix(rome_js_formatter): Trailing comma inside import rome#3600 (rome#3624)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Linter Area: linter
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants