-
Notifications
You must be signed in to change notification settings - Fork 653
feat(rome_js_analyzer): rule useKeyWithClickEvents
#3426
feat(rome_js_analyzer): rule useKeyWithClickEvents
#3426
Conversation
…m/jk-gan/rome-tools into feature/use-key-with-click-events
✅ Deploy Preview for docs-rometools ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
useKeyWithClickEvents
I've fixed the codes based on the suggestions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, I left some suggestion
use rome_rowan::{declare_node_union, AstNode}; | ||
|
||
declare_rule! { | ||
/// Pair the `onClick` mouse event with the `onKeyUp`, the `onKeyDown`, or the `noKeyPress` keyboard event. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Pair the `onClick` mouse event with the `onKeyUp`, the `onKeyDown`, or the `noKeyPress` keyboard event. | |
/// Enforce to have the`onClick` mouse event with the `onKeyUp`, the `onKeyDown`, or the `noKeyPress` keyboard event. |
fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
let node = ctx.query(); | ||
|
||
let required_props = ["onKeyDown", "onKeyUp", "onKeyPress"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be a const
of UseKeyWithClickEvents
:
impl UseKeyWithClickEvents {
const REQUIRED_PROPS: [&str; 3] = ["onKeyDown", "onKeyUp", "onKeyPress"];
}
// ..
if Self::REQUIRED_PROPS.contains(&name.as_str()) {
return None;
}
Like this, we don't need to create this array every time we run the rule.
It seems that the rule triggered in our code base :) That's good!
Would you mind to fix it? |
I've updated the codes based on the suggestions |
Summary
Closes #3301 (Reopen the PR because #3372 is not able to detect the correct file change after I rebased with main branch)
Test Plan
Added various test cases