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.

[ryankeairns]

See also https://github.com/elastic/kibana/pull/216194

[JasonStoltz]

@zinckiwi A couple of additional approaches to consider:

• https://github.com/elastic/kibana/pull/216194 - Proposes a global link handler -- this would effectively handle the scenario you mentioned above at a Global level, outside of EUI, if you were to create something similar in Cloud UI
• Rather than an alwaysUseOnClick function exposed on EuiLink, we could expose a hook for a global handler for all EUI links defined at the top-level. This is described as a "Contextual Link Handler" in the Kibana draft PR mentioned above Imagine something like:

<EuiProvider 
  globalLinkHandler={() => 
    // You would implement your guard against modifier keys before invoking the supplied prop callback here, which would globally apply to all EUI links
  }
/>

[zinckiwi]

Thanks for the follow up Jason. If I'm reading the first bullet's proposal correctly, it sounds like it might be difficult to customise the behaviour conditionally. I assume we'd be able to pass in some context (minimally, the props the upstream Link received) to globalLinkHandler, which makes that seem like a nice compromise.

[JasonStoltz]

@tkajtoch Does this sound viable to you?

We would provide a globalLinkHandler prop (or something named better), and they would provide a link handler like follows, configured at the root EuiProvider:

const guardedClickHandler = (event) => {
    if (event.defaultPrevented) return;
    if (isModifiedEvent(event) || !isLeftClickEvent(event)) return;
    event.preventDefault();
    const url = event.currentTarget.getAttribute("href");
    navigate(url);
  };

<EuiProvider globalLinkHandler={guardedClickHandler} />

[JasonStoltz]

Also, FYI @clintandrewhall, to get your take, since you had mentioned this in https://github.com/elastic/kibana/pull/216194.