Short-circuit EuiLink#onClick if modifier key is pressed

[zinckiwi]

Is your feature request related to a problem? Please describe.

Cloud UI had a bug report/feature request around the handling of links. In short, when Ctrl/Cmd/Shift-clicking to open an EuiLink in a new window, it doesn't work. This is due to our usage of both href and onClick in EuiLink:

• onClick is the main handler so that internal navigation within the app can be handled programmatically (history.push) for optimal UX
• href is still provided to allow browser native functionality for right-click context menus (e.g. "Open in new tab")

We are not, however, consulting the state of any modification keys in the onClick handler, so attempting to use a keyboard shortcut doesn't have the intended effect in-browser.

Describe the solution you'd like

I propose that EuiLink#onClick first guard against modifier keys before invoking the supplied prop callback. If a modifier key is detected, assume that the user wants to open the link in a new destination and therefore programmatic onClick behaviour is not necessary. If there are foreseeable edge cases, we could also consider an additional prop for fine-grained control (e.g. alwaysUseOnClick?: true | undefined), but I'm having a hard time coming up with any.

Describe alternatives you've considered

The most obvious alternative is to bake this functionality into a wrapper component within Cloud UI, which performs the guard but otherwise passes through to EuiLink. However, I think this is a fundamental enough concern that it is naturally coupled to the component that handles the click and makes the distinction between programmatic (onClick) and native (href) navigation -- EuiLink.

Desired timeline

Open

Additional context

I suspect when I say EuiLink I also mean EuiButton-as-supplied-with-an-href.

[JasonStoltz]

Hey @zinckiwi,

Just wrangling some context and thoughts here.

Since this is mostly related to SPA routing, for reference, this is our current recommendation: https://github.com/elastic/eui/blob/main/wiki/consuming-eui/react-router.md#1-conversion-function-recommended

I've see this implemented in a bunch of different ways in Kibana:

https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-router-utils/src/get_router_link_props/index.ts#L38
https://github.com/elastic/kibana/blob/46c8c17728979ff3eebcf69a17f79e97710a547c/x-pack/solutions/search/plugins/enterprise_search/public/applications/shared/react_router_helpers/generate_react_router_props.ts#L31

• This one includes a nice wrapper:
  https://github.com/elastic/kibana/blob/46c8c17728979ff3eebcf69a17f79e97710a547c/x-pack/solutions/search/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_components.tsx#L38

https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/cloud/connection_details/components/spa_no_router_link/spa_no_router_link.tsx#L24
https://github.com/elastic/kibana/blob/main/x-pack/platform/plugins/shared/osquery/public/agent_policies/agents_policy_link.tsx#L40

It's definitely messy and fairly inconsistent. Something like you propose seems helpful and I assume is generally useful regardless of what library you're using for SPA routing (EDIT: Unless your router already provides utilities for this, like useLinkClickHandler in React Router 6+)

Actually, at a glance, I think the larger problem Kibana has at this point is that many folks simply don't provide an href along with their onClick for SPA links, so we end up with a lot of buttons where we want anchors.

I propose that EuiLink#onClick first guard against modifier keys before invoking the supplied prop callback. If a modifier key is detected, assume that the user wants to open the link in a new destination and therefore programmatic onClick behaviour is not necessary. If there are foreseeable edge cases, we could also consider an additional prop for fine-grained control (e.g. alwaysUseOnClick?: true | undefined), but I'm having a hard time coming up with any.

I do think we would need to consider edge cases here. I suspect there are times where folks will want to trigger side effects with onClick. Gathering analytics is a case that is top of mind for more. For example, as seen here.

[JasonStoltz]

Just thinking that if we added something like alwaysUseOnClick, we probably wouldn't want to make it the default generally in the library, but we could leverage our ComponentDefaults service to enable it globally specifically for the Elastic products where we need it.