eui icon indicating copy to clipboard operation
eui copied to clipboard

[EuiMarkdownFormat] : Allow links to open in a new tab!

Open adhishthite opened this issue 1 year ago • 9 comments

Is your feature request related to a problem? Please describe. I'm always frustrated when links in the EuiMarkdownFormat open in the same page.

Describe the solution you'd like We should at least have an option that will allow links to open in a new tab without referrers etc, so that the context and flow is not lost.

Describe alternatives you've considered Tried using custom props, but did not work.

Desired timeline This seems like a minor change. Should happen quick.

Additional context Thanks!

adhishthite avatar Jul 26 '24 12:07 adhishthite

@adhishthite Did you have a specific use case from Kibana in mind?

JasonStoltz avatar Jul 29 '24 21:07 JasonStoltz

Are you looking for a mechanism to selectively pick which links open in a new tab, or a configuration to open all links in a new tab?

JasonStoltz avatar Jul 29 '24 21:07 JasonStoltz

Configuration for all links to open in a new tab! Selectively choosing links would be tough. But EuiMarkdownFormat can have a openLinksInNewTab boolean prop. I do not have a specific use case in mind right now but I am working on an internal app and I am finding that opening links in the same tab is an undesirable behavior since the user loses context!On 30 Jul 2024, at 2:45 AM, Jason Stoltzfus @.***> wrote: Are you looking for a mechanism to selectively pick which links open in a new tab, or a configuration to open all links in a new tab?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

adhishthite avatar Jul 30 '24 01:07 adhishthite

Hey @adhishthite I had the same requirement and used the MarkdownPlugin capability to add a quick bit of code to open links in a new tab if they were external to the current app.

I am sure it isn't perfect and a built-in prop would be useful, but hopefully this will get you by for the time being.

import { EuiLink, getDefaultEuiMarkdownProcessingPlugins } from "@elastic/eui";

// eslint-disable-next-line no-restricted-globals
const isExternalURL = (url) => new URL(url).origin !== location.origin;

const markdownProcessingPlugins = () => {
  const defaultPlugins = getDefaultEuiMarkdownProcessingPlugins();
  defaultPlugins[1][1].components.a = (props) => {
    const isExternal = isExternalURL(props.href);
    return (
      <EuiLink target={isExternal ? "_blank" : ""} href={props.href}>
        {props.children}
      </EuiLink>
    );
  };
  return defaultPlugins;
};

export default markdownProcessingPlugins;

zebde avatar Jul 31 '24 08:07 zebde

Thanks @zebde ! I will definitely look into this. Appreciate the help.

adhishthite avatar Jul 31 '24 18:07 adhishthite

Thanks for the suggestion @zebde! I'm very glad that the extensibility of EuiMarkdownEditor is working as intended to allow consumers to expand on its functionality. One quick caution, I know that links in new tabs can be considered an accessibility issue, which is the only reason we might hesitate to incorporate this into EUI directly or allow it as a configuration.

@1Copenut or @alexwizp, any chance you can jump in here with a recommendation as our resident accessibility experts?

cee-chen avatar Aug 05 '24 16:08 cee-chen

@adhishthite We discussed this synchronously as a team today. In this case, we think there is value in making this change -- it would be a small change and has the potential to enhance to the user experience.

From a priority and planning perspective -- we aren't going to prioritize this as we don't have a concrete user facing need at this time and there is a workaround available.

However, I am going to add a "help wanted" label and we would gladly accept a PR for this one.

If you do end up adding a workaround in code, definitely consider adding a reference to this issue (https://github.com/elastic/eui/issues/7919) so we can circle back to it if this enhancement is made.

JasonStoltz avatar Aug 19 '24 17:08 JasonStoltz

👋 Thank you for your suggestion or request! While the EUI team agrees that it's valid, it's unlikely that we will prioritize this issue on our roadmap. We'll leave the issue open if you or anyone else in the community wants to implement it by contributing to EUI. If not, this issue will auto close in one year.

github-actions[bot] avatar Aug 19 '24 17:08 github-actions[bot]

PR: https://github.com/elastic/eui/pull/7985 Solves the Issue

hsk11 avatar Aug 27 '24 18:08 hsk11

The API to use to accomplish rendering tabs with target="_blank" will be:

const { processingPlugins } = getDefaultEuiMarkdownPlugins({
  processingConfig: {
    linkProps: { target: '_blank' }, // Accepts any prop that EuiLink takes - for example, `color: 'danger'` or `disabled: true`
  },
});

<EuiMarkdownFormat processingPluginList={processingPlugins}>
  [Opens in new tab](https://elastic.co)
</EuiMarkdownFormat>

<EuiMarkdownEditor processingPluginList={processingPlugins} />

See https://eui.elastic.co/pr_7985/#/editors-syntax/markdown-plugins#configuring-the-default-plugins for a full list of default plugin configurations that will be supported next release.

cee-chen avatar Aug 29 '24 02:08 cee-chen

Great work @hsk11 ! Thanks

adhishthite avatar Aug 29 '24 03:08 adhishthite