From 4dc556445cd78b18323cd9bcb5d6d104eca02644 Mon Sep 17 00:00:00 2001 From: Jack Westbrook Date: Mon, 27 Sep 2021 18:06:47 +0200 Subject: [PATCH] Plugin Catalog: Use routing for PluginDetails Tabs (#39555) * feat(catalog): introduce id and href to PluginDetailsTabs * feat(catalog): add hrefs and ids to PluginDetails tabs. Pass queryParams to PluginDetailsBody * feat(catalog): pass queryParams to PluginsDetailsBody and add page param to PluginListCard * fix(catalog): prevent flicker of content by waiting for fetch details to finish loading * feat(catalog): add tab icons to PluginDetails page * feat(catalog): make breadcrumbs in PluginDetailsHeader aware of page queryparam * fix(catalog): fix deeplinking to PluginDetails by comparing tabs length * test(catalog): update tests with correct props and wrap in router --- .../admin/components/PluginDetailsBody.tsx | 28 +++--- .../PluginDetailsHeaderDependencies.tsx | 4 +- .../admin/components/PluginListCard.test.tsx | 2 +- .../admin/components/PluginListCard.tsx | 6 +- .../admin/hooks/usePluginDetailsTabs.tsx | 15 +++- .../admin/pages/PluginDetails.test.tsx | 60 +++++++++---- .../plugins/admin/pages/PluginDetails.tsx | 89 +++++++++++-------- .../app/features/plugins/admin/state/hooks.ts | 7 ++ public/app/features/plugins/admin/types.ts | 13 ++- 9 files changed, 147 insertions(+), 77 deletions(-) diff --git a/public/app/features/plugins/admin/components/PluginDetailsBody.tsx b/public/app/features/plugins/admin/components/PluginDetailsBody.tsx index 2b68d3e7ba9..0213808a8dc 100644 --- a/public/app/features/plugins/admin/components/PluginDetailsBody.tsx +++ b/public/app/features/plugins/admin/components/PluginDetailsBody.tsx @@ -1,25 +1,26 @@ import React from 'react'; import { css, cx } from '@emotion/css'; -import { AppPlugin, GrafanaTheme2 } from '@grafana/data'; +import { AppPlugin, GrafanaTheme2, UrlQueryMap } from '@grafana/data'; import { useStyles2 } from '@grafana/ui'; -import { CatalogPlugin, PluginTabLabels } from '../types'; +import { CatalogPlugin, PluginTabIds } from '../types'; import { VersionList } from '../components/VersionList'; import { usePluginConfig } from '../hooks/usePluginConfig'; import { AppConfigCtrlWrapper } from '../../wrappers/AppConfigWrapper'; import { PluginDashboards } from '../../PluginDashboards'; type Props = { - tab: { label: string }; plugin: CatalogPlugin; + queryParams: UrlQueryMap; }; -export function PluginDetailsBody({ tab, plugin }: Props): JSX.Element | null { +export function PluginDetailsBody({ plugin, queryParams }: Props): JSX.Element { const styles = useStyles2(getStyles); const { value: pluginConfig } = usePluginConfig(plugin); + const pageId = queryParams.page; - if (tab?.label === PluginTabLabels.OVERVIEW) { + if (pageId === PluginTabIds.OVERVIEW) { return (
@@ -38,7 +39,7 @@ export function PluginDetailsBody({ tab, plugin }: Props): JSX.Element | null { ); } - if (tab?.label === PluginTabLabels.CONFIG && pluginConfig?.angularConfigCtrl) { + if (pageId === PluginTabIds.CONFIG && pluginConfig?.angularConfigCtrl) { return (
@@ -48,18 +49,17 @@ export function PluginDetailsBody({ tab, plugin }: Props): JSX.Element | null { if (pluginConfig?.configPages) { for (const configPage of pluginConfig.configPages) { - if (tab?.label === configPage.title) { + if (pageId === configPage.id) { return (
- {/* TODO: we should pass the query params down */} - +
); } } } - if (tab?.label === PluginTabLabels.DASHBOARDS && pluginConfig) { + if (pageId === PluginTabIds.DASHBOARDS && pluginConfig) { return (
@@ -67,7 +67,11 @@ export function PluginDetailsBody({ tab, plugin }: Props): JSX.Element | null { ); } - return null; + return ( +
+

Page not found.

+
+ ); } export const getStyles = (theme: GrafanaTheme2) => ({ diff --git a/public/app/features/plugins/admin/components/PluginDetailsHeaderDependencies.tsx b/public/app/features/plugins/admin/components/PluginDetailsHeaderDependencies.tsx index 045979a72ac..2c15312a27a 100644 --- a/public/app/features/plugins/admin/components/PluginDetailsHeaderDependencies.tsx +++ b/public/app/features/plugins/admin/components/PluginDetailsHeaderDependencies.tsx @@ -2,7 +2,7 @@ import React from 'react'; import { css } from '@emotion/css'; import { GrafanaTheme2 } from '@grafana/data'; import { useStyles2, Icon } from '@grafana/ui'; -import { CatalogPlugin, IconName } from '../types'; +import { CatalogPlugin, PluginIconName } from '../types'; type Props = { plugin: CatalogPlugin; @@ -37,7 +37,7 @@ export function PluginDetailsHeaderDependencies({ plugin, className }: Props): R {pluginDependencies.map((p) => { return ( - + {p.name} {p.version} ); diff --git a/public/app/features/plugins/admin/components/PluginListCard.test.tsx b/public/app/features/plugins/admin/components/PluginListCard.test.tsx index 8440a925b95..8f0188c8dd7 100644 --- a/public/app/features/plugins/admin/components/PluginListCard.test.tsx +++ b/public/app/features/plugins/admin/components/PluginListCard.test.tsx @@ -33,7 +33,7 @@ describe('PluginCard', () => { it('renders a card with link, image, name, orgName and badges', () => { render(); - expect(screen.getByRole('link')).toHaveAttribute('href', '/plugins/test-plugin'); + expect(screen.getByRole('link')).toHaveAttribute('href', '/plugins/test-plugin?page=overview'); const logo = screen.getByRole('img'); expect(logo).toHaveAttribute('src', plugin.info.logos.small); diff --git a/public/app/features/plugins/admin/components/PluginListCard.tsx b/public/app/features/plugins/admin/components/PluginListCard.tsx index 8db2d9197e8..f32801a1cf4 100644 --- a/public/app/features/plugins/admin/components/PluginListCard.tsx +++ b/public/app/features/plugins/admin/components/PluginListCard.tsx @@ -2,7 +2,7 @@ import React from 'react'; import { css } from '@emotion/css'; import { Icon, useStyles2, CardContainer, HorizontalGroup, VerticalGroup, Tooltip } from '@grafana/ui'; import { GrafanaTheme2 } from '@grafana/data'; -import { CatalogPlugin, IconName } from '../types'; +import { CatalogPlugin, PluginIconName, PluginTabIds } from '../types'; import { PluginLogo } from './PluginLogo'; import { PluginListBadges } from './PluginListBadges'; @@ -17,7 +17,7 @@ export function PluginListCard({ plugin, pathName }: PluginListCardProps) { const styles = useStyles2(getStyles); return ( - +
{plugin.name} {plugin.type && (
- +
)}
diff --git a/public/app/features/plugins/admin/hooks/usePluginDetailsTabs.tsx b/public/app/features/plugins/admin/hooks/usePluginDetailsTabs.tsx index 6cbfe243e24..32610b91d9c 100644 --- a/public/app/features/plugins/admin/hooks/usePluginDetailsTabs.tsx +++ b/public/app/features/plugins/admin/hooks/usePluginDetailsTabs.tsx @@ -1,6 +1,7 @@ import { useMemo } from 'react'; +import { useLocation } from 'react-router-dom'; import { PluginIncludeType, PluginType } from '@grafana/data'; -import { CatalogPlugin, PluginDetailsTab } from '../types'; +import { CatalogPlugin, PluginDetailsTab, PluginTabIds } from '../types'; import { isOrgAdmin } from '../helpers'; import { usePluginConfig } from '../hooks/usePluginConfig'; @@ -12,6 +13,7 @@ type ReturnType = { export const usePluginDetailsTabs = (plugin?: CatalogPlugin, defaultTabs: PluginDetailsTab[] = []): ReturnType => { const { loading, error, value: pluginConfig } = usePluginConfig(plugin); + const { pathname } = useLocation(); const tabs = useMemo(() => { const canConfigurePlugins = isOrgAdmin(); const tabs: PluginDetailsTab[] = [...defaultTabs]; @@ -26,6 +28,9 @@ export const usePluginDetailsTabs = (plugin?: CatalogPlugin, defaultTabs: Plugin if (pluginConfig.angularConfigCtrl) { tabs.push({ label: 'Config', + icon: 'cog', + id: PluginTabIds.CONFIG, + href: `${pathname}?page=${PluginTabIds.CONFIG}`, }); } @@ -33,6 +38,9 @@ export const usePluginDetailsTabs = (plugin?: CatalogPlugin, defaultTabs: Plugin for (const page of pluginConfig.configPages) { tabs.push({ label: page.title, + icon: page.icon, + id: page.id, + href: `${pathname}?page=${page.id}`, }); } } @@ -40,13 +48,16 @@ export const usePluginDetailsTabs = (plugin?: CatalogPlugin, defaultTabs: Plugin if (pluginConfig.meta.includes?.find((include) => include.type === PluginIncludeType.dashboard)) { tabs.push({ label: 'Dashboards', + icon: 'apps', + id: PluginTabIds.DASHBOARDS, + href: `${pathname}?page=${PluginTabIds.DASHBOARDS}`, }); } } } return tabs; - }, [pluginConfig, defaultTabs]); + }, [pluginConfig, defaultTabs, pathname]); return { error, diff --git a/public/app/features/plugins/admin/pages/PluginDetails.test.tsx b/public/app/features/plugins/admin/pages/PluginDetails.test.tsx index b1601a4341f..8c87c75deec 100644 --- a/public/app/features/plugins/admin/pages/PluginDetails.test.tsx +++ b/public/app/features/plugins/admin/pages/PluginDetails.test.tsx @@ -1,12 +1,13 @@ import React from 'react'; import { Provider } from 'react-redux'; +import { MemoryRouter } from 'react-router-dom'; import { render, RenderResult, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { config } from '@grafana/runtime'; import { configureStore } from 'app/store/configureStore'; import PluginDetailsPage from './PluginDetails'; import { getRouteComponentProps } from 'app/core/navigation/__mocks__/routeProps'; -import { CatalogPlugin } from '../types'; +import { CatalogPlugin, PluginTabIds } from '../types'; import * as api from '../api'; import { mockPluginApis, getCatalogPluginMock, getPluginsStateMock } from '../__mocks__'; import { PluginErrorCode, PluginSignatureStatus } from '@grafana/data'; @@ -24,10 +25,19 @@ jest.mock('@grafana/runtime', () => { return mockedRuntime; }); -const renderPluginDetails = (pluginOverride: Partial): RenderResult => { +const renderPluginDetails = (pluginOverride: Partial, pageId = PluginTabIds.OVERVIEW): RenderResult => { const plugin = getCatalogPluginMock(pluginOverride); const { id } = plugin; - const props = getRouteComponentProps({ match: { params: { pluginId: id }, isExact: true, url: '', path: '' } }); + const props = getRouteComponentProps({ + match: { params: { pluginId: id }, isExact: true, url: '', path: '' }, + queryParams: { page: pageId }, + location: { + hash: '', + pathname: `/plugins/${id}`, + search: `?page=${pageId}`, + state: undefined, + }, + }); const store = configureStore({ plugins: getPluginsStateMock([plugin]), }); @@ -35,7 +45,8 @@ const renderPluginDetails = (pluginOverride: Partial): RenderResu return render( - + , + { wrapper: MemoryRouter } ); }; @@ -66,12 +77,22 @@ describe('Plugin details page', () => { local: { id }, }); - const props = getRouteComponentProps({ match: { params: { pluginId: id }, isExact: true, url: '', path: '' } }); + const props = getRouteComponentProps({ + match: { params: { pluginId: id }, isExact: true, url: '', path: '' }, + queryParams: { page: PluginTabIds.OVERVIEW }, + location: { + hash: '', + pathname: `/plugins/${id}`, + search: `?page=${PluginTabIds.OVERVIEW}`, + state: undefined, + }, + }); const store = configureStore(); const { queryByText } = render( - + , + { wrapper: MemoryRouter } ); await waitFor(() => expect(queryByText(/licensed under the apache 2.0 license/i)).toBeInTheDocument()); @@ -129,24 +150,25 @@ describe('Plugin details page', () => { }); it('should display version history in case it is available', async () => { - const { queryByText, getByText, getByRole } = renderPluginDetails({ - id, - details: { - links: [], - versions: [ - { - version: '1.0.0', - createdAt: '2016-04-06T20:23:41.000Z', - }, - ], + const { queryByText, getByRole } = renderPluginDetails( + { + id, + details: { + links: [], + versions: [ + { + version: '1.0.0', + createdAt: '2016-04-06T20:23:41.000Z', + }, + ], + }, }, - }); + PluginTabIds.VERSIONS + ); // Check if version information is available await waitFor(() => expect(queryByText(/version history/i)).toBeInTheDocument()); - // Go to the versions tab - userEvent.click(getByText(/version history/i)); expect( getByRole('columnheader', { name: /version/i, diff --git a/public/app/features/plugins/admin/pages/PluginDetails.tsx b/public/app/features/plugins/admin/pages/PluginDetails.tsx index 2829dcb4734..81758126f8b 100644 --- a/public/app/features/plugins/admin/pages/PluginDetails.tsx +++ b/public/app/features/plugins/admin/pages/PluginDetails.tsx @@ -1,7 +1,9 @@ -import React, { useEffect, useState, useCallback } from 'react'; +import React, { useEffect } from 'react'; import { css } from '@emotion/css'; +import { usePrevious } from 'react-use'; import { GrafanaTheme2 } from '@grafana/data'; -import { useStyles2, TabsBar, TabContent, Tab, Alert } from '@grafana/ui'; +import { useStyles2, TabsBar, TabContent, Tab, Alert, IconName } from '@grafana/ui'; +import { locationService } from '@grafana/runtime'; import { Layout } from '@grafana/ui/src/components/Layout/Layout'; import { Page } from 'app/core/components/Page/Page'; import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; @@ -10,43 +12,53 @@ import { PluginDetailsHeader } from '../components/PluginDetailsHeader'; import { PluginDetailsBody } from '../components/PluginDetailsBody'; import { Page as PluginPage } from '../components/Page'; import { Loader } from '../components/Loader'; -import { PluginTabLabels, PluginDetailsTab } from '../types'; -import { useGetSingle, useFetchStatus } from '../state/hooks'; +import { PluginTabLabels, PluginTabIds, PluginDetailsTab } from '../types'; +import { useGetSingle, useFetchStatus, useFetchDetailsStatus } from '../state/hooks'; import { usePluginDetailsTabs } from '../hooks/usePluginDetailsTabs'; import { AppNotificationSeverity } from 'app/types'; import { PluginDetailsDisabledError } from '../components/PluginDetailsDisabledError'; type Props = GrafanaRouteComponentProps<{ pluginId?: string }>; -type State = { - tabs: PluginDetailsTab[]; - activeTabIndex: number; -}; - -const DefaultState = { - tabs: [{ label: PluginTabLabels.OVERVIEW }, { label: PluginTabLabels.VERSIONS }], - activeTabIndex: 0, -}; - -export default function PluginDetails({ match }: Props): JSX.Element | null { - const { pluginId = '' } = match.params; - const [state, setState] = useState(DefaultState); +export default function PluginDetails({ match, queryParams }: Props): JSX.Element | null { + const { + params: { pluginId = '' }, + url, + } = match; + const pageId = (queryParams.page as PluginTabIds) || PluginTabIds.OVERVIEW; + const parentUrl = url.substring(0, url.lastIndexOf('/')); + const defaultTabs: PluginDetailsTab[] = [ + { + label: PluginTabLabels.OVERVIEW, + icon: 'file-alt', + id: PluginTabIds.OVERVIEW, + href: `${url}?page=${PluginTabIds.OVERVIEW}`, + }, + { + label: PluginTabLabels.VERSIONS, + icon: 'history', + id: PluginTabIds.VERSIONS, + href: `${url}?page=${PluginTabIds.VERSIONS}`, + }, + ]; const plugin = useGetSingle(pluginId); // fetches the localplugin settings - const { tabs } = usePluginDetailsTabs(plugin, DefaultState.tabs); - const { activeTabIndex } = state; - const { isLoading } = useFetchStatus(); + const { tabs } = usePluginDetailsTabs(plugin, defaultTabs); + const { isLoading: isFetchLoading } = useFetchStatus(); + const { isLoading: isFetchDetailsLoading } = useFetchDetailsStatus(); const styles = useStyles2(getStyles); - const setActiveTab = useCallback((activeTabIndex: number) => setState({ ...state, activeTabIndex }), [state]); - const parentUrl = match.url.substring(0, match.url.lastIndexOf('/')); + const prevTabs = usePrevious(tabs); // If an app plugin is uninstalled we need to reset the active tab when the config / dashboards tabs are removed. useEffect(() => { - if (activeTabIndex > tabs.length - 1) { - setActiveTab(0); - } - }, [setActiveTab, activeTabIndex, tabs]); + const hasUninstalledWithConfigPages = prevTabs && prevTabs.length > tabs.length; + const isViewingAConfigPage = pageId !== PluginTabIds.OVERVIEW && pageId !== PluginTabIds.VERSIONS; - if (isLoading) { + if (hasUninstalledWithConfigPages && isViewingAConfigPage) { + locationService.replace(`${url}?page=${PluginTabIds.OVERVIEW}`); + } + }, [pageId, url, tabs, prevTabs]); + + if (isFetchLoading || isFetchDetailsLoading) { return ( @@ -68,25 +80,28 @@ export default function PluginDetails({ match }: Props): JSX.Element | null { return ( - + {/* Tab navigation */} - {tabs.map((tab: PluginDetailsTab, idx: number) => ( - setActiveTab(idx)} - /> - ))} + {tabs.map((tab: PluginDetailsTab) => { + return ( + + ); + })} {/* Active tab */} - + diff --git a/public/app/features/plugins/admin/state/hooks.ts b/public/app/features/plugins/admin/state/hooks.ts index e7bf87d6ab7..793eb2c7dc3 100644 --- a/public/app/features/plugins/admin/state/hooks.ts +++ b/public/app/features/plugins/admin/state/hooks.ts @@ -70,6 +70,13 @@ export const useFetchStatus = () => { return { isLoading, error }; }; +export const useFetchDetailsStatus = () => { + const isLoading = useSelector(selectIsRequestPending(fetchDetails.typePrefix)); + const error = useSelector(selectRequestError(fetchDetails.typePrefix)); + + return { isLoading, error }; +}; + export const useInstallStatus = () => { const isInstalling = useSelector(selectIsRequestPending(install.typePrefix)); const error = useSelector(selectRequestError(install.typePrefix)); diff --git a/public/app/features/plugins/admin/types.ts b/public/app/features/plugins/admin/types.ts index a8d893941d0..ce94c1f78d4 100644 --- a/public/app/features/plugins/admin/types.ts +++ b/public/app/features/plugins/admin/types.ts @@ -6,6 +6,7 @@ import { PluginDependencies, PluginErrorCode, } from '@grafana/data'; +import { IconName } from '@grafana/ui'; import { StoreState, PluginsState } from 'app/types'; export type PluginTypeCode = 'app' | 'panel' | 'datasource'; @@ -19,7 +20,7 @@ export enum PluginAdminRoutes { DetailsAdmin = 'plugins-details-admin', } -export enum IconName { +export enum PluginIconName { app = 'apps', datasource = 'database', panel = 'credit-card', @@ -203,6 +204,13 @@ export enum PluginTabLabels { DASHBOARDS = 'Dashboards', } +export enum PluginTabIds { + OVERVIEW = 'overview', + VERSIONS = 'version-history', + CONFIG = 'config', + DASHBOARDS = 'dashboards', +} + export enum RequestStatus { Pending = 'Pending', Fulfilled = 'Fulfilled', @@ -219,6 +227,9 @@ export type RequestInfo = { export type PluginDetailsTab = { label: PluginTabLabels | string; + icon?: IconName | string; + id: PluginTabIds | string; + href?: string; }; // TODO