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 90dbdd434e2eb..cb2479bfb5f51 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 @@ -54,6 +54,9 @@ describe('Dashboard link component', () => { type: 'dashboardLink' as const, }; + const onLoading = jest.fn(); + const onRender = jest.fn(); + let linksEmbeddable: LinksEmbeddable; beforeEach(async () => { window.open = jest.fn(); @@ -76,10 +79,16 @@ describe('Dashboard link component', () => { test('by default uses navigateToApp to open in same tab', async () => { render( - + ); + await waitFor(() => expect(onLoading).toHaveBeenCalledTimes(1)); await waitFor(() => expect(fetchDashboard).toHaveBeenCalledTimes(1)); expect(fetchDashboard).toHaveBeenCalledWith(defaultLinkInfo.destination); expect(getDashboardLocator).toHaveBeenCalledTimes(1); @@ -90,6 +99,7 @@ describe('Dashboard link component', () => { }, linksEmbeddable, }); + await waitFor(() => expect(onRender).toHaveBeenCalledTimes(1)); const link = await screen.findByTestId('dashboardLink--foo'); expect(link).toHaveTextContent('another dashboard'); @@ -104,10 +114,17 @@ describe('Dashboard link component', () => { test('modified click does not trigger event.preventDefault', async () => { render( - + ); + await waitFor(() => expect(onLoading).toHaveBeenCalledTimes(1)); await waitFor(() => expect(fetchDashboard).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(onRender).toHaveBeenCalledTimes(1)); const link = await screen.findByTestId('dashboardLink--foo'); const clickEvent = createEvent.click(link, { ctrlKey: true }); const preventDefault = jest.spyOn(clickEvent, 'preventDefault'); @@ -122,12 +139,19 @@ describe('Dashboard link component', () => { }; render( - + ); + await waitFor(() => expect(onLoading).toHaveBeenCalledTimes(1)); await waitFor(() => expect(fetchDashboard).toHaveBeenCalledTimes(1)); expect(fetchDashboard).toHaveBeenCalledWith(linkInfo.destination); expect(getDashboardLocator).toHaveBeenCalledWith({ link: linkInfo, linksEmbeddable }); + await waitFor(() => expect(onRender).toHaveBeenCalledTimes(1)); const link = await screen.findByTestId('dashboardLink--foo'); expect(link).toBeInTheDocument(); await userEvent.click(link); @@ -147,11 +171,18 @@ describe('Dashboard link component', () => { }; render( - + ); + await waitFor(() => expect(onLoading).toHaveBeenCalledTimes(1)); await waitFor(() => expect(fetchDashboard).toHaveBeenCalledTimes(1)); expect(getDashboardLocator).toHaveBeenCalledWith({ link: linkInfo, linksEmbeddable }); + await waitFor(() => expect(onRender).toHaveBeenCalledTimes(1)); }); test('shows an error when fetchDashboard fails', async () => { @@ -162,10 +193,17 @@ describe('Dashboard link component', () => { }; render( - + ); + await waitFor(() => expect(onLoading).toHaveBeenCalledTimes(1)); await waitFor(() => expect(fetchDashboard).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(onRender).toHaveBeenCalledTimes(1)); const link = await screen.findByTestId('dashboardLink--notfound--error'); expect(link).toHaveTextContent(DashboardLinkStrings.getDashboardErrorLabel()); }); @@ -178,10 +216,17 @@ describe('Dashboard link component', () => { }; render( - + ); + await waitFor(() => expect(onLoading).toHaveBeenCalledTimes(1)); await waitFor(() => expect(fetchDashboard).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(onRender).toHaveBeenCalledTimes(1)); const link = await screen.findByTestId('dashboardLink--bar'); expect(link).toHaveTextContent('current dashboard'); await userEvent.click(link); @@ -192,10 +237,17 @@ describe('Dashboard link component', () => { test('shows dashboard title and description in tooltip', async () => { render( - + ); + await waitFor(() => expect(onLoading).toHaveBeenCalledTimes(1)); await waitFor(() => expect(fetchDashboard).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(onRender).toHaveBeenCalledTimes(1)); const link = await screen.findByTestId('dashboardLink--foo'); await userEvent.hover(link); const tooltip = await screen.findByTestId('dashboardLink--foo--tooltip'); @@ -211,10 +263,17 @@ describe('Dashboard link component', () => { }; render( - + ); + await waitFor(() => expect(onLoading).toHaveBeenCalledTimes(1)); await waitFor(() => expect(fetchDashboard).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(onRender).toHaveBeenCalledTimes(1)); const link = await screen.findByTestId('dashboardLink--foo'); expect(link).toHaveTextContent(label); await userEvent.hover(link); 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 563cf6277c796..38097b478509a 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 @@ -8,7 +8,7 @@ import classNames from 'classnames'; import useAsync from 'react-use/lib/useAsync'; -import React, { useMemo, useState } from 'react'; +import React, { useEffect, useMemo, useState } from 'react'; import useObservable from 'react-use/lib/useObservable'; import { @@ -27,9 +27,13 @@ import { fetchDashboard, getDashboardHref, getDashboardLocator } from './dashboa export const DashboardLinkComponent = ({ link, layout, + onLoading, + onRender, }: { link: Link; layout: LinksLayoutType; + onLoading: () => void; + onRender: () => void; }) => { const linksEmbeddable = useLinks(); const [error, setError] = useState(); @@ -133,6 +137,21 @@ export const DashboardLinkComponent = ({ }; }, [link]); + useEffect(() => { + if (loadingDestinationDashboard || loadingOnClickProps) { + onLoading(); + } else { + onRender(); + } + }, [ + link, + linksEmbeddable, + loadingDestinationDashboard, + loadingOnClickProps, + onLoading, + onRender, + ]); + const id = `dashboardLink--${link.id}`; return loadingDestinationDashboard ? ( 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 1afdc17c43563..02f984435f8ba 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 @@ -17,6 +17,8 @@ import { ExternalLinkComponent } from './external_link_component'; import { coreServices } from '../../services/kibana_services'; import { DEFAULT_URL_DRILLDOWN_OPTIONS } from '@kbn/ui-actions-enhanced-plugin/public'; +const onRender = jest.fn(); + describe('external link component', () => { const defaultLinkInfo = { destination: 'https://example.com', @@ -38,10 +40,15 @@ describe('external link component', () => { test('by default opens in new tab', async () => { render( - + ); + expect(onRender).toBeCalledTimes(1); const link = await screen.findByTestId('externalLink--foo'); expect(link).toBeInTheDocument(); await userEvent.click(link); @@ -55,9 +62,10 @@ describe('external link component', () => { }; render( - + ); + expect(onRender).toBeCalledTimes(1); const link = await screen.findByTestId('externalLink--foo'); expect(link).toHaveTextContent('https://example.com'); const clickEvent = createEvent.click(link, { ctrlKey: true }); @@ -73,9 +81,10 @@ describe('external link component', () => { }; render( - + ); + expect(onRender).toBeCalledTimes(1); const link = await screen.findByTestId('externalLink--foo'); await userEvent.click(link); expect(coreServices.application.navigateToUrl).toBeCalledTimes(1); @@ -89,9 +98,10 @@ describe('external link component', () => { }; render( - + ); + expect(onRender).toBeCalledTimes(1); const link = await screen.findByTestId('externalLink--foo--error'); expect(link).toBeDisabled(); /** 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 9c2231fe6b711..ac409cfbac4cf 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 @@ -7,6 +7,7 @@ */ import React, { useMemo, useState } from 'react'; +import useMount from 'react-use/lib/useMount'; import { UrlDrilldownOptions, @@ -21,12 +22,18 @@ import { Link, LinksLayoutType, LINKS_VERTICAL_LAYOUT } from '../../../common/co export const ExternalLinkComponent = ({ link, layout, + onRender, }: { link: Link; layout: LinksLayoutType; + onRender: () => void; }) => { const [error, setError] = useState(); + useMount(() => { + onRender(); + }); + const linkOptions = useMemo(() => { return { ...DEFAULT_URL_DRILLDOWN_OPTIONS, diff --git a/src/plugins/links/public/components/links_component.tsx b/src/plugins/links/public/components/links_component.tsx index 9400dc9fe7308..c72c0db04fd57 100644 --- a/src/plugins/links/public/components/links_component.tsx +++ b/src/plugins/links/public/components/links_component.tsx @@ -6,7 +6,8 @@ * Side Public License, v 1. */ -import React, { useMemo } from 'react'; +import React, { useEffect, useMemo } from 'react'; +import useMap from 'react-use/lib/useMap'; import { EuiListGroup, EuiPanel } from '@elastic/eui'; import { useLinks } from '../embeddable/links_embeddable'; import { ExternalLinkComponent } from './external_link/external_link_component'; @@ -25,6 +26,22 @@ export const LinksComponent = () => { const links = linksEmbeddable.select((state) => state.componentState.links); const layout = linksEmbeddable.select((state) => state.componentState.layout); + const [linksLoading, { set: setLinkIsLoading }] = useMap( + Object.fromEntries( + (links ?? []).map((link) => { + return [link.id, true]; + }) + ) + ); + + useEffect(() => { + if (Object.values(linksLoading).includes(true)) { + linksEmbeddable.onLoading(); + } else { + linksEmbeddable.onRender(); + } + }, [linksLoading, linksEmbeddable]); + const orderedLinks = useMemo(() => { if (!links) return []; return memoizedGetOrderedLinkList(links); @@ -42,18 +59,21 @@ export const LinksComponent = () => { key={currentLink.id} link={currentLink} layout={layout ?? LINKS_VERTICAL_LAYOUT} + onLoading={() => setLinkIsLoading(currentLink.id, true)} + onRender={() => setLinkIsLoading(currentLink.id, false)} /> ) : ( setLinkIsLoading(currentLink.id, false)} /> ), }, }; }, {}); - }, [links, layout]); + }, [links, layout, setLinkIsLoading]); return ( diff --git a/test/functional/apps/dashboard_elements/links/links_navigation.ts b/test/functional/apps/dashboard_elements/links/links_navigation.ts index c4adf6e2c7042..d9f6f5b2bb6a9 100644 --- a/test/functional/apps/dashboard_elements/links/links_navigation.ts +++ b/test/functional/apps/dashboard_elements/links/links_navigation.ts @@ -18,13 +18,17 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const kibanaServer = getService('kibanaServer'); const dashboardAddPanel = getService('dashboardAddPanel'); - const { dashboard, common, timePicker } = getPageObjects(['dashboard', 'common', 'timePicker']); + const { dashboard, common, header, timePicker } = getPageObjects([ + 'dashboard', + 'common', + 'header', + 'timePicker', + ]); const FROM_TIME = 'Oct 22, 2018 @ 00:00:00.000'; const TO_TIME = 'Dec 3, 2018 @ 00:00:00.000'; - // Failing: See https://github.com/elastic/kibana/issues/167713 - describe.skip('links panel navigation', () => { + describe('links panel navigation', () => { before(async () => { await kibanaServer.savedObjects.cleanStandardList(); await security.testUser.setRoles([ @@ -84,6 +88,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { it('should disable link if dashboard does not exist', async () => { await dashboard.loadSavedDashboard('links 001'); + await dashboard.waitForRenderComplete(); expect(await testSubjects.exists('dashboardLink--link004--error')).to.be(true); expect(await testSubjects.isEnabled('dashboardLink--link004--error')).to.be(false); }); @@ -96,10 +101,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { * but should not override the date range. */ await dashboard.loadSavedDashboard('links 002'); - await testSubjects.click('dashboardLink--link001'); + await dashboard.waitForRenderComplete(); + await testSubjects.clickWhenNotDisabled('dashboardLink--link001'); + await header.waitUntilLoadingHasFinished(); expect(await dashboard.getDashboardIdFromCurrentUrl()).to.equal( '0930f310-5bc2-11ee-9a85-7b86504227bc' ); + await dashboard.waitForRenderComplete(); // Should pass the filters expect(await filterBar.getFilterCount()).to.equal(2); const filterLabels = await filterBar.getFiltersLabel(); @@ -127,10 +135,14 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { * but should not pass its filters. */ await dashboard.loadSavedDashboard('links 001'); - await testSubjects.click('dashboardLink--link002'); + await dashboard.waitForRenderComplete(); + await testSubjects.clickWhenNotDisabled('dashboardLink--link002'); + await header.waitUntilLoadingHasFinished(); expect(await dashboard.getDashboardIdFromCurrentUrl()).to.equal( '24751520-5bc2-11ee-9a85-7b86504227bc' ); + + await dashboard.waitForRenderComplete(); // Should pass the date range const time = await timePicker.getTimeConfig(); expect(time.start).to.be('Oct 31, 2018 @ 00:00:00.000'); @@ -157,7 +169,9 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { * to dashboard links003. */ await dashboard.loadSavedDashboard('links 001'); - await testSubjects.click('dashboardLink--link003'); + await dashboard.waitForRenderComplete(); + await testSubjects.clickWhenNotDisabled('dashboardLink--link003'); + await header.waitUntilLoadingHasFinished(); // Should have opened another tab const windowHandlers = await browser.getAllWindowHandles(); @@ -167,6 +181,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { '27398c50-5bc2-11ee-9a85-7b86504227bc' ); + await dashboard.waitForRenderComplete(); // Should not pass any filters expect((await filterBar.getFiltersLabel()).length).to.equal(0); @@ -180,6 +195,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { describe('external links', () => { before(async () => { await dashboard.loadSavedDashboard('dashboard with external links'); + await header.waitUntilLoadingHasFinished(); }); afterEach(async () => { @@ -197,8 +213,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(isDisabled).to.be('true'); }); - it('should create an external link when openInNewTab is enabled', async () => { - await testSubjects.click('externalLink--link999'); + // TODO We should not be using an external website for our tests. This will be flaky + // if external network connectivity issues exist. + it.skip('should create an external link when openInNewTab is enabled', async () => { + await testSubjects.clickWhenNotDisabled('externalLink--link999'); // Should have opened another tab const windowHandlers = await browser.getAllWindowHandles(); @@ -208,8 +226,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(currentUrl).to.be('https://example.com/1'); }); - it('should open in same tab when openInNewTab is disabled', async () => { - await testSubjects.click('externalLink--link888'); + it.skip('should open in same tab when openInNewTab is disabled', async () => { + await testSubjects.clickWhenNotDisabled('externalLink--link888'); // Should have opened in the same tab const windowHandlers = await browser.getAllWindowHandles();