Tools: 💭 How to handle libraries that implement onClick and onKeyPress for us

[claytron5000]

Specifically, I'm using the Material UI Button component, https://material-ui.com/api/button/.

This component handles key events for us, and uses the onClick handler as a fallback: https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/ButtonBase/ButtonBase.js (line 212).

I don't want to have to re-implement the logic in handleKeyDown, and I also don't want to add suppressions to every button implementation.

One possible solution in the user space is to create my own Button implementation, add the suppression in one place, then try and convince the other dev's to use that implementation. That sounds like a people problem. This does not fix the issue because Rome still looks at the onClick and tells us to pair it. A very hacky solution would be to rename that property on my implementation (clickerOony), and then intercept that into onClick.

pseudo code verison (very icky)

interface MyProps {
  clickerOony: Function
}

export function MyButton(props: ButtonProps | MyProps) {
    // maybe some handling if we pass key handler and onClick instead...
    // rome-ignore lint/jsx/noPropSpreading
    return <MUIButton onClick={onClick}{...props}></MUIButton>
}

If it's not clear, what I'm asking is how can I avoid the above?

[ematipico]

Hi @claytron5000 , could you share an example of the code that Rome doesn't like please?

I am not sure what the real issue is. If it the {...props} or something else.

About your code, maybe, a solution would be:

export function MyButton(props: ButtonProps | MyProps) {
    const { onClick, ...propsWithoutClick } = props;
    return <MUIButton onClick={onClick} {...propsWithoutClick}></MUIButton>
}

[sebmck]

Should we restrict some of the a11y rules to only DOM elements?

[ematipico]

I think so

[claytron5000]

@ematipico The issue is the missing keyhandler. I confused the issue with my example code.

This is venturing into secondary bug territory, but I'm going to show some of the issues here:

Sometimes it doesn't see the rome-ignore

 assets/js/react-app/pages/StyleGuide.tsx:413:5 lint/jsx-a11y/useKeyWithClickEvents ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ Pair the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.

    411 │         </h2>
    412 │         <FloatingCard>
  > 413 │           <ProfileButton text="M"
        │           ^^^^^^^^^^^^^^^^^^^^^^^
  > 414 │           //rome-ignore lint/jsx-a11y/useKeyWithClickEvents
  > 415 │           onClick={() => {}} />

And sometimes it thinks the suppression is unused

assets/js/react-app/pages/StyleGuide.tsx:111:6 suppressions/unused ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ Unused suppression. Did not hide any errors.

    109 │             <Button variant="outlined"
    110 │             color="secondary"
  > 111 │             // rome-ignore lint/jsx-a11y/useKeyWithClickEvents
        │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    112 │             onClick={() => {}}>

Let me know if I should create separate tickets for that issue, though restricting a11y rules to DOM elements would fix the entire issue for me.

thank you.

[ematipico]

Thank you @claytron5000 !

They are valid bugs, we should limit the rule only to DOM elements.