Allow href and onClick in ListGroupItem

[joshdover]

Summary

Having an href and an onClick handler is a perfectly valid usecase that EUI should support. There are instances where you may want to override the default behavior with event.preventDefault() in an onClick handler in some conditions, but allow the default behavior in others.

Checklist

• Documentation examples were added
• A changelog entry exists and is marked appropriately
• This was checked for breaking changes and labeled appropriately
• Jest tests were updated or added to match the most common scenarios

[cchaos]

@joshdover As you might know this has been quite the controversial topic between EUI and Kibana. However, we've usually resolved that the use-case doesn't or shouldn't actually need this ability. Browser handling of href vs onClick can be unstable, though it is getting better.

Our main concern usually comes back to accessibility. <button>s should behave as buttons (micro-interactions) and <a>s should behave as links (change the page url). Usually if you're forcing the need to do both, it means you're bypassing the natural browser history and routing.

If it's conditional routing/handling of the click that you need, then you should be able to perform that condition before rendering the item and therefore supplying the correct onClick or href.

However, we also don't know your particular use case, so feel free to explain.

---

Here's a good button vs link checklist that I eventually intend to add to our guidelines:

• If a screen reader user tabbed onto an interactive element, would its role tell them what to expect? (Would it navigate away from the page? They'd want to know.)
• Suppressing link features like URL changes or right click? Consider a button.
• Encourage routing in your application with href, react-router, etc.
• Page navigation deserves title changes and history.

[joshdover]

Got it, thanks for the explanation. I think I've found a way to work around this.

[joshdover]

@cchaos I'm reopening this for discussion.

One of the suggestions above is "Encourage routing in your application with href, react-router, etc". The way react-router actually handles this is by using both an href and an onClick handler that calls event.preventDefault in some cases.

This allows both "Open in new tab" behavior via right-click menu or CMD+click as well as allowing the SPA-style navigation history.push calls.

I cannot find a way to replicate this behavior without having both href and an onClick handler. Do you have any suggestions?

[snide]

@joshdover Ultimately I think we're throwing our hands up trying to enforce this. To get this PR good to go, can you document some best practices. Specifically just something in the prop docs to say we should attempt not to use onclick and href together. We just need something defensible when we do see this being used over aggressively.

[joshdover]

@cchaos this is ready for review.

I also converted this to TypeScript while I was here. This was to help with making sure the comments I added show up in IntelliSense in VSCode. I haven't done this conversion in EUI before, so let me know if there's any steps I missed.

[cchaos]

LGTM, but I'd like @thompsongl to double-check the TS conversions. The docs still behave the same way. Just found a couple typos

[thompsongl]

TS conversion looks good to me

[thompsongl]

@joshdover I think this is good to go once the conflict is resolved