From 018733dd24df2c2701ba079f859a29db254ecf1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Mon, 26 Sep 2022 15:04:07 +0200 Subject: [PATCH] PluginDetails: Make plugin details page look good in topnav (#55571) * PluginDetails: Make plugin details page look good in topnav * Minor style tweak aligning things * minor refactoring where I moved the logic to decide the default tab into its own hook. * refactor(plugindetails): first pass at using navmodel for usePluginDetailsTabs hook * refactor(plugindetails): move "reset page when uninstalling plugin" to installcontrols this prevents a user from seeing a blank page if they uninstall an app plugin whilst viewing a config page * refactor(plugindetails): remove usage of toIconName and reduce nested if * Trying to fix tests * minor fix * test(plugindetails): update selectors causing failing tests * chore(plugindetails): remove commented out test code * test(plugindetails): clean up - remove unnecesary usage of waitFor Co-authored-by: Marcus Andersson Co-authored-by: Jack Westbrook --- packages/grafana-data/src/types/navModel.ts | 2 + pkg/build/packaging/grafana.go | 2 +- pkg/services/ngalert/eval/evaluator_mock.go | 22 +-- .../core/components/PageHeader/PageHeader.tsx | 1 + .../core/components/PageNew/PageHeader.tsx | 1 + public/app/core/services/context_srv.ts | 2 +- .../InstallControls/InstallControlsButton.tsx | 13 +- .../admin/components/PluginDetailsBody.tsx | 5 +- .../admin/components/PluginDetailsHeader.tsx | 119 +++++--------- .../PluginDetailsHeaderDependencies.tsx | 7 +- .../admin/hooks/usePluginDetailsTabs.tsx | 154 +++++++++++------- .../admin/pages/PluginDetails.test.tsx | 115 +++++++------ .../plugins/admin/pages/PluginDetails.tsx | 61 +------ public/app/features/plugins/admin/types.ts | 2 +- 14 files changed, 233 insertions(+), 273 deletions(-) diff --git a/packages/grafana-data/src/types/navModel.ts b/packages/grafana-data/src/types/navModel.ts index 23ee7b140a4..40e6683c52e 100644 --- a/packages/grafana-data/src/types/navModel.ts +++ b/packages/grafana-data/src/types/navModel.ts @@ -36,6 +36,8 @@ export interface NavModelItem extends NavLinkDTO { highlightId?: string; tabSuffix?: ComponentType<{ className?: string }>; hideFromBreadcrumbs?: boolean; + /** To render custom things between title and child tabs */ + headerExtra?: ComponentType; } export enum NavSection { diff --git a/pkg/build/packaging/grafana.go b/pkg/build/packaging/grafana.go index 8532ebb8392..1c966286983 100644 --- a/pkg/build/packaging/grafana.go +++ b/pkg/build/packaging/grafana.go @@ -1033,7 +1033,7 @@ func createZip(srcDir, version, variantStr, sfx, grafanaDir string) error { return nil } -//nolint +// nolint func createTarball(srcDir, version, variantStr, sfx, grafanaDir string) error { fpath := filepath.Join(grafanaDir, "dist", fmt.Sprintf("grafana%s-%s.%s.tar.gz", sfx, version, variantStr)) //nolint:gosec diff --git a/pkg/services/ngalert/eval/evaluator_mock.go b/pkg/services/ngalert/eval/evaluator_mock.go index d2ad62726f5..dbfcd3360c9 100644 --- a/pkg/services/ngalert/eval/evaluator_mock.go +++ b/pkg/services/ngalert/eval/evaluator_mock.go @@ -51,10 +51,10 @@ type FakeEvaluator_ConditionEval_Call struct { } // ConditionEval is a helper method to define mock.On call -// - ctx context.Context -// - _a1 *user.SignedInUser -// - condition models.Condition -// - now time.Time +// - ctx context.Context +// - _a1 *user.SignedInUser +// - condition models.Condition +// - now time.Time func (_e *FakeEvaluator_Expecter) ConditionEval(ctx interface{}, _a1 interface{}, condition interface{}, now interface{}) *FakeEvaluator_ConditionEval_Call { return &FakeEvaluator_ConditionEval_Call{Call: _e.mock.On("ConditionEval", ctx, _a1, condition, now)} } @@ -100,10 +100,10 @@ type FakeEvaluator_QueriesAndExpressionsEval_Call struct { } // QueriesAndExpressionsEval is a helper method to define mock.On call -// - ctx context.Context -// - _a1 *user.SignedInUser -// - data []models.AlertQuery -// - now time.Time +// - ctx context.Context +// - _a1 *user.SignedInUser +// - data []models.AlertQuery +// - now time.Time func (_e *FakeEvaluator_Expecter) QueriesAndExpressionsEval(ctx interface{}, _a1 interface{}, data interface{}, now interface{}) *FakeEvaluator_QueriesAndExpressionsEval_Call { return &FakeEvaluator_QueriesAndExpressionsEval_Call{Call: _e.mock.On("QueriesAndExpressionsEval", ctx, _a1, data, now)} } @@ -140,9 +140,9 @@ type FakeEvaluator_Validate_Call struct { } // Validate is a helper method to define mock.On call -// - ctx context.Context -// - _a1 *user.SignedInUser -// - condition models.Condition +// - ctx context.Context +// - _a1 *user.SignedInUser +// - condition models.Condition func (_e *FakeEvaluator_Expecter) Validate(ctx interface{}, _a1 interface{}, condition interface{}) *FakeEvaluator_Validate_Call { return &FakeEvaluator_Validate_Call{Call: _e.mock.On("Validate", ctx, _a1, condition)} } diff --git a/public/app/core/components/PageHeader/PageHeader.tsx b/public/app/core/components/PageHeader/PageHeader.tsx index e9d6d292c03..991b2d055c7 100644 --- a/public/app/core/components/PageHeader/PageHeader.tsx +++ b/public/app/core/components/PageHeader/PageHeader.tsx @@ -108,6 +108,7 @@ function renderHeaderTitle(main: NavModelItem) {
{renderTitle(main.text, main.breadcrumbs ?? [], main.highlightText)} {main.subTitle &&
{main.subTitle}
} + {main.headerExtra && }
); diff --git a/public/app/core/components/PageNew/PageHeader.tsx b/public/app/core/components/PageNew/PageHeader.tsx index 930e8e5a1b4..8516a27dfda 100644 --- a/public/app/core/components/PageNew/PageHeader.tsx +++ b/public/app/core/components/PageNew/PageHeader.tsx @@ -20,6 +20,7 @@ export function PageHeader({ navItem, subTitle }: Props) { {navItem.text} {sub &&
{sub}
} + {navItem.headerExtra && } ); } diff --git a/public/app/core/services/context_srv.ts b/public/app/core/services/context_srv.ts index baaf6388e5f..aac450a9208 100644 --- a/public/app/core/services/context_srv.ts +++ b/public/app/core/services/context_srv.ts @@ -169,7 +169,7 @@ export class ContextSrv { return this.hasPermission(action); } - hasAccessInMetadata(action: string, object: WithAccessControlMetadata, fallBack: boolean) { + hasAccessInMetadata(action: string, object: WithAccessControlMetadata, fallBack: boolean): boolean { if (!this.accessControlEnabled()) { return fallBack; } diff --git a/public/app/features/plugins/admin/components/InstallControls/InstallControlsButton.tsx b/public/app/features/plugins/admin/components/InstallControls/InstallControlsButton.tsx index 6b47cb70014..25044906169 100644 --- a/public/app/features/plugins/admin/components/InstallControls/InstallControlsButton.tsx +++ b/public/app/features/plugins/admin/components/InstallControls/InstallControlsButton.tsx @@ -1,11 +1,14 @@ import React, { useState } from 'react'; +import { useLocation } from 'react-router-dom'; import { AppEvents } from '@grafana/data'; +import { locationService } from '@grafana/runtime'; import { Button, HorizontalGroup, ConfirmModal } from '@grafana/ui'; import appEvents from 'app/core/app_events'; +import { useQueryParams } from 'app/core/hooks/useQueryParams'; import { useInstallStatus, useUninstallStatus, useInstall, useUninstall } from '../../state/hooks'; -import { CatalogPlugin, PluginStatus, Version } from '../../types'; +import { CatalogPlugin, PluginStatus, PluginTabIds, Version } from '../../types'; type InstallControlsButtonProps = { plugin: CatalogPlugin; @@ -14,6 +17,8 @@ type InstallControlsButtonProps = { }; export function InstallControlsButton({ plugin, pluginStatus, latestCompatibleVersion }: InstallControlsButtonProps) { + const [queryParams] = useQueryParams(); + const location = useLocation(); const { isInstalling, error: errorInstalling } = useInstallStatus(); const { isUninstalling, error: errorUninstalling } = useUninstallStatus(); const install = useInstall(); @@ -34,6 +39,12 @@ export function InstallControlsButton({ plugin, pluginStatus, latestCompatibleVe hideConfirmModal(); await uninstall(plugin.id); if (!errorUninstalling) { + // If an app plugin is uninstalled we need to reset the active tab when the config / dashboards tabs are removed. + const activePageId = queryParams.page; + const isViewingAppConfigPage = activePageId !== PluginTabIds.OVERVIEW && activePageId !== PluginTabIds.VERSIONS; + if (isViewingAppConfigPage) { + locationService.replace(`${location.pathname}?page=${PluginTabIds.OVERVIEW}`); + } appEvents.emit(AppEvents.alertSuccess, [`Uninstalled ${plugin.name}`]); } }; diff --git a/public/app/features/plugins/admin/components/PluginDetailsBody.tsx b/public/app/features/plugins/admin/components/PluginDetailsBody.tsx index 7a9ffffbe98..3e2bb01fc2e 100644 --- a/public/app/features/plugins/admin/components/PluginDetailsBody.tsx +++ b/public/app/features/plugins/admin/components/PluginDetailsBody.tsx @@ -85,10 +85,7 @@ export function PluginDetailsBody({ plugin, queryParams, pageId }: Props): JSX.E } export const getStyles = (theme: GrafanaTheme2) => ({ - container: css` - padding: ${theme.spacing(3, 4)}; - height: 100%; - `, + container: css``, readme: css` & img { max-width: 100%; diff --git a/public/app/features/plugins/admin/components/PluginDetailsHeader.tsx b/public/app/features/plugins/admin/components/PluginDetailsHeader.tsx index a866e671935..91f0cff634d 100644 --- a/public/app/features/plugins/admin/components/PluginDetailsHeader.tsx +++ b/public/app/features/plugins/admin/components/PluginDetailsHeader.tsx @@ -1,4 +1,4 @@ -import { css, cx } from '@emotion/css'; +import { css } from '@emotion/css'; import React from 'react'; import { GrafanaTheme2 } from '@grafana/data'; @@ -12,94 +12,54 @@ import { GetStartedWithPlugin } from './GetStartedWithPlugin'; import { InstallControls } from './InstallControls'; import { PluginDetailsHeaderDependencies } from './PluginDetailsHeaderDependencies'; import { PluginDetailsHeaderSignature } from './PluginDetailsHeaderSignature'; -import { PluginLogo } from './PluginLogo'; type Props = { - currentUrl: string; - parentUrl: string; plugin: CatalogPlugin; }; -export function PluginDetailsHeader({ plugin, currentUrl, parentUrl }: Props): React.ReactElement { +export function PluginDetailsHeader({ plugin }: Props): React.ReactElement { const styles = useStyles2(getStyles); const latestCompatibleVersion = getLatestCompatibleVersion(plugin.details?.versions); const version = plugin.installedVersion || latestCompatibleVersion?.version; return ( -
-
-
- +
+ {plugin.description &&
{plugin.description}
} -
- {/* Title & navigation */} - +
+ {/* Version */} + {Boolean(version) && Version: {version}} -
- {/* Org name */} - {plugin.orgName} + {/* Org name */} + From: {plugin.orgName} - {/* Links */} - {plugin.details?.links.map((link: any) => ( - - {link.name} - - ))} + {/* Links */} + {plugin.details?.links.map((link: any) => ( + + {link.name} + + ))} - {/* Downloads */} - {plugin.downloads > 0 && ( - - - {` ${new Intl.NumberFormat().format(plugin.downloads)}`}{' '} - - )} + {/* Downloads */} + {plugin.downloads > 0 && ( + + + {` ${new Intl.NumberFormat().format(plugin.downloads)}`}{' '} + + )} - {/* Version */} - {Boolean(version) && {version}} + {/* Signature information */} + - {/* Signature information */} - + {plugin.isDisabled && } - {plugin.isDisabled && } -
- - - -

{plugin.description}

- - - - - -
-
+
+ + + + +
); } @@ -108,12 +68,11 @@ export const getStyles = (theme: GrafanaTheme2) => { return { headerContainer: css` display: flex; - margin-bottom: ${theme.spacing(3)}; - margin-top: ${theme.spacing(3)}; - min-height: 120px; + flex-direction: column; + margin-bottom: ${theme.spacing(1)}; `, - headerWrapper: css` - margin-left: ${theme.spacing(3)}; + description: css` + margin: ${theme.spacing(-1, 0, 1)}; `, breadcrumb: css` font-size: ${theme.typography.h2.fontSize}; @@ -132,9 +91,9 @@ export const getStyles = (theme: GrafanaTheme2) => { headerInformationRow: css` display: flex; align-items: center; - margin-top: ${theme.spacing()}; - margin-bottom: ${theme.spacing()}; + margin-bottom: ${theme.spacing(1)}; flex-flow: wrap; + & > * { &::after { content: '|'; @@ -145,7 +104,6 @@ export const getStyles = (theme: GrafanaTheme2) => { padding-right: 0; } } - font-size: ${theme.typography.h4.fontSize}; a { &:hover { @@ -153,9 +111,6 @@ export const getStyles = (theme: GrafanaTheme2) => { } } `, - headerInformationRowSecondary: css` - font-size: ${theme.typography.body.fontSize}; - `, headerOrgName: css` font-size: ${theme.typography.h4.fontSize}; `, diff --git a/public/app/features/plugins/admin/components/PluginDetailsHeaderDependencies.tsx b/public/app/features/plugins/admin/components/PluginDetailsHeaderDependencies.tsx index ba73cf57716..3be03f5a415 100644 --- a/public/app/features/plugins/admin/components/PluginDetailsHeaderDependencies.tsx +++ b/public/app/features/plugins/admin/components/PluginDetailsHeaderDependencies.tsx @@ -2,7 +2,7 @@ import { css } from '@emotion/css'; import React from 'react'; import { GrafanaTheme2 } from '@grafana/data'; -import { useStyles2, Icon } from '@grafana/ui'; +import { useStyles2, Icon, Stack } from '@grafana/ui'; import { Version, CatalogPlugin, PluginIconName } from '../types'; @@ -29,7 +29,7 @@ export function PluginDetailsHeaderDependencies({ } return ( -
+
Dependencies:
{/* Grafana dependency */} @@ -53,14 +53,13 @@ export function PluginDetailsHeaderDependencies({ })}
)} -
+ ); } export const getStyles = (theme: GrafanaTheme2) => { return { dependencyTitle: css` - font-weight: ${theme.typography.fontWeightBold}; margin-right: ${theme.spacing(0.5)}; &::after { diff --git a/public/app/features/plugins/admin/hooks/usePluginDetailsTabs.tsx b/public/app/features/plugins/admin/hooks/usePluginDetailsTabs.tsx index db1d65f9e09..4e015d721b0 100644 --- a/public/app/features/plugins/admin/hooks/usePluginDetailsTabs.tsx +++ b/public/app/features/plugins/admin/hooks/usePluginDetailsTabs.tsx @@ -1,104 +1,146 @@ -import { useMemo } from 'react'; +import React, { useMemo } from 'react'; import { useLocation } from 'react-router-dom'; -import { PluginIncludeType, PluginType } from '@grafana/data'; +import { GrafanaPlugin, NavModelItem, PluginIncludeType, PluginType } from '@grafana/data'; import { config } from '@grafana/runtime'; import { contextSrv } from 'app/core/core'; import { AccessControlAction } from 'app/types'; +import { PluginDetailsHeader } from '../components/PluginDetailsHeader'; import { usePluginConfig } from '../hooks/usePluginConfig'; import { isOrgAdmin } from '../permissions'; -import { CatalogPlugin, PluginDetailsTab, PluginTabIds, PluginTabLabels } from '../types'; +import { CatalogPlugin, PluginTabIds, PluginTabLabels } from '../types'; type ReturnType = { error: Error | undefined; loading: boolean; - tabs: PluginDetailsTab[]; - defaultTab: string; + navModel: NavModelItem; + activePageId: PluginTabIds | string; }; -export const usePluginDetailsTabs = (plugin?: CatalogPlugin, defaultTabs: PluginDetailsTab[] = []): ReturnType => { +export const usePluginDetailsTabs = (plugin?: CatalogPlugin, pageId?: PluginTabIds): ReturnType => { const { loading, error, value: pluginConfig } = usePluginConfig(plugin); - const isPublished = Boolean(plugin?.isPublished); const { pathname } = useLocation(); + const defaultTab = useDefaultPage(plugin, pluginConfig); + const parentUrl = pathname.substring(0, pathname.lastIndexOf('/')); + const isPublished = Boolean(plugin?.isPublished); - const [tabs, defaultTab] = useMemo(() => { + const currentPageId = pageId || defaultTab; + const navModelChildren = useMemo(() => { const canConfigurePlugins = plugin && contextSrv.hasAccessInMetadata(AccessControlAction.PluginsWrite, plugin, isOrgAdmin()); - const tabs: PluginDetailsTab[] = [...defaultTabs]; - let defaultTab; + const navModelChildren: NavModelItem[] = []; if (isPublished) { - tabs.push({ - label: PluginTabLabels.VERSIONS, - icon: 'history', + navModelChildren.push({ + text: PluginTabLabels.VERSIONS, id: PluginTabIds.VERSIONS, - href: `${pathname}?page=${PluginTabIds.VERSIONS}`, + icon: 'history', + url: `${pathname}?page=${PluginTabIds.VERSIONS}`, + active: PluginTabIds.VERSIONS === currentPageId, }); } // Not extending the tabs with the config pages if the plugin is not installed if (!pluginConfig) { - defaultTab = PluginTabIds.OVERVIEW; - return [tabs, defaultTab]; + return navModelChildren; } if (config.featureToggles.panelTitleSearch && pluginConfig.meta.type === PluginType.panel) { - tabs.push({ - label: PluginTabLabels.USAGE, + navModelChildren.push({ + text: PluginTabLabels.USAGE, icon: 'list-ul', id: PluginTabIds.USAGE, - href: `${pathname}?page=${PluginTabIds.USAGE}`, + url: `${pathname}?page=${PluginTabIds.USAGE}`, + active: PluginTabIds.USAGE === currentPageId, }); } - if (canConfigurePlugins) { - if (pluginConfig.meta.type === PluginType.app) { - if (pluginConfig.angularConfigCtrl) { - tabs.push({ - label: 'Config', - icon: 'cog', - id: PluginTabIds.CONFIG, - href: `${pathname}?page=${PluginTabIds.CONFIG}`, - }); - defaultTab = PluginTabIds.CONFIG; - } + if (!canConfigurePlugins) { + return navModelChildren; + } - if (pluginConfig.configPages) { - for (const page of pluginConfig.configPages) { - tabs.push({ - label: page.title, - icon: page.icon, - id: page.id, - href: `${pathname}?page=${page.id}`, - }); - if (!defaultTab) { - defaultTab = page.id; - } - } - } + if (pluginConfig.meta.type === PluginType.app) { + if (pluginConfig.angularConfigCtrl) { + navModelChildren.push({ + text: 'Config', + icon: 'cog', + id: PluginTabIds.CONFIG, + url: `${pathname}?page=${PluginTabIds.CONFIG}`, + active: PluginTabIds.CONFIG === currentPageId, + }); + } - if (pluginConfig.meta.includes?.find((include) => include.type === PluginIncludeType.dashboard)) { - tabs.push({ - label: 'Dashboards', - icon: 'apps', - id: PluginTabIds.DASHBOARDS, - href: `${pathname}?page=${PluginTabIds.DASHBOARDS}`, + if (pluginConfig.configPages) { + for (const configPage of pluginConfig.configPages) { + navModelChildren.push({ + text: configPage.title, + icon: configPage.icon, + id: configPage.id, + url: `${pathname}?page=${configPage.id}`, + active: configPage.id === currentPageId, }); } } + + if (pluginConfig.meta.includes?.find((include) => include.type === PluginIncludeType.dashboard)) { + navModelChildren.push({ + text: 'Dashboards', + icon: 'apps', + id: PluginTabIds.DASHBOARDS, + url: `${pathname}?page=${PluginTabIds.DASHBOARDS}`, + active: PluginTabIds.DASHBOARDS === currentPageId, + }); + } } - if (!defaultTab) { - defaultTab = PluginTabIds.OVERVIEW; - } + return navModelChildren; + }, [plugin, pluginConfig, pathname, isPublished, currentPageId]); - return [tabs, defaultTab]; - }, [plugin, pluginConfig, defaultTabs, pathname, isPublished]); + const navModel: NavModelItem = { + text: plugin?.name ?? '', + img: plugin?.info.logos.small, + breadcrumbs: [{ title: 'Plugins', url: parentUrl }], + children: [ + { + text: PluginTabLabels.OVERVIEW, + icon: 'file-alt', + id: PluginTabIds.OVERVIEW, + url: `${pathname}?page=${PluginTabIds.OVERVIEW}`, + active: PluginTabIds.OVERVIEW === currentPageId, + }, + ...navModelChildren, + ], + headerExtra: () => { + return plugin ? : null; + }, + }; return { error, loading, - tabs, - defaultTab, + navModel, + activePageId: currentPageId, }; }; + +function useDefaultPage(plugin: CatalogPlugin | undefined, pluginConfig: GrafanaPlugin | undefined | null) { + if (!plugin || !pluginConfig) { + return PluginTabIds.OVERVIEW; + } + + const hasAccess = contextSrv.hasAccessInMetadata(AccessControlAction.PluginsWrite, plugin, isOrgAdmin()); + + if (!hasAccess || pluginConfig.meta.type !== PluginType.app) { + return PluginTabIds.OVERVIEW; + } + + if (pluginConfig.angularConfigCtrl) { + return PluginTabIds.CONFIG; + } + + if (pluginConfig.configPages?.length) { + return pluginConfig.configPages[0].id; + } + + return PluginTabIds.OVERVIEW; +} diff --git a/public/app/features/plugins/admin/pages/PluginDetails.test.tsx b/public/app/features/plugins/admin/pages/PluginDetails.test.tsx index 2d92c367a10..5b2742aa66f 100644 --- a/public/app/features/plugins/admin/pages/PluginDetails.test.tsx +++ b/public/app/features/plugins/admin/pages/PluginDetails.test.tsx @@ -153,7 +153,6 @@ describe('Plugin details page', () => { - , ); @@ -163,7 +162,7 @@ describe('Plugin details page', () => { it('should display an overview (plugin readme) by default', async () => { const { queryByText } = renderPluginDetails({ id }); - await waitFor(() => expect(queryByText(/licensed under the apache 2.0 license/i)).toBeInTheDocument()); + expect(await queryByText(/licensed under the apache 2.0 license/i)).toBeInTheDocument(); }); it('should display an app config page by default for installed app plugins', async () => { @@ -197,7 +196,7 @@ describe('Plugin details page', () => { type: PluginType.app, }); - await waitFor(() => expect(queryByText(/custom config page/i)).toBeInTheDocument()); + expect(await queryByText(/custom config page/i)).toBeInTheDocument(); }); it('should display the number of downloads in the header', async () => { @@ -208,14 +207,14 @@ describe('Plugin details page', () => { const expected = new Intl.NumberFormat().format(downloads); const { queryByText } = renderPluginDetails({ id, downloads }); - await waitFor(() => expect(queryByText(expected, options)).toBeInTheDocument()); + expect(await queryByText(expected, options)).toBeInTheDocument(); }); it('should display the installed version if a plugin is installed', async () => { const installedVersion = '1.3.443'; const { queryByText } = renderPluginDetails({ id, installedVersion }); - await waitFor(() => expect(queryByText(installedVersion)).toBeInTheDocument()); + expect(await queryByText(`Version: ${installedVersion}`)).toBeInTheDocument(); }); it('should display the latest compatible version in the header if a plugin is not installed', async () => { @@ -230,40 +229,40 @@ describe('Plugin details page', () => { ], }; - const { queryByText } = renderPluginDetails({ id, details }); - await waitFor(() => expect(queryByText('1.1.1')).toBeInTheDocument()); - await waitFor(() => expect(queryByText(/>=8.0.0/i)).toBeInTheDocument()); + const { findByText, queryByText } = renderPluginDetails({ id, details }); + expect(await findByText('Version: 1.1.1')).toBeInTheDocument(); + expect(queryByText(/>=8.0.0/i)).toBeInTheDocument(); }); it('should display description in the header', async () => { const description = 'This is my description'; const { queryByText } = renderPluginDetails({ id, description }); - await waitFor(() => expect(queryByText(description)).toBeInTheDocument()); + expect(await queryByText(description)).toBeInTheDocument(); }); it('should display a "Signed" badge if the plugin signature is verified', async () => { const { queryByText } = renderPluginDetails({ id, signature: PluginSignatureStatus.valid }); - await waitFor(() => expect(queryByText('Signed')).toBeInTheDocument()); + expect(await queryByText('Signed')).toBeInTheDocument(); }); it('should display a "Missing signature" badge if the plugin signature is missing', async () => { const { queryByText } = renderPluginDetails({ id, signature: PluginSignatureStatus.missing }); - await waitFor(() => expect(queryByText('Missing signature')).toBeInTheDocument()); + expect(await queryByText('Missing signature')).toBeInTheDocument(); }); it('should display a "Modified signature" badge if the plugin signature is modified', async () => { const { queryByText } = renderPluginDetails({ id, signature: PluginSignatureStatus.modified }); - await waitFor(() => expect(queryByText('Modified signature')).toBeInTheDocument()); + expect(await queryByText('Modified signature')).toBeInTheDocument(); }); it('should display a "Invalid signature" badge if the plugin signature is invalid', async () => { const { queryByText } = renderPluginDetails({ id, signature: PluginSignatureStatus.invalid }); - await waitFor(() => expect(queryByText('Invalid signature')).toBeInTheDocument()); + expect(await queryByText('Invalid signature')).toBeInTheDocument(); }); it('should display version history if the plugin is published', async () => { @@ -288,7 +287,7 @@ describe('Plugin details page', () => { }, ]; - const { queryByText, getByRole } = renderPluginDetails( + const { findByRole, queryByText, getByRole } = renderPluginDetails( { id, details: { @@ -300,7 +299,7 @@ describe('Plugin details page', () => { ); // Check if version information is available - await waitFor(() => expect(queryByText(/version history/i)).toBeInTheDocument()); + expect(await findByRole('tab', { name: `Tab ${PluginTabLabels.VERSIONS}` })).toBeInTheDocument(); // Check the column headers expect(getByRole('columnheader', { name: /version/i })).toBeInTheDocument(); @@ -321,7 +320,7 @@ describe('Plugin details page', () => { it("should display an install button for a plugin that isn't installed", async () => { const { queryByRole } = renderPluginDetails({ id, isInstalled: false }); - await waitFor(() => expect(queryByRole('button', { name: /^install/i })).toBeInTheDocument()); + expect(await queryByRole('button', { name: /^install/i })).toBeInTheDocument(); // Does not display "uninstall" button expect(queryByRole('button', { name: /uninstall/i })).not.toBeInTheDocument(); }); @@ -329,7 +328,7 @@ describe('Plugin details page', () => { it('should display an uninstall button for an already installed plugin', async () => { const { queryByRole } = renderPluginDetails({ id, isInstalled: true }); - await waitFor(() => expect(queryByRole('button', { name: /uninstall/i })).toBeInTheDocument()); + expect(await queryByRole('button', { name: /uninstall/i })).toBeInTheDocument(); // Does not display "install" button expect(queryByRole('button', { name: /^install/i })).not.toBeInTheDocument(); }); @@ -338,7 +337,7 @@ describe('Plugin details page', () => { const { queryByRole } = renderPluginDetails({ id, isInstalled: true, hasUpdate: true }); // Displays an "update" button - await waitFor(() => expect(queryByRole('button', { name: /update/i })).toBeInTheDocument()); + expect(await queryByRole('button', { name: /update/i })).toBeInTheDocument(); expect(queryByRole('button', { name: /uninstall/i })).toBeInTheDocument(); // Does not display "install" button @@ -350,7 +349,7 @@ describe('Plugin details page', () => { const { queryByRole } = renderPluginDetails({ id, isInstalled: false, isEnterprise: true }); - await waitFor(() => expect(queryByRole('button', { name: /install/i })).toBeInTheDocument()); + expect(await queryByRole('button', { name: /install/i })).toBeInTheDocument(); }); it('should not display install button for enterprise plugins if license is invalid', async () => { @@ -358,7 +357,7 @@ describe('Plugin details page', () => { const { queryByRole, queryByText } = renderPluginDetails({ id, isInstalled: true, isEnterprise: true }); - await waitFor(() => expect(queryByRole('button', { name: /install/i })).not.toBeInTheDocument()); + expect(await queryByRole('button', { name: /install/i })).not.toBeInTheDocument(); expect(queryByText(/no valid Grafana Enterprise license detected/i)).toBeInTheDocument(); expect(queryByRole('link', { name: /learn more/i })).toBeInTheDocument(); }); @@ -366,22 +365,22 @@ describe('Plugin details page', () => { it('should not display install / uninstall buttons for core plugins', async () => { const { queryByRole } = renderPluginDetails({ id, isInstalled: true, isCore: true }); - await waitFor(() => expect(queryByRole('button', { name: /update/i })).not.toBeInTheDocument()); - await waitFor(() => expect(queryByRole('button', { name: /(un)?install/i })).not.toBeInTheDocument()); + expect(await queryByRole('button', { name: /update/i })).not.toBeInTheDocument(); + expect(await queryByRole('button', { name: /(un)?install/i })).not.toBeInTheDocument(); }); it('should not display install / uninstall buttons for disabled plugins', async () => { const { queryByRole } = renderPluginDetails({ id, isInstalled: true, isDisabled: true }); - await waitFor(() => expect(queryByRole('button', { name: /update/i })).not.toBeInTheDocument()); - await waitFor(() => expect(queryByRole('button', { name: /(un)?install/i })).not.toBeInTheDocument()); + expect(await queryByRole('button', { name: /update/i })).not.toBeInTheDocument(); + expect(await queryByRole('button', { name: /(un)?install/i })).not.toBeInTheDocument(); }); it('should not display install / uninstall buttons for renderer plugins', async () => { const { queryByRole } = renderPluginDetails({ id, type: PluginType.renderer }); - await waitFor(() => expect(queryByRole('button', { name: /update/i })).not.toBeInTheDocument()); - await waitFor(() => expect(queryByRole('button', { name: /(un)?install/i })).not.toBeInTheDocument()); + expect(await queryByRole('button', { name: /update/i })).not.toBeInTheDocument(); + expect(await queryByRole('button', { name: /(un)?install/i })).not.toBeInTheDocument(); }); it('should display install link with `config.pluginAdminExternalManageEnabled` set to true', async () => { @@ -389,7 +388,7 @@ describe('Plugin details page', () => { const { queryByRole } = renderPluginDetails({ id, isInstalled: false }); - await waitFor(() => expect(queryByRole('link', { name: /install via grafana.com/i })).toBeInTheDocument()); + expect(await queryByRole('link', { name: /install via grafana.com/i })).toBeInTheDocument(); }); it('should display uninstall link for an installed plugin with `config.pluginAdminExternalManageEnabled` set to true', async () => { @@ -397,7 +396,7 @@ describe('Plugin details page', () => { const { queryByRole } = renderPluginDetails({ id, isInstalled: true }); - await waitFor(() => expect(queryByRole('link', { name: /uninstall via grafana.com/i })).toBeInTheDocument()); + expect(await queryByRole('link', { name: /uninstall via grafana.com/i })).toBeInTheDocument(); }); it('should display update and uninstall links for a plugin with an available update and `config.pluginAdminExternalManageEnabled` set to true', async () => { @@ -405,7 +404,7 @@ describe('Plugin details page', () => { const { queryByRole } = renderPluginDetails({ id, isInstalled: true, hasUpdate: true }); - await waitFor(() => expect(queryByRole('link', { name: /update via grafana.com/i })).toBeInTheDocument()); + expect(await queryByRole('link', { name: /update via grafana.com/i })).toBeInTheDocument(); expect(queryByRole('link', { name: /uninstall via grafana.com/i })).toBeInTheDocument(); }); @@ -417,7 +416,7 @@ describe('Plugin details page', () => { error: PluginErrorCode.modifiedSignature, }); - await waitFor(() => expect(queryByLabelText(selectors.pages.PluginPage.disabledInfo)).toBeInTheDocument()); + expect(await queryByLabelText(selectors.pages.PluginPage.disabledInfo)).toBeInTheDocument(); }); it('should display grafana dependencies for a plugin if they are available', async () => { @@ -431,7 +430,7 @@ describe('Plugin details page', () => { }); // Wait for the dependencies part to be loaded - await waitFor(() => expect(queryByText(/dependencies:/i)).toBeInTheDocument()); + expect(await queryByText(/dependencies:/i)).toBeInTheDocument(); expect(queryByText('Grafana >=8.0.0')).toBeInTheDocument(); }); @@ -440,7 +439,7 @@ describe('Plugin details page', () => { // @ts-ignore api.uninstallPlugin = jest.fn(); - const { queryByText, getByRole } = renderPluginDetails({ + const { queryByText, getByRole, findByRole } = renderPluginDetails({ id, name: 'Akumuli', isInstalled: true, @@ -460,7 +459,7 @@ describe('Plugin details page', () => { }); // Wait for the install controls to be loaded - await waitFor(() => expect(queryByText(PluginTabLabels.OVERVIEW)).toBeInTheDocument()); + expect(await findByRole('tab', { name: `Tab ${PluginTabLabels.OVERVIEW}` })).toBeInTheDocument(); // Open the confirmation modal await userEvent.click(getByRole('button', { name: /uninstall/i })); @@ -496,17 +495,17 @@ describe('Plugin details page', () => { // Does not show an Install button rendered = renderPluginDetails({ id }, { pluginsStateOverride }); - await waitFor(() => expect(rendered.queryByRole('button', { name: /(un)?install/i })).not.toBeInTheDocument()); + expect(await rendered.queryByRole('button', { name: /(un)?install/i })).not.toBeInTheDocument(); rendered.unmount(); // Does not show a Uninstall button rendered = renderPluginDetails({ id, isInstalled: true }, { pluginsStateOverride }); - await waitFor(() => expect(rendered.queryByRole('button', { name: /(un)?install/i })).not.toBeInTheDocument()); + expect(await rendered.queryByRole('button', { name: /(un)?install/i })).not.toBeInTheDocument(); rendered.unmount(); // Does not show an Update button rendered = renderPluginDetails({ id, isInstalled: true, hasUpdate: true }, { pluginsStateOverride }); - await waitFor(() => expect(rendered.queryByRole('button', { name: /update/i })).not.toBeInTheDocument()); + expect(await rendered.queryByRole('button', { name: /update/i })).not.toBeInTheDocument(); // Shows a message to the user // TODO @@ -522,15 +521,15 @@ describe('Plugin details page', () => { // Should not show an "Install" button rendered = renderPluginDetails({ id, isInstalled: false }); - await waitFor(() => expect(rendered.queryByRole('button', { name: /^install/i })).not.toBeInTheDocument()); + expect(await rendered.queryByRole('button', { name: /^install/i })).not.toBeInTheDocument(); // Should not show an "Uninstall" button rendered = renderPluginDetails({ id, isInstalled: true }); - await waitFor(() => expect(rendered.queryByRole('button', { name: /^uninstall/i })).not.toBeInTheDocument()); + expect(await rendered.queryByRole('button', { name: /^uninstall/i })).not.toBeInTheDocument(); // Should not show an "Update" button rendered = renderPluginDetails({ id, isInstalled: true, hasUpdate: true }); - await waitFor(() => expect(rendered.queryByRole('button', { name: /^update/i })).not.toBeInTheDocument()); + expect(await rendered.queryByRole('button', { name: /^update/i })).not.toBeInTheDocument(); }); it('should display a "Create" button as a post installation step for installed data source plugins', async () => { @@ -703,20 +702,18 @@ describe('Plugin details page', () => { }); it('should not display versions tab for plugins not published to gcom', async () => { - const { queryByText } = renderPluginDetails({ + const { queryByRole } = renderPluginDetails({ name: 'Akumuli', isInstalled: true, type: PluginType.app, isPublished: false, }); - await waitFor(() => expect(queryByText(PluginTabLabels.OVERVIEW)).toBeInTheDocument()); - - expect(queryByText(PluginTabLabels.VERSIONS)).toBeNull(); + expect(await queryByRole('tab', { name: `Tab ${PluginTabLabels.VERSIONS}` })).not.toBeInTheDocument(); }); it('should not display update for plugins not published to gcom', async () => { - const { queryByText, queryByRole } = renderPluginDetails({ + const { findByRole, queryByRole } = renderPluginDetails({ name: 'Akumuli', isInstalled: true, hasUpdate: true, @@ -724,13 +721,13 @@ describe('Plugin details page', () => { isPublished: false, }); - await waitFor(() => expect(queryByText(PluginTabLabels.OVERVIEW)).toBeInTheDocument()); + expect(await findByRole('tab', { name: `Tab ${PluginTabLabels.OVERVIEW}` })).toBeInTheDocument(); expect(queryByRole('button', { name: /update/i })).not.toBeInTheDocument(); }); it('should not display install for plugins not published to gcom', async () => { - const { queryByText, queryByRole } = renderPluginDetails({ + const { findByRole, queryByRole } = renderPluginDetails({ name: 'Akumuli', isInstalled: false, hasUpdate: false, @@ -738,13 +735,13 @@ describe('Plugin details page', () => { isPublished: false, }); - await waitFor(() => expect(queryByText(PluginTabLabels.OVERVIEW)).toBeInTheDocument()); + expect(await findByRole('tab', { name: `Tab ${PluginTabLabels.OVERVIEW}` })).toBeInTheDocument(); expect(queryByRole('button', { name: /^install/i })).not.toBeInTheDocument(); }); it('should not display uninstall for plugins not published to gcom', async () => { - const { queryByText, queryByRole } = renderPluginDetails({ + const { findByRole, queryByRole } = renderPluginDetails({ name: 'Akumuli', isInstalled: true, hasUpdate: false, @@ -752,7 +749,7 @@ describe('Plugin details page', () => { isPublished: false, }); - await waitFor(() => expect(queryByText(PluginTabLabels.OVERVIEW)).toBeInTheDocument()); + expect(await findByRole('tab', { name: `Tab ${PluginTabLabels.OVERVIEW}` })).toBeInTheDocument(); expect(queryByRole('button', { name: /uninstall/i })).not.toBeInTheDocument(); }); @@ -768,37 +765,35 @@ describe('Plugin details page', () => { }); it("should not display an install button for a plugin that isn't installed", async () => { - const { queryByRole, queryByText } = renderPluginDetails({ id, isInstalled: false }); + const { queryByRole, findByRole } = renderPluginDetails({ id, isInstalled: false }); - await waitFor(() => expect(queryByText(PluginTabLabels.OVERVIEW)).toBeInTheDocument()); + expect(await findByRole('tab', { name: `Tab ${PluginTabLabels.OVERVIEW}` })).toBeInTheDocument(); expect(queryByRole('button', { name: /^install/i })).not.toBeInTheDocument(); }); it('should not display an uninstall button for an already installed plugin', async () => { - const { queryByRole, queryByText } = renderPluginDetails({ id, isInstalled: true }); + const { queryByRole, findByRole } = renderPluginDetails({ id, isInstalled: true }); - await waitFor(() => expect(queryByText(PluginTabLabels.OVERVIEW)).toBeInTheDocument()); + expect(await findByRole('tab', { name: `Tab ${PluginTabLabels.OVERVIEW}` })).toBeInTheDocument(); expect(queryByRole('button', { name: /uninstall/i })).not.toBeInTheDocument(); }); it('should not display update or uninstall buttons for a plugin with update', async () => { - const { queryByRole, queryByText } = renderPluginDetails({ id, isInstalled: true, hasUpdate: true }); + const { queryByRole, findByRole } = renderPluginDetails({ id, isInstalled: true, hasUpdate: true }); - await waitFor(() => expect(queryByText(PluginTabLabels.OVERVIEW)).toBeInTheDocument()); + expect(await findByRole('tab', { name: `Tab ${PluginTabLabels.OVERVIEW}` })).toBeInTheDocument(); expect(queryByRole('button', { name: /update/i })).not.toBeInTheDocument(); expect(queryByRole('button', { name: /uninstall/i })).not.toBeInTheDocument(); }); it('should not display an install button for enterprise plugins if license is valid', async () => { - config.licenseInfo.enabledFeatures = { 'enterprise.plugins': true }; - const { queryByRole, queryByText } = renderPluginDetails({ id, isInstalled: false, isEnterprise: true }); + const { findByRole, queryByRole } = renderPluginDetails({ id, isInstalled: false, isEnterprise: true }); - await waitFor(() => expect(queryByText(PluginTabLabels.OVERVIEW)).toBeInTheDocument()); - - expect(queryByRole('button', { name: /^install/i })).not.toBeInTheDocument(); + expect(await findByRole('tab', { name: `Tab ${PluginTabLabels.OVERVIEW}` })).toBeInTheDocument(); + expect(await queryByRole('button', { name: /^install/i })).not.toBeInTheDocument(); }); }); diff --git a/public/app/features/plugins/admin/pages/PluginDetails.tsx b/public/app/features/plugins/admin/pages/PluginDetails.tsx index 562c02594a9..219ece99780 100644 --- a/public/app/features/plugins/admin/pages/PluginDetails.tsx +++ b/public/app/features/plugins/admin/pages/PluginDetails.tsx @@ -1,10 +1,8 @@ import { css } from '@emotion/css'; -import React, { useEffect } from 'react'; -import { usePrevious } from 'react-use'; +import React from 'react'; import { GrafanaTheme2 } from '@grafana/data'; -import { locationService } from '@grafana/runtime'; -import { useStyles2, TabsBar, TabContent, Tab, Alert, toIconName } from '@grafana/ui'; +import { useStyles2, TabContent, Alert } from '@grafana/ui'; import { Layout } from '@grafana/ui/src/components/Layout/Layout'; import { Page } from 'app/core/components/Page/Page'; import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; @@ -13,11 +11,10 @@ import { AppNotificationSeverity } from 'app/types'; import { Loader } from '../components/Loader'; import { PluginDetailsBody } from '../components/PluginDetailsBody'; import { PluginDetailsDisabledError } from '../components/PluginDetailsDisabledError'; -import { PluginDetailsHeader } from '../components/PluginDetailsHeader'; import { PluginDetailsSignature } from '../components/PluginDetailsSignature'; import { usePluginDetailsTabs } from '../hooks/usePluginDetailsTabs'; import { useGetSingle, useFetchStatus, useFetchDetailsStatus } from '../state/hooks'; -import { PluginTabLabels, PluginTabIds, PluginDetailsTab } from '../types'; +import { PluginTabIds } from '../types'; type Props = GrafanaRouteComponentProps<{ pluginId?: string }>; @@ -27,35 +24,16 @@ export default function PluginDetails({ match, queryParams }: Props): JSX.Elemen url, } = match; const parentUrl = url.substring(0, url.lastIndexOf('/')); - const defaultTabs: PluginDetailsTab[] = [ - { - label: PluginTabLabels.OVERVIEW, - icon: 'file-alt', - id: PluginTabIds.OVERVIEW, - href: `${url}?page=${PluginTabIds.OVERVIEW}`, - }, - ]; + const plugin = useGetSingle(pluginId); // fetches the localplugin settings - const { tabs, defaultTab } = usePluginDetailsTabs(plugin, defaultTabs); + const { navModel, activePageId } = usePluginDetailsTabs(plugin, queryParams.page as PluginTabIds); const { isLoading: isFetchLoading } = useFetchStatus(); const { isLoading: isFetchDetailsLoading } = useFetchDetailsStatus(); const styles = useStyles2(getStyles); - const prevTabs = usePrevious(tabs); - const pageId = (queryParams.page as PluginTabIds) || defaultTab; - - // If an app plugin is uninstalled we need to reset the active tab when the config / dashboards tabs are removed. - useEffect(() => { - const hasUninstalledWithConfigPages = prevTabs && prevTabs.length > tabs.length; - const isViewingAConfigPage = pageId !== PluginTabIds.OVERVIEW && pageId !== PluginTabIds.VERSIONS; - - if (hasUninstalledWithConfigPages && isViewingAConfigPage) { - locationService.replace(`${url}?page=${PluginTabIds.OVERVIEW}`); - } - }, [pageId, url, tabs, prevTabs]); if (isFetchLoading || isFetchDetailsLoading) { return ( - + ); @@ -73,32 +51,12 @@ export default function PluginDetails({ match, queryParams }: Props): JSX.Elemen } return ( - - - {/* Tab navigation */} -
-
- - {tabs.map((tab: PluginDetailsTab) => { - return ( - - ); - })} - -
-
+ - {/* Active tab */} - + @@ -108,8 +66,7 @@ export default function PluginDetails({ match, queryParams }: Props): JSX.Elemen export const getStyles = (theme: GrafanaTheme2) => { return { alert: css` - margin: ${theme.spacing(3)}; - margin-bottom: 0; + margin-bottom: ${theme.spacing(2)}; `, // Needed due to block formatting context tabContent: css` diff --git a/public/app/features/plugins/admin/types.ts b/public/app/features/plugins/admin/types.ts index fe400db1ab6..708464557ec 100644 --- a/public/app/features/plugins/admin/types.ts +++ b/public/app/features/plugins/admin/types.ts @@ -242,7 +242,7 @@ export type RequestInfo = { export type PluginDetailsTab = { label: PluginTabLabels | string; - icon?: IconName | string; + icon?: IconName; id: PluginTabIds | string; href?: string; };