Conversation
| {React.isValidElement(title) ? ( | ||
| title | ||
| ) : ( | ||
| <button onClick={onClick}> |
There was a problem hiding this comment.
There were existing rules for list-item__title regarding text truncation, but being the direct child of a button tag was blocking them from taking effect. This button element and associated onClick value are actually redundant, as the list item wrapper handles the onClick event for all of its contents:
There was a problem hiding this comment.
Huh, interesting find!
The use of button here was important though - it allowed keyboard navigation. Though... now that I'm testing it... it seems to have made it focusable, but pressing "Enter" did nothing 😞.
Well, given that this already wasn't keyboard-friendly, this doesn't need to block here. But that's something we'll want to do. I'd suggest changing the div with the onClick set to a button.
There was a problem hiding this comment.
Suggestion makes sense here 👍 Updated the outer container to be of type button:
Builds ready [187c782]
Page Load Metrics (794 ± 170 ms)
|
Gudahtt
left a comment
There was a problem hiding this comment.
LGTM!
As noted in a comment, this does impact accessibility, but this list already had issues there.
|
Awesome work @ryanml ! |
Builds ready [a2b419e]
Page Load Metrics (621 ± 38 ms)
|
Builds ready [73ea889]
Page Load Metrics (584 ± 27 ms)
|
Fixes #9997
Manual testing steps: