From 928a35d39ab077fec4a53a10ad6dc461716e120a Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Mon, 8 Jan 2024 10:55:43 -0700 Subject: [PATCH] [Dashboard Navigation] Add external link icon (#174407) Closes https://github.com/elastic/kibana/issues/165771 ## Summary This PR adds the `external` prop to links in the Link panel in the following cases: 1. A dashboard link has the "Open in new tab" setting set to `true` 2. All URLs, regardless of if the "Open in new tab" setting is set to `true` or `false` This `external` prop renders the popout icon beside the links, like so: ![linksExternalProp](https://github.com/elastic/kibana/assets/8698078/3bc1ea05-fffd-455a-a4e3-81456bb6b024) ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --- .../dashboard_link_component.test.tsx | 17 +++++++++++-- .../dashboard_link_component.tsx | 1 + .../external_link_component.test.tsx | 25 ++++++++++++++++--- .../external_link/external_link_component.tsx | 1 + 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/plugins/links/public/components/dashboard_link/dashboard_link_component.test.tsx b/src/plugins/links/public/components/dashboard_link/dashboard_link_component.test.tsx index a63636d40df26..4e6eb32319baf 100644 --- a/src/plugins/links/public/components/dashboard_link/dashboard_link_component.test.tsx +++ b/src/plugins/links/public/components/dashboard_link/dashboard_link_component.test.tsx @@ -11,7 +11,7 @@ import React from 'react'; import { getDashboardLocatorParamsFromEmbeddable } from '@kbn/dashboard-plugin/public'; import { DashboardContainer } from '@kbn/dashboard-plugin/public/dashboard_container'; import { DEFAULT_DASHBOARD_DRILLDOWN_OPTIONS } from '@kbn/presentation-util-plugin/public'; -import { createEvent, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { createEvent, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { LINKS_VERTICAL_LAYOUT } from '../../../common/content_management'; @@ -104,8 +104,15 @@ describe('Dashboard link component', () => { expect(fetchDashboard).toHaveBeenCalledWith(defaultLinkInfo.destination); await waitFor(() => expect(onRender).toHaveBeenCalledTimes(1)); + // renders dashboard title const link = await screen.findByTestId('dashboardLink--foo'); expect(link).toHaveTextContent('another dashboard'); + + // does not render external link icon + const externalIcon = within(link).queryByText('External link'); + expect(externalIcon).toBeNull(); + + // calls `navigate` on click userEvent.click(link); expect(dashboardContainer.locator?.getRedirectUrl).toBeCalledWith({ dashboardId: '456', @@ -134,7 +141,7 @@ describe('Dashboard link component', () => { expect(preventDefault).toHaveBeenCalledTimes(0); }); - test('openInNewTab uses window.open, not navigateToApp', async () => { + test('openInNewTab uses window.open, not navigateToApp, and renders external icon', async () => { const linkInfo = { ...defaultLinkInfo, options: { ...DEFAULT_DASHBOARD_DRILLDOWN_OPTIONS, openInNewTab: true }, @@ -155,6 +162,12 @@ describe('Dashboard link component', () => { await waitFor(() => expect(onRender).toHaveBeenCalledTimes(1)); const link = await screen.findByTestId('dashboardLink--foo'); expect(link).toBeInTheDocument(); + + // external link icon is rendered + const externalIcon = within(link).getByText('External link'); + expect(externalIcon?.getAttribute('data-euiicon-type')).toBe('popout'); + + // calls `window.open` userEvent.click(link); expect(dashboardContainer.locator?.navigate).toBeCalledTimes(0); expect(window.open).toHaveBeenCalledWith('https://my-kibana.com/dashboard/123', '_blank'); diff --git a/src/plugins/links/public/components/dashboard_link/dashboard_link_component.tsx b/src/plugins/links/public/components/dashboard_link/dashboard_link_component.tsx index 30977b593238a..a6ef1c7c4b72d 100644 --- a/src/plugins/links/public/components/dashboard_link/dashboard_link_component.tsx +++ b/src/plugins/links/public/components/dashboard_link/dashboard_link_component.tsx @@ -187,6 +187,7 @@ export const DashboardLinkComponent = ({ 'dashboardLinkError--noLabel': !link.label, })} label={linkLabel} + external={link.options?.openInNewTab} data-test-subj={error ? `${id}--error` : `${id}`} /> ); diff --git a/src/plugins/links/public/components/external_link/external_link_component.test.tsx b/src/plugins/links/public/components/external_link/external_link_component.test.tsx index 02f984435f8ba..e0eb0686144b1 100644 --- a/src/plugins/links/public/components/external_link/external_link_component.test.tsx +++ b/src/plugins/links/public/components/external_link/external_link_component.test.tsx @@ -9,7 +9,7 @@ import React from 'react'; import userEvent from '@testing-library/user-event'; -import { createEvent, fireEvent, render, screen } from '@testing-library/react'; +import { createEvent, fireEvent, render, screen, within } from '@testing-library/react'; import { LinksEmbeddable, LinksContext } from '../../embeddable/links_embeddable'; import { mockLinksPanel } from '../../../common/mocks'; import { LINKS_VERTICAL_LAYOUT } from '../../../common/content_management'; @@ -37,7 +37,7 @@ describe('external link component', () => { jest.clearAllMocks(); }); - test('by default opens in new tab', async () => { + test('by default opens in new tab and renders external icon', async () => { render( { expect(onRender).toBeCalledTimes(1); const link = await screen.findByTestId('externalLink--foo'); expect(link).toBeInTheDocument(); - await userEvent.click(link); + const externalIcon = within(link).getByText('External link'); + expect(externalIcon.getAttribute('data-euiicon-type')).toBe('popout'); + userEvent.click(link); expect(window.open).toHaveBeenCalledWith('https://example.com', '_blank'); }); + test('renders external icon even when `openInNewTab` setting is `false`', async () => { + const linkInfo = { + ...defaultLinkInfo, + options: { ...DEFAULT_URL_DRILLDOWN_OPTIONS, openInNewTab: false }, + }; + render( + + + + ); + const link = await screen.findByTestId('externalLink--foo'); + const externalIcon = within(link).getByText('External link'); + expect(externalIcon?.getAttribute('data-euiicon-type')).toBe('popout'); + }); + test('modified click does not trigger event.preventDefault', async () => { const linkInfo = { ...defaultLinkInfo, @@ -86,7 +103,7 @@ describe('external link component', () => { ); expect(onRender).toBeCalledTimes(1); const link = await screen.findByTestId('externalLink--foo'); - await userEvent.click(link); + userEvent.click(link); expect(coreServices.application.navigateToUrl).toBeCalledTimes(1); expect(coreServices.application.navigateToUrl).toBeCalledWith('https://example.com'); }); diff --git a/src/plugins/links/public/components/external_link/external_link_component.tsx b/src/plugins/links/public/components/external_link/external_link_component.tsx index 4af95c83cc325..d2209efb8f100 100644 --- a/src/plugins/links/public/components/external_link/external_link_component.tsx +++ b/src/plugins/links/public/components/external_link/external_link_component.tsx @@ -65,6 +65,7 @@ export const ExternalLinkComponent = ({ return (