feat: add middle click support for workspace rows #9834
Conversation
| // Awkward type definition (the hover preview in VS Code isn't great, either), | ||
| // but this basically takes all click props from TableRowProps, but makes | ||
| // onClick required, and adds an optional onMiddleClick | ||
| type UseClickableTableRowConfig = { |
There was a problem hiding this comment.
The more I think about this, the more I think I should probably update this with this "less-clever"/"actually maintainable" type:
type UseClickableRowConfig = {
onClick: MouseEventHandler<HTMLTableRowElement>;
onAuxClick?: MouseEventHandler<HTMLTableRowElement>;
onDoubleClick?: MouseEventHandler<HTMLTableRowElement>;
onMiddleClick?: MouseEventHandler<HTMLTableRowElement>;
};You lose the explicit connection to the underlying TableRowProps type, so any changes to it might not immediately get flagged at the compiler level, but it's way more readable and gives you better information when you hover over the type in VS Code
There was a problem hiding this comment.
Good work, I think it is a good solution for now but I have a few thoughts:
- Wondering how accessible it is since it only supports middle click but would not work with right click > open in new tab
- I'm 0wondering if we should start a conversation to:
- Get away from tables so we could wrap the rows as an ""
- Make only the name column be clickable so we could achieve that easily... but not sure what other folks would think about that 🤔
Yeah, good point. I feel like that would require some changes to how the components are designed – namely, we would have to update the semantics and ARIA values to make the rows into links, rather than buttons. But I do also think the rows should probably change a little bit. I don't know what features we could add down the line that would warrant having different functionality for each cell, but at the very least, I feel like the checkbox column can be separated and can be set up as a big, implicit checkbox. So even if someone accidentally misses the visible checkbox, just clicking the area will still make the check go through. That would help the a11y a little bit |
Closes #9749
This seemed really easy at first – and getting the runtime logic updated was no problem – but it quickly turned into an ordeal because of all the TypeScript type parameters I had to juggle.
Changes
useClickablehook to expose a ref. This can be used in effects, but right now, the main point is to simulate a click for the element the props are attached touseClickableTableRowto supportonClick,onDoubleClickandonAuxClickcallbacks (onAuxClickwas needed for middle-clicks, so I figured I'd add support for everything)Out-of-scope changes for later
useClickableTableRowhook, but I tried to make it as easy as possible to add that functionality later, if they need it