Skip to content
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

Sonar: Add a 'onKeyPress|onKeyDown|onKeyUp' attribute to this <span> tag #26107

Merged
merged 1 commit into from May 11, 2024

Conversation

qmonmert
Copy link
Contributor

Fix #26106


Please make sure the below checklist is followed for Pull Requests.

When you are still working on the PR, consider converting it to Draft (below reviewers) and adding skip-ci label, you can still see CI build result at your branch.

mraible
mraible previously approved these changes May 10, 2024
@mshima
Copy link
Member

mshima commented May 10, 2024

Won’t (keypress) conflict with tabs? Shouldn’t we use (keypress.Space) or (keypress.enter)?

@qmonmert qmonmert marked this pull request as draft May 10, 2024 20:04
@qmonmert
Copy link
Contributor Author

qmonmert commented May 10, 2024

@mshima to try I added tabindex="0" on the span and it works with (keydown) but I don't add tabindex because Sonar complains "tabIndex" should only be declared on interactive elements.sonarlint(Web:S6845)

@mshima
Copy link
Member

mshima commented May 10, 2024

Are the remaining 7 bugs false positives?
Maybe change span to button if not false positives?

@qmonmert qmonmert force-pushed the sonarrely branch 2 times, most recently from 4f8c2b2 to a56d274 Compare May 11, 2024 15:17
@qmonmert qmonmert marked this pull request as ready for review May 11, 2024 16:03
@qmonmert
Copy link
Contributor Author

@mshima not false positives for me, I updated with button

@qmonmert qmonmert requested a review from DanielFran May 11, 2024 19:18
@mshima mshima merged commit a6372ce into jhipster:main May 11, 2024
39 checks passed
@qmonmert
Copy link
Contributor Author

7 issues fixed
image

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.

Sonar: 49 reliability issues
4 participants