From d1b0e70f8de88e3e4c824613279072436c287ba6 Mon Sep 17 00:00:00 2001 From: Yulia Shanyrova Date: Wed, 14 Aug 2024 10:24:10 +0200 Subject: [PATCH] Plugins: Remove cards and list display options functionality from plugin catalogue (#91840) * remove displayMode functionality from plugin catalogue * remove test for removed functionality * fix linting --- .../plugins/admin/__mocks__/mockHelpers.ts | 13 +-- .../admin/components/PluginList.test.tsx | 79 ------------------- .../plugins/admin/components/PluginList.tsx | 14 ++-- .../admin/components/PluginListItem.test.tsx | 45 +---------- .../admin/components/PluginListItem.tsx | 41 ++-------- .../plugins/admin/pages/Browse.test.tsx | 36 --------- .../features/plugins/admin/pages/Browse.tsx | 23 +----- .../app/features/plugins/admin/state/hooks.ts | 14 +--- .../features/plugins/admin/state/reducer.ts | 13 +-- .../features/plugins/admin/state/selectors.ts | 2 - public/app/features/plugins/admin/types.ts | 8 -- 11 files changed, 18 insertions(+), 270 deletions(-) delete mode 100644 public/app/features/plugins/admin/components/PluginList.test.tsx diff --git a/public/app/features/plugins/admin/__mocks__/mockHelpers.ts b/public/app/features/plugins/admin/__mocks__/mockHelpers.ts index 096f7384dc2..6034e8860e9 100644 --- a/public/app/features/plugins/admin/__mocks__/mockHelpers.ts +++ b/public/app/features/plugins/admin/__mocks__/mockHelpers.ts @@ -2,15 +2,7 @@ import { setBackendSrv } from '@grafana/runtime'; import { API_ROOT, GCOM_API_ROOT } from '../constants'; import * as permissions from '../permissions'; -import { - CatalogPlugin, - LocalPlugin, - RemotePlugin, - Version, - ReducerState, - RequestStatus, - PluginListDisplayMode, -} from '../types'; +import { CatalogPlugin, LocalPlugin, RemotePlugin, Version, ReducerState, RequestStatus } from '../types'; import catalogPluginMock from './catalogPlugin.mock'; import localPluginMock from './localPlugin.mock'; @@ -40,9 +32,6 @@ export const getPluginsStateMock = (plugins: CatalogPlugin[] = []): ReducerState status: RequestStatus.Fulfilled, }, }, - settings: { - displayMode: PluginListDisplayMode.Grid, - }, // Backward compatibility plugins: [], errors: [], diff --git a/public/app/features/plugins/admin/components/PluginList.test.tsx b/public/app/features/plugins/admin/components/PluginList.test.tsx deleted file mode 100644 index e3fd9c61eed..00000000000 --- a/public/app/features/plugins/admin/components/PluginList.test.tsx +++ /dev/null @@ -1,79 +0,0 @@ -import { render } from '@testing-library/react'; -import { useLocation } from 'react-router-dom'; - -import { PluginSignatureStatus } from '@grafana/data'; -import { config } from '@grafana/runtime'; - -import { CatalogPlugin, PluginListDisplayMode } from '../types'; - -import { PluginList } from './PluginList'; - -jest.mock('react-router-dom', () => ({ - useLocation: jest.fn(), -})); - -jest.mock('@grafana/runtime', () => ({ - ...jest.requireActual('@grafana/runtime'), - config: { - appSubUrl: '', - }, -})); - -const useLocationMock = useLocation as jest.Mock; - -const getMockPlugin = (id: string): CatalogPlugin => { - return { - description: 'The test plugin', - downloads: 5, - id, - info: { - logos: { - small: 'https://grafana.com/api/plugins/test-plugin/versions/0.0.10/logos/small', - large: 'https://grafana.com/api/plugins/test-plugin/versions/0.0.10/logos/large', - }, - keywords: ['test', 'plugin'], - }, - name: 'Testing Plugin', - orgName: 'Test', - popularity: 0, - signature: PluginSignatureStatus.valid, - publishedAt: '2020-09-01', - updatedAt: '2021-06-28', - hasUpdate: false, - isInstalled: false, - isCore: false, - isDev: false, - isEnterprise: false, - isDisabled: false, - isDeprecated: false, - isPublished: true, - isManaged: false, - }; -}; - -const plugins = [getMockPlugin('test1'), getMockPlugin('test2'), getMockPlugin('test3')]; -describe('PluginList', () => { - beforeAll(() => { - useLocationMock.mockImplementation(() => ({ - pathname: '/plugins', - })); - }); - - it('renders a plugin list', () => { - const result = render(); - expect(result.getByTestId('plugin-list')).toBeTruthy(); - const links = result.getAllByRole('link'); - for (const link of links) { - expect(link).toHaveAttribute('href', expect.stringMatching(/^\/plugins\/test\d/)); - } - }); - it('renders a plugin list with a subAppUrl', () => { - config.appSubUrl = 'test-sub-url'; - const result = render(); - expect(result.getByTestId('plugin-list')).toBeTruthy(); - const links = result.getAllByRole('link'); - for (const link of links) { - expect(link).toHaveAttribute('href', expect.stringMatching(/^test-sub-url\/plugins\/test\d/)); - } - }); -}); diff --git a/public/app/features/plugins/admin/components/PluginList.tsx b/public/app/features/plugins/admin/components/PluginList.tsx index a026047fe84..2650e3abee2 100644 --- a/public/app/features/plugins/admin/components/PluginList.tsx +++ b/public/app/features/plugins/admin/components/PluginList.tsx @@ -4,18 +4,16 @@ import { config } from '@grafana/runtime'; import { EmptyState, Grid } from '@grafana/ui'; import { t } from 'app/core/internationalization'; -import { CatalogPlugin, PluginListDisplayMode } from '../types'; +import { CatalogPlugin } from '../types'; import { PluginListItem } from './PluginListItem'; interface Props { plugins: CatalogPlugin[]; - displayMode: PluginListDisplayMode; isLoading?: boolean; } -export const PluginList = ({ plugins, displayMode, isLoading }: Props) => { - const isList = displayMode === PluginListDisplayMode.List; +export const PluginList = ({ plugins, isLoading }: Props) => { const { pathname } = useLocation(); const pathName = config.appSubUrl + (pathname.endsWith('/') ? pathname.slice(0, -1) : pathname); @@ -24,12 +22,10 @@ export const PluginList = ({ plugins, displayMode, isLoading }: Props) => { } return ( - + {isLoading - ? new Array(50).fill(null).map((_, index) => ) - : plugins.map((plugin) => ( - - ))} + ? new Array(50).fill(null).map((_, index) => ) + : plugins.map((plugin) => )} ); }; diff --git a/public/app/features/plugins/admin/components/PluginListItem.test.tsx b/public/app/features/plugins/admin/components/PluginListItem.test.tsx index 9a609f4be84..0339c154d74 100644 --- a/public/app/features/plugins/admin/components/PluginListItem.test.tsx +++ b/public/app/features/plugins/admin/components/PluginListItem.test.tsx @@ -2,7 +2,7 @@ import { render, screen } from '@testing-library/react'; import { PluginErrorCode, PluginSignatureStatus, PluginType } from '@grafana/data'; -import { CatalogPlugin, PluginListDisplayMode } from '../types'; +import { CatalogPlugin } from '../types'; import { PluginListItem } from './PluginListItem'; @@ -102,47 +102,4 @@ describe('PluginListItem', () => { expect(screen.getByText(/disabled/i)).toBeVisible(); }); - - /** As List */ - it('renders a row with link, image, name, orgName and badges', () => { - render(); - - expect(screen.getByRole('link')).toHaveAttribute('href', '/plugins/test-plugin'); - - const logo = screen.getByRole('presentation'); - expect(logo).toHaveAttribute('src', plugin.info.logos.small); - - expect(screen.getByRole('heading', { name: /testing plugin/i })).toBeVisible(); - expect(screen.getByText(`By ${plugin.orgName}`)).toBeVisible(); - expect(screen.getByText(/signed/i)).toBeVisible(); - expect(screen.queryByLabelText(/icon/i)).not.toBeInTheDocument(); - }); - - it('renders a datasource plugin with correct icon', () => { - const datasourcePlugin = { ...plugin, type: PluginType.datasource }; - render(); - - expect(screen.getByTitle(/datasource plugin/i)).toBeInTheDocument(); - }); - - it('renders a panel plugin with correct icon', () => { - const panelPlugin = { ...plugin, type: PluginType.panel }; - render(); - - expect(screen.getByTitle(/panel plugin/i)).toBeInTheDocument(); - }); - - it('renders an app plugin with correct icon', () => { - const appPlugin = { ...plugin, type: PluginType.app }; - render(); - - expect(screen.getByTitle(/app plugin/i)).toBeInTheDocument(); - }); - - it('renders a disabled plugin with a badge to indicate its error', () => { - const pluginWithError = { ...plugin, isDisabled: true, error: PluginErrorCode.modifiedSignature }; - render(); - - expect(screen.getByText(/disabled/i)).toBeVisible(); - }); }); diff --git a/public/app/features/plugins/admin/components/PluginListItem.tsx b/public/app/features/plugins/admin/components/PluginListItem.tsx index d54ea5aea3b..252ab24be13 100644 --- a/public/app/features/plugins/admin/components/PluginListItem.tsx +++ b/public/app/features/plugins/admin/components/PluginListItem.tsx @@ -6,7 +6,7 @@ import { locationService, reportInteraction } from '@grafana/runtime'; import { Badge, Icon, Stack, useStyles2 } from '@grafana/ui'; import { SkeletonComponent, attachSkeleton } from '@grafana/ui/src/unstable'; -import { CatalogPlugin, PluginIconName, PluginListDisplayMode } from '../types'; +import { CatalogPlugin, PluginIconName } from '../types'; import { PluginListItemBadges } from './PluginListItemBadges'; import { PluginLogo } from './PluginLogo'; @@ -16,12 +16,10 @@ export const LOGO_SIZE = '48px'; type Props = { plugin: CatalogPlugin; pathName: string; - displayMode?: PluginListDisplayMode; }; -function PluginListItemComponent({ plugin, pathName, displayMode = PluginListDisplayMode.Grid }: Props) { +function PluginListItemComponent({ plugin, pathName }: Props) { const styles = useStyles2(getStyles); - const isList = displayMode === PluginListDisplayMode.List; const reportUserClickInteraction = () => { if (locationService.getSearchObject()?.q) { @@ -29,11 +27,7 @@ function PluginListItemComponent({ plugin, pathName, displayMode = PluginListDis } }; return ( - +

{plugin.name}

@@ -47,15 +41,11 @@ function PluginListItemComponent({ plugin, pathName, displayMode = PluginListDis ); } -const PluginListItemSkeleton: SkeletonComponent> = ({ - displayMode = PluginListDisplayMode.Grid, - rootProps, -}) => { +const PluginListItemSkeleton: SkeletonComponent = ({ rootProps }) => { const styles = useStyles2(getStyles); - const isList = displayMode === PluginListDisplayMode.List; return ( -
+
{ background: theme.colors.emphasize(theme.colors.background.secondary, 0.03), }, }), - list: css({ - rowGap: 0, - - '> img': { - alignSelf: 'start', - }, - - '> .plugin-content': { - minHeight: 0, - gridArea: '2 / 2 / 4 / 3', - - '> p': { - margin: theme.spacing(0, 0, 0.5, 0), - }, - }, - - '> .plugin-name': { - alignSelf: 'center', - gridArea: '1 / 2 / 2 / 3', - }, - }), pluginType: css({ gridArea: '1 / 3 / 2 / 4', color: theme.colors.text.secondary, diff --git a/public/app/features/plugins/admin/pages/Browse.test.tsx b/public/app/features/plugins/admin/pages/Browse.test.tsx index 3cb936fb7c4..48ac0897243 100644 --- a/public/app/features/plugins/admin/pages/Browse.test.tsx +++ b/public/app/features/plugins/admin/pages/Browse.test.tsx @@ -1,5 +1,4 @@ import { render, RenderResult, waitFor, within } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; import { TestProvider } from 'test/helpers/TestProvider'; import { PluginType, escapeStringForRegex } from '@grafana/data'; @@ -406,39 +405,4 @@ describe('Browse list of plugins', () => { await waitFor(() => expect(getByRole('radio', { name: 'Installed' })).toBeDisabled()); }); }); - - it('should be possible to switch between display modes', async () => { - const { findByTestId, getByRole, getByTitle, queryByText } = renderBrowse('/plugins?filterBy=all', [ - getCatalogPluginMock({ id: 'plugin-1', name: 'Plugin 1' }), - getCatalogPluginMock({ id: 'plugin-2', name: 'Plugin 2' }), - getCatalogPluginMock({ id: 'plugin-3', name: 'Plugin 3' }), - ]); - - await findByTestId('plugin-list'); - - const listOptionTitle = 'Display plugins in list'; - const gridOptionTitle = 'Display plugins in a grid layout'; - const listOption = getByRole('radio', { name: listOptionTitle }); - const listOptionLabel = getByTitle(listOptionTitle); - const gridOption = getByRole('radio', { name: gridOptionTitle }); - const gridOptionLabel = getByTitle(gridOptionTitle); - - // All options should be visible - expect(listOptionLabel).toBeVisible(); - expect(gridOptionLabel).toBeVisible(); - - // The default display mode should be "grid" - expect(gridOption).toBeChecked(); - expect(listOption).not.toBeChecked(); - - // Switch to "list" view - await userEvent.click(listOption); - expect(gridOption).not.toBeChecked(); - expect(listOption).toBeChecked(); - - // All plugins are still visible - expect(queryByText('Plugin 1')).toBeInTheDocument(); - expect(queryByText('Plugin 2')).toBeInTheDocument(); - expect(queryByText('Plugin 3')).toBeInTheDocument(); - }); }); diff --git a/public/app/features/plugins/admin/pages/Browse.tsx b/public/app/features/plugins/admin/pages/Browse.tsx index 0e60bb60003..c72d702ab73 100644 --- a/public/app/features/plugins/admin/pages/Browse.tsx +++ b/public/app/features/plugins/admin/pages/Browse.tsx @@ -17,14 +17,12 @@ import { RoadmapLinks } from '../components/RoadmapLinks'; import { SearchField } from '../components/SearchField'; import { Sorters } from '../helpers'; import { useHistory } from '../hooks/useHistory'; -import { useGetAll, useIsRemotePluginsAvailable, useDisplayMode } from '../state/hooks'; -import { PluginListDisplayMode } from '../types'; +import { useGetAll, useIsRemotePluginsAvailable } from '../state/hooks'; export default function Browse({ route }: GrafanaRouteComponentProps): ReactElement | null { const location = useLocation(); const locationSearch = locationSearchToObject(location.search); const navModel = useSelector((state) => getNavModel(state.navIndex, 'plugins')); - const { displayMode, setDisplayMode } = useDisplayMode(); const styles = useStyles2(getStyles); const history = useHistory(); const remotePluginsAvailable = useIsRemotePluginsAvailable(); @@ -143,27 +141,10 @@ export default function Browse({ route }: GrafanaRouteComponentProps): ReactElem ]} /> - - {/* Display mode */} - - - className={styles.displayAs} - value={displayMode} - onChange={setDisplayMode} - options={[ - { - value: PluginListDisplayMode.Grid, - icon: 'table', - description: 'Display plugins in a grid layout', - }, - { value: PluginListDisplayMode.List, icon: 'list-ul', description: 'Display plugins in list' }, - ]} - /> -
- +
diff --git a/public/app/features/plugins/admin/state/hooks.ts b/public/app/features/plugins/admin/state/hooks.ts index c50d9548348..a1cd84b3e16 100644 --- a/public/app/features/plugins/admin/state/hooks.ts +++ b/public/app/features/plugins/admin/state/hooks.ts @@ -4,17 +4,15 @@ import { PluginError, PluginType } from '@grafana/data'; import { useDispatch, useSelector } from 'app/types'; import { sortPlugins, Sorters } from '../helpers'; -import { CatalogPlugin, PluginListDisplayMode } from '../types'; +import { CatalogPlugin } from '../types'; import { fetchAll, fetchDetails, fetchRemotePlugins, install, uninstall, fetchAllLocal, unsetInstall } from './actions'; -import { setDisplayMode } from './reducer'; import { selectPlugins, selectById, selectIsRequestPending, selectRequestError, selectIsRequestNotFetched, - selectDisplayMode, selectPluginErrors, type PluginFilters, } from './selectors'; @@ -150,13 +148,3 @@ export const useFetchDetailsLazy = () => { return (id: string) => dispatch(fetchDetails(id)); }; - -export const useDisplayMode = () => { - const dispatch = useDispatch(); - const displayMode = useSelector(selectDisplayMode); - - return { - displayMode, - setDisplayMode: (v: PluginListDisplayMode) => dispatch(setDisplayMode(v)), - }; -}; diff --git a/public/app/features/plugins/admin/state/reducer.ts b/public/app/features/plugins/admin/state/reducer.ts index 2891856ea4a..f2414a31405 100644 --- a/public/app/features/plugins/admin/state/reducer.ts +++ b/public/app/features/plugins/admin/state/reducer.ts @@ -3,7 +3,7 @@ import { createSlice, createEntityAdapter, Reducer, AnyAction, PayloadAction } f import { PanelPlugin } from '@grafana/data'; import { STATE_PREFIX } from '../constants'; -import { CatalogPlugin, PluginListDisplayMode, ReducerState, RequestStatus } from '../types'; +import { CatalogPlugin, ReducerState, RequestStatus } from '../types'; import { fetchDetails, @@ -33,9 +33,7 @@ const getOriginalActionType = (type: string) => { export const initialState: ReducerState = { items: pluginsAdapter.getInitialState(), requests: {}, - settings: { - displayMode: PluginListDisplayMode.Grid, - }, + // Backwards compatibility // (we need to have the following fields in the store as well to be backwards compatible with other parts of Grafana) // TODO @@ -51,11 +49,7 @@ export const initialState: ReducerState = { const slice = createSlice({ name: 'plugins', initialState, - reducers: { - setDisplayMode(state, action: PayloadAction) { - state.settings.displayMode = action.payload; - }, - }, + reducers: {}, extraReducers: (builder) => builder .addCase(addPlugins, (state, action: PayloadAction) => { @@ -113,5 +107,4 @@ const slice = createSlice({ }), }); -export const { setDisplayMode } = slice.actions; export const reducer: Reducer = slice.reducer; diff --git a/public/app/features/plugins/admin/state/selectors.ts b/public/app/features/plugins/admin/state/selectors.ts index db0de51035c..8ffb65ce52c 100644 --- a/public/app/features/plugins/admin/state/selectors.ts +++ b/public/app/features/plugins/admin/state/selectors.ts @@ -12,8 +12,6 @@ export const selectRoot = (state: PluginCatalogStoreState) => state.plugins; export const selectItems = createSelector(selectRoot, ({ items }) => items); -export const selectDisplayMode = createSelector(selectRoot, ({ settings }) => settings.displayMode); - export const { selectAll, selectById } = pluginsAdapter.getSelectors(selectItems); export type PluginFilters = { diff --git a/public/app/features/plugins/admin/types.ts b/public/app/features/plugins/admin/types.ts index 0991c0921bb..b8f8e3b1669 100644 --- a/public/app/features/plugins/admin/types.ts +++ b/public/app/features/plugins/admin/types.ts @@ -13,11 +13,6 @@ import { StoreState, PluginsState } from 'app/types'; export type PluginTypeCode = 'app' | 'panel' | 'datasource'; -export enum PluginListDisplayMode { - Grid = 'grid', - List = 'list', -} - export enum PluginAdminRoutes { Home = 'plugins-home', Browse = 'plugins-browse', @@ -296,9 +291,6 @@ export type PluginDetailsTab = { export type ReducerState = PluginsState & { items: EntityState; requests: Record; - settings: { - displayMode: PluginListDisplayMode; - }; }; // TODO