From 4c049a1a135994c89cbfbb8181984ebc4bcffd67 Mon Sep 17 00:00:00 2001 From: Juan Lulkin Date: Tue, 23 Feb 2021 18:00:55 +0100 Subject: [PATCH 01/20] Allows trigger button to render without pager duty key --- .changeset/sour-insects-marry.md | 5 + .../components/TriggerButton/index.test.tsx | 149 ++++++++++++++++++ .../src/components/TriggerButton/index.tsx | 33 ++-- .../TriggerDialog/TriggerDialog.test.tsx | 26 ++- .../TriggerDialog/TriggerDialog.tsx | 7 +- 5 files changed, 189 insertions(+), 31 deletions(-) create mode 100644 .changeset/sour-insects-marry.md create mode 100644 plugins/pagerduty/src/components/TriggerButton/index.test.tsx diff --git a/.changeset/sour-insects-marry.md b/.changeset/sour-insects-marry.md new file mode 100644 index 0000000000..f52f7fd8be --- /dev/null +++ b/.changeset/sour-insects-marry.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-pagerduty': patch +--- + +Allows the TriggerButton component to render when key is missing diff --git a/plugins/pagerduty/src/components/TriggerButton/index.test.tsx b/plugins/pagerduty/src/components/TriggerButton/index.test.tsx new file mode 100644 index 0000000000..390c8143a5 --- /dev/null +++ b/plugins/pagerduty/src/components/TriggerButton/index.test.tsx @@ -0,0 +1,149 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React from 'react'; +import { act, fireEvent, waitFor } from '@testing-library/react'; +import { renderInTestApp } from '@backstage/test-utils'; +import { + ApiRegistry, + alertApiRef, + createApiRef, + ApiProvider, + IdentityApi, + identityApiRef, +} from '@backstage/core'; +import { pagerDutyApiRef } from '../../api'; +import { Entity } from '@backstage/catalog-model'; +import { EntityProvider } from '@backstage/plugin-catalog-react'; +import { TriggerButton } from './'; + +describe('TriggerButton', () => { + const mockIdentityApi: Partial = { + getUserId: () => 'guest@example.com', + }; + + const mockTriggerAlarmFn = jest.fn(); + const mockPagerDutyApi = { + triggerAlarm: mockTriggerAlarmFn, + }; + + const apis = ApiRegistry.from([ + [ + alertApiRef, + createApiRef({ + id: 'core.alert', + description: 'Used to report alerts and forward them to the app', + }), + ], + [identityApiRef, mockIdentityApi], + [pagerDutyApiRef, mockPagerDutyApi], + ]); + + it('renders the trigger button, opens and closes dialog', async () => { + const entity: Entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'pagerduty-test', + annotations: { + 'pagerduty.com/integration-key': 'abc123', + }, + }, + }; + + const { queryByRole, getByRole, getByTestId } = await renderInTestApp( + + + + + , + ); + + expect(getByTestId('trigger-button')).toBeInTheDocument(); + expect(queryByRole('dialog')).not.toBeInTheDocument(); + + const triggerButton = getByTestId('trigger-button'); + expect(triggerButton.textContent).toBe('Create Incident'); + + await act(async () => { + fireEvent.click(triggerButton); + }); + await waitFor(() => { + expect(getByRole('dialog')).toBeInTheDocument(); + }); + + const closeButton = getByTestId('close-button'); + await act(async () => { + fireEvent.click(closeButton); + }); + await waitFor(() => { + expect(queryByRole('dialog')).not.toBeInTheDocument(); + }); + }); + + it('renders the trigger button with children', async () => { + const entity: Entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'pagerduty-test', + annotations: { + 'pagerduty.com/integration-key': 'abc123', + }, + }, + }; + + const { getByTestId } = await renderInTestApp( + + + Send an alert + + , + ); + + const triggerButton = getByTestId('trigger-button'); + expect(triggerButton.textContent).toBe('Send an alert'); + }); + + it('renders a disabled trigger button if entity does not include key', async () => { + const entity: Entity = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'pagerduty-test', + }, + }; + + const { queryByRole, getByTestId } = await renderInTestApp( + + + + + , + ); + + expect(getByTestId('trigger-button')).toBeInTheDocument(); + + const triggerButton = getByTestId('trigger-button'); + expect(triggerButton.textContent).toBe('Missing integration key'); + + await act(async () => { + fireEvent.click(triggerButton); + }); + await waitFor(() => { + expect(queryByRole('dialog')).not.toBeInTheDocument(); + }); + }); +}); diff --git a/plugins/pagerduty/src/components/TriggerButton/index.tsx b/plugins/pagerduty/src/components/TriggerButton/index.tsx index 833e01d8aa..92d7c61301 100644 --- a/plugins/pagerduty/src/components/TriggerButton/index.tsx +++ b/plugins/pagerduty/src/components/TriggerButton/index.tsx @@ -13,8 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import React, { useCallback, PropsWithChildren } from 'react'; -import { createGlobalState } from 'react-use'; +import React, { useCallback, PropsWithChildren, useState } from 'react'; import { makeStyles, Button } from '@material-ui/core'; import { useEntity } from '@backstage/plugin-catalog-react'; import { BackstageTheme } from '@backstage/theme'; @@ -50,8 +49,6 @@ const useStyles = makeStyles(theme => ({ }, })); -export const useShowDialog = createGlobalState(false); - export function TriggerButton({ design, onIncidentCreated, @@ -59,7 +56,7 @@ export function TriggerButton({ }: PropsWithChildren) { const { buttonStyle, triggerAlarm } = useStyles(); const { entity } = useEntity(); - const [dialogShown = false, setDialogShown] = useShowDialog(); + const [dialogShown, setDialogShown] = useState(false); const showDialog = useCallback(() => { setDialogShown(true); @@ -68,9 +65,8 @@ export function TriggerButton({ setDialogShown(false); }, [setDialogShown]); - const integrationKey = entity.metadata.annotations![ - PAGERDUTY_INTEGRATION_KEY - ]; + const integrationKey = + entity.metadata.annotations?.[PAGERDUTY_INTEGRATION_KEY]; return ( <> @@ -79,16 +75,21 @@ export function TriggerButton({ {...(design === 'link' && { color: 'secondary' })} onClick={showDialog} className={design === 'link' ? triggerAlarm : buttonStyle} + disabled={!integrationKey} > - {children ?? 'Create Incident'} + {integrationKey + ? children ?? 'Create Incident' + : 'Missing integration key'} - + {integrationKey && ( + + )} ); } diff --git a/plugins/pagerduty/src/components/TriggerDialog/TriggerDialog.test.tsx b/plugins/pagerduty/src/components/TriggerDialog/TriggerDialog.test.tsx index 8073ab44b5..863061ca5f 100644 --- a/plugins/pagerduty/src/components/TriggerDialog/TriggerDialog.test.tsx +++ b/plugins/pagerduty/src/components/TriggerDialog/TriggerDialog.test.tsx @@ -14,8 +14,8 @@ * limitations under the License. */ import React from 'react'; -import { render, fireEvent, act } from '@testing-library/react'; -import { wrapInTestApp } from '@backstage/test-utils'; +import { fireEvent, act } from '@testing-library/react'; +import { renderInTestApp } from '@backstage/test-utils'; import { ApiRegistry, alertApiRef, @@ -62,18 +62,16 @@ describe('TriggerDialog', () => { }, }; - const { getByText, getByRole, getByTestId } = render( - wrapInTestApp( - - {}} - name={entity.metadata.name} - integrationKey="abc123" - onIncidentCreated={() => {}} - /> - , - ), + const { getByText, getByRole, getByTestId } = await renderInTestApp( + + {}} + name={entity.metadata.name} + integrationKey="abc123" + onIncidentCreated={() => {}} + /> + , ); expect(getByRole('dialog')).toBeInTheDocument(); diff --git a/plugins/pagerduty/src/components/TriggerDialog/TriggerDialog.tsx b/plugins/pagerduty/src/components/TriggerDialog/TriggerDialog.tsx index c2084a41ed..34c3b5a2f7 100644 --- a/plugins/pagerduty/src/components/TriggerDialog/TriggerDialog.tsx +++ b/plugins/pagerduty/src/components/TriggerDialog/TriggerDialog.tsx @@ -140,7 +140,12 @@ export const TriggerDialog = ({ > Trigger Incident - From 10e78b3c58d8394b592476ca44e3c23d57959caa Mon Sep 17 00:00:00 2001 From: Juan Lulkin Date: Wed, 24 Feb 2021 13:55:21 +0100 Subject: [PATCH 02/20] Allows HeaderIconLinkRow to handle onClicks --- .../HeaderIconLinkRow/IconLinkVertical.tsx | 39 ++++++------- .../src/components/PagerDutyCard.tsx | 56 ++++++++++++------- 2 files changed, 55 insertions(+), 40 deletions(-) diff --git a/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx b/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx index 7a9078c3e4..436af62807 100644 --- a/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx +++ b/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx @@ -20,11 +20,13 @@ import LinkIcon from '@material-ui/icons/Link'; import { Link as RouterLink } from '../Link'; export type IconLinkVerticalProps = { + key?: string; icon?: React.ReactNode; href?: string; + onClick?: React.AnchorHTMLAttributes['onClick']; disabled?: boolean; label: string; - action?: React.ReactNode; + color?: 'primary' | 'secondary'; }; const useIconStyles = makeStyles(theme => ({ @@ -37,23 +39,27 @@ const useIconStyles = makeStyles(theme => ({ disabled: { color: 'gray', }, + primary: { + color: theme.palette.primary.main, + }, + secondary: { + color: theme.palette.secondary.main, + }, label: { fontSize: '0.7rem', textTransform: 'uppercase', fontWeight: 600, letterSpacing: 1.2, }, - linkStyle: { - color: theme.palette.secondary.main, - }, })); export function IconLinkVertical({ icon = , href = '#', disabled = false, - action, - ...props + color = 'primary', + label, + onClick, }: IconLinkVerticalProps) { const classes = useIconStyles(); @@ -62,27 +68,22 @@ export function IconLinkVertical({ {icon} - {props.label} - - ); - } - - if (action) { - return ( - - {icon} - {action} + {label} ); } return ( - + {icon} - {props.label} + {label} ); } diff --git a/plugins/pagerduty/src/components/PagerDutyCard.tsx b/plugins/pagerduty/src/components/PagerDutyCard.tsx index 7ec425a062..a5ebd70db6 100644 --- a/plugins/pagerduty/src/components/PagerDutyCard.tsx +++ b/plugins/pagerduty/src/components/PagerDutyCard.tsx @@ -27,7 +27,7 @@ import AlarmAddIcon from '@material-ui/icons/AlarmAdd'; import { MissingTokenError } from './Errors/MissingTokenError'; import WebIcon from '@material-ui/icons/Web'; import { PAGERDUTY_INTEGRATION_KEY } from './constants'; -import { TriggerButton, useShowDialog } from './TriggerButton'; +import { TriggerDialog } from './TriggerDialog'; export const isPluginApplicableToEntity = (entity: Entity) => Boolean(entity.metadata.annotations?.[PAGERDUTY_INTEGRATION_KEY]); @@ -36,14 +36,18 @@ export const PagerDutyCard = () => { const { entity } = useEntity(); const api = useApi(pagerDutyApiRef); const [refreshIncidents, setRefreshIncidents] = useState(false); + const [dialogShown, setDialogShown] = useState(false); + + const showDialog = useCallback(() => { + setDialogShown(true); + }, [setDialogShown]); + const hideDialog = useCallback(() => { + setDialogShown(false); + }, [setDialogShown]); + const integrationKey = entity.metadata.annotations![ PAGERDUTY_INTEGRATION_KEY ]; - const setShowDialog = useShowDialog()[1]; - - const showDialog = useCallback(() => { - setShowDialog(true); - }, [setShowDialog]); const handleRefresh = useCallback(() => { setRefreshIncidents(x => !x); @@ -84,24 +88,34 @@ export const PagerDutyCard = () => { const triggerLink = { label: 'Create Incident', - action: , - icon: , + onClick: showDialog, + icon: , + color: 'secondary' as 'secondary', // DUH }; return ( - - } - /> - - - + + } /> - - - + + + + + + + + ); }; From 0f8e1548efb27410ac02ea07c86a6b2c8ecf64ef Mon Sep 17 00:00:00 2001 From: Juan Lulkin Date: Wed, 24 Feb 2021 14:32:27 +0100 Subject: [PATCH 03/20] Adds testId to IconLinkVertical --- .../HeaderIconLinkRow/IconLinkVertical.tsx | 8 ++++-- .../src/components/HeaderIconLinkRow/index.ts | 2 ++ .../src/components/PagerDutyCard.test.tsx | 4 +-- .../src/components/PagerDutyCard.tsx | 17 ++++++++---- .../src/components/TriggerButton/index.tsx | 26 +++---------------- 5 files changed, 25 insertions(+), 32 deletions(-) diff --git a/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx b/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx index 436af62807..3f7b489322 100644 --- a/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx +++ b/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx @@ -20,12 +20,13 @@ import LinkIcon from '@material-ui/icons/Link'; import { Link as RouterLink } from '../Link'; export type IconLinkVerticalProps = { - key?: string; + key?: React.Key; icon?: React.ReactNode; href?: string; - onClick?: React.AnchorHTMLAttributes['onClick']; + onClick?: React.MouseEventHandler; disabled?: boolean; label: string; + testId?: string; color?: 'primary' | 'secondary'; }; @@ -59,6 +60,7 @@ export function IconLinkVertical({ disabled = false, color = 'primary', label, + testId, onClick, }: IconLinkVerticalProps) { const classes = useIconStyles(); @@ -66,6 +68,7 @@ export function IconLinkVertical({ if (disabled) { return ( @@ -77,6 +80,7 @@ export function IconLinkVertical({ return ( { await waitFor(() => !queryByTestId('progress')); expect(getByText('Service Directory')).toBeInTheDocument(); expect(getByText('Create Incident')).toBeInTheDocument(); - const triggerButton = getByTestId('trigger-button'); + const triggerLink = getByTestId('trigger-link'); await act(async () => { - fireEvent.click(triggerButton); + fireEvent.click(triggerLink); }); expect(getByRole('dialog')).toBeInTheDocument(); }); diff --git a/plugins/pagerduty/src/components/PagerDutyCard.tsx b/plugins/pagerduty/src/components/PagerDutyCard.tsx index a5ebd70db6..d9dad09622 100644 --- a/plugins/pagerduty/src/components/PagerDutyCard.tsx +++ b/plugins/pagerduty/src/components/PagerDutyCard.tsx @@ -14,7 +14,12 @@ * limitations under the License. */ import React, { useState, useCallback } from 'react'; -import { useApi, Progress, HeaderIconLinkRow } from '@backstage/core'; +import { + useApi, + Progress, + HeaderIconLinkRow, + IconLinkVerticalProps, +} from '@backstage/core'; import { Entity } from '@backstage/catalog-model'; import { useEntity } from '@backstage/plugin-catalog-react'; import { Card, CardHeader, Divider, CardContent } from '@material-ui/core'; @@ -80,22 +85,23 @@ export const PagerDutyCard = () => { return ; } - const serviceLink = { + const serviceLink: IconLinkVerticalProps = { label: 'Service Directory', href: service!.url, icon: , }; - const triggerLink = { + const triggerLink: IconLinkVerticalProps = { label: 'Create Incident', onClick: showDialog, icon: , - color: 'secondary' as 'secondary', // DUH + color: 'secondary', + testId: 'trigger-link', }; return ( <> - + } @@ -110,6 +116,7 @@ export const PagerDutyCard = () => { void; -} +export type TriggerButtonProps = {}; const useStyles = makeStyles(theme => ({ buttonStyle: { @@ -34,27 +31,12 @@ const useStyles = makeStyles(theme => ({ backgroundColor: theme.palette.error.dark, }, }, - triggerAlarm: { - paddingTop: 0, - paddingBottom: 0, - fontSize: '0.7rem', - textTransform: 'uppercase', - fontWeight: 600, - letterSpacing: 1.2, - lineHeight: 1.5, - '&:hover, &:focus, &.focus': { - backgroundColor: 'transparent', - textDecoration: 'none', - }, - }, })); export function TriggerButton({ - design, - onIncidentCreated, children, }: PropsWithChildren) { - const { buttonStyle, triggerAlarm } = useStyles(); + const { buttonStyle } = useStyles(); const { entity } = useEntity(); const [dialogShown, setDialogShown] = useState(false); @@ -72,9 +54,8 @@ export function TriggerButton({ <> - From 3d124a59169df04cef252edc1c35e1db8fe7a18b Mon Sep 17 00:00:00 2001 From: Juan Lulkin Date: Wed, 24 Feb 2021 23:05:27 +0100 Subject: [PATCH 11/20] Changes component in docs example to be EntityPagerDutyCard --- plugins/pagerduty/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/pagerduty/README.md b/plugins/pagerduty/README.md index 1bb14e2b2f..0e5d75d41c 100644 --- a/plugins/pagerduty/README.md +++ b/plugins/pagerduty/README.md @@ -39,7 +39,7 @@ import { { isPagerDutyAvailable(entity) && ( - + ); } From a47bb1e75682521ef08e492de5d3bbc99e80887c Mon Sep 17 00:00:00 2001 From: Juan Lulkin Date: Thu, 25 Feb 2021 10:02:29 +0100 Subject: [PATCH 12/20] Removes testId on IconLinkVerical --- .../src/components/HeaderIconLinkRow/IconLinkVertical.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx b/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx index 6d692c1ecb..3ea7981aa5 100644 --- a/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx +++ b/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx @@ -25,7 +25,6 @@ export type IconLinkVerticalProps = { onClick?: React.MouseEventHandler; disabled?: boolean; label: string; - testId?: string; color?: 'primary' | 'secondary'; }; @@ -59,7 +58,6 @@ export function IconLinkVertical({ disabled = false, color = 'primary', label, - testId, onClick, }: IconLinkVerticalProps) { const classes = useIconStyles(); @@ -67,7 +65,6 @@ export function IconLinkVertical({ if (disabled) { return ( @@ -79,7 +76,6 @@ export function IconLinkVertical({ return ( Date: Thu, 25 Feb 2021 11:33:50 +0100 Subject: [PATCH 13/20] Updates AboutCard to match new IconLinkVertical contract --- .../HeaderIconLinkRow/IconLinkVertical.tsx | 20 +++--- .../components/AboutCard/AboutCard.test.tsx | 66 ++++++++++++------- .../src/components/AboutCard/AboutCard.tsx | 11 ++-- 3 files changed, 62 insertions(+), 35 deletions(-) diff --git a/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx b/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx index 3ea7981aa5..c3c817362a 100644 --- a/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx +++ b/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx @@ -20,12 +20,13 @@ import LinkIcon from '@material-ui/icons/Link'; import { Link as RouterLink } from '../Link'; export type IconLinkVerticalProps = { - icon?: React.ReactNode; - href?: string; - onClick?: React.MouseEventHandler; - disabled?: boolean; - label: string; color?: 'primary' | 'secondary'; + disabled?: boolean; + href?: string; + icon?: React.ReactNode; + label: string; + onClick?: React.MouseEventHandler; + title?: string; }; const useIconStyles = makeStyles(theme => ({ @@ -53,18 +54,20 @@ const useIconStyles = makeStyles(theme => ({ })); export function IconLinkVertical({ - icon = , - href = '#', - disabled = false, color = 'primary', + disabled = false, + href = '#', + icon = , label, onClick, + title, }: IconLinkVerticalProps) { const classes = useIconStyles(); if (disabled) { return ( @@ -76,6 +79,7 @@ export function IconLinkVertical({ return ( GitHub', () => { - it('renders info and "view source" link', () => { + it('renders info and "view source" link', async () => { const entity = { apiVersion: 'v1', kind: 'Component', @@ -41,7 +41,7 @@ describe(' GitHub', () => { lifecycle: 'production', }, }; - const { getByText } = render( + const { getByText, getByTitle } = render( , @@ -51,15 +51,21 @@ describe(' GitHub', () => { 'href', 'https://github.com/backstage/backstage/blob/master/software.yaml', ); - expect(getByText('View Source').closest('a')).toHaveAttribute( - 'edithref', - 'https://github.com/backstage/backstage/edit/master/software.yaml', + + const editButton = getByTitle('Edit Metadata'); + window.open = jest.fn(); + await act(async () => { + fireEvent.click(editButton); + }); + expect(window.open).toHaveBeenCalledWith( + `https://github.com/backstage/backstage/edit/master/software.yaml`, + '_blank', ); }); }); describe(' GitLab', () => { - it('renders info and "view source" link', () => { + it('renders info and "view source" link', async () => { const entity = { apiVersion: 'v1', kind: 'Component', @@ -76,25 +82,32 @@ describe(' GitLab', () => { lifecycle: 'production', }, }; - const { getByText } = render( + const { getByText, getByTitle } = render( , ); + expect(getByText('service')).toBeInTheDocument(); expect(getByText('View Source').closest('a')).toHaveAttribute( 'href', 'https://gitlab.com/backstage/backstage/-/blob/master/software.yaml', ); - expect(getByText('View Source').closest('a')).toHaveAttribute( - 'edithref', - 'https://gitlab.com/backstage/backstage/-/edit/master/software.yaml', + + const editButton = getByTitle('Edit Metadata'); + window.open = jest.fn(); + await act(async () => { + fireEvent.click(editButton); + }); + expect(window.open).toHaveBeenCalledWith( + `https://gitlab.com/backstage/backstage/-/edit/master/software.yaml`, + '_blank', ); }); }); describe(' BitBucket', () => { - it('renders info and "view source" link', () => { + it('renders info and "view source" link', async () => { const entity = { apiVersion: 'v1', kind: 'Component', @@ -111,7 +124,7 @@ describe(' BitBucket', () => { lifecycle: 'production', }, }; - const { getByText } = render( + const { getByText, getByTitle } = render( , @@ -121,15 +134,21 @@ describe(' BitBucket', () => { 'href', 'https://bitbucket.org/backstage/backstage/src/master/software.yaml', ); - expect(getByText('View Source').closest('a')).toHaveAttribute( - 'edithref', - 'https://bitbucket.org/backstage/backstage/src/master/software.yaml?mode=edit&spa=0&at=master', + + const editButton = getByTitle('Edit Metadata'); + window.open = jest.fn(); + await act(async () => { + fireEvent.click(editButton); + }); + expect(window.open).toHaveBeenCalledWith( + `https://bitbucket.org/backstage/backstage/src/master/software.yaml?mode=edit&spa=0&at=master`, + '_blank', ); }); }); describe(' custom links', () => { - it('renders info and "view source" link', () => { + it('renders info and "view source" link', async () => { const entity = { apiVersion: 'v1', kind: 'Component', @@ -149,7 +168,7 @@ describe(' custom links', () => { lifecycle: 'production', }, }; - const { getByText } = render( + const { getByText, getByTitle } = render( , @@ -159,9 +178,12 @@ describe(' custom links', () => { 'href', 'https://another.place/backstage.git', ); - expect(getByText('View Source').closest('a')).toHaveAttribute( - 'edithref', - 'https://another.place', - ); + + const editButton = getByTitle('Edit Metadata'); + window.open = jest.fn(); + await act(async () => { + fireEvent.click(editButton); + }); + expect(window.open).toHaveBeenCalledWith(`https://another.place`, '_blank'); }); }); diff --git a/plugins/catalog/src/components/AboutCard/AboutCard.tsx b/plugins/catalog/src/components/AboutCard/AboutCard.tsx index 64564f5ec4..ff411f80d4 100644 --- a/plugins/catalog/src/components/AboutCard/AboutCard.tsx +++ b/plugins/catalog/src/components/AboutCard/AboutCard.tsx @@ -21,7 +21,7 @@ import { SOURCE_LOCATION_ANNOTATION, RELATION_PROVIDES_API, } from '@backstage/catalog-model'; -import { HeaderIconLinkRow } from '@backstage/core'; +import { HeaderIconLinkRow, IconLinkVerticalProps } from '@backstage/core'; import { useEntity } from '@backstage/plugin-catalog-react'; import { Card, @@ -105,11 +105,12 @@ export function AboutCard({ variant }: AboutCardProps) { const codeLink = getCodeLinkInfo(entity); // TODO: Also support RELATION_CONSUMES_API here const hasApis = entity.relations?.some(r => r.type === RELATION_PROVIDES_API); - const viewInSource = { + const viewInSource: IconLinkVerticalProps = { label: 'View Source', - ...codeLink, + href: codeLink.href, + icon: codeLink.icon, }; - const viewInTechDocs = { + const viewInTechDocs: IconLinkVerticalProps = { label: 'View TechDocs', disabled: !entity.metadata.annotations?.['backstage.io/techdocs-ref'], icon: , @@ -117,7 +118,7 @@ export function AboutCard({ variant }: AboutCardProps) { entity.kind }/${entity.metadata.name}`, }; - const viewApi = { + const viewApi: IconLinkVerticalProps = { title: hasApis ? '' : 'No APIs available', label: 'View API', disabled: !hasApis, From 1a074cfe5e00da81c916282cca7371ff034cac67 Mon Sep 17 00:00:00 2001 From: Juan Lulkin Date: Thu, 25 Feb 2021 13:37:02 +0100 Subject: [PATCH 14/20] Makes SplunkOnCallCard comply with new IconLinkVertical APIs --- .../HeaderIconLinkRow/IconLinkVertical.tsx | 3 +++ .../src/components/SplunkOnCallCard.test.tsx | 3 ++- .../src/components/SplunkOnCallCard.tsx | 17 +++++------------ 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx b/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx index c3c817362a..b3e665820a 100644 --- a/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx +++ b/packages/core/src/components/HeaderIconLinkRow/IconLinkVertical.tsx @@ -35,6 +35,9 @@ const useIconStyles = makeStyles(theme => ({ justifyItems: 'center', gridGap: 4, textAlign: 'center', + '&:active': { + cursor: 'grabbing', + }, }, disabled: { color: 'gray', diff --git a/plugins/splunk-on-call/src/components/SplunkOnCallCard.test.tsx b/plugins/splunk-on-call/src/components/SplunkOnCallCard.test.tsx index b1280ab21c..8a32dc58b2 100644 --- a/plugins/splunk-on-call/src/components/SplunkOnCallCard.test.tsx +++ b/plugins/splunk-on-call/src/components/SplunkOnCallCard.test.tsx @@ -132,6 +132,7 @@ describe('SplunkOnCallCard', () => { ), ).toBeInTheDocument(); }); + it('opens the dialog when trigger button is clicked', async () => { mockSplunkOnCallApi.getUsers = jest .fn() @@ -146,7 +147,7 @@ describe('SplunkOnCallCard', () => { ); await waitFor(() => !queryByTestId('progress')); expect(getByText('Create Incident')).toBeInTheDocument(); - const triggerButton = getByTestId('trigger-button'); + const triggerButton = getByText('Create Incident'); await act(async () => { fireEvent.click(triggerButton); }); diff --git a/plugins/splunk-on-call/src/components/SplunkOnCallCard.tsx b/plugins/splunk-on-call/src/components/SplunkOnCallCard.tsx index 84d94d60d3..ae6b0420b0 100644 --- a/plugins/splunk-on-call/src/components/SplunkOnCallCard.tsx +++ b/plugins/splunk-on-call/src/components/SplunkOnCallCard.tsx @@ -21,6 +21,7 @@ import { MissingAnnotationEmptyState, configApiRef, EmptyState, + IconLinkVerticalProps, } from '@backstage/core'; import { Entity } from '@backstage/catalog-model'; import { @@ -159,19 +160,11 @@ export const SplunkOnCallCard = ({ entity }: Props) => { ); }; - const triggerLink = { + const triggerLink: IconLinkVerticalProps = { label: 'Create Incident', - action: ( - - ), - icon: , + onClick: handleDialog, + color: 'secondary', + icon: , }; return ( From 8801ede1919b064ce4d7b3250e7a52da394066ba Mon Sep 17 00:00:00 2001 From: Juan Lulkin Date: Thu, 25 Feb 2021 13:40:23 +0100 Subject: [PATCH 15/20] Removes unused getByTestId --- plugins/splunk-on-call/src/components/SplunkOnCallCard.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/splunk-on-call/src/components/SplunkOnCallCard.test.tsx b/plugins/splunk-on-call/src/components/SplunkOnCallCard.test.tsx index 8a32dc58b2..0cbaab649e 100644 --- a/plugins/splunk-on-call/src/components/SplunkOnCallCard.test.tsx +++ b/plugins/splunk-on-call/src/components/SplunkOnCallCard.test.tsx @@ -138,7 +138,7 @@ describe('SplunkOnCallCard', () => { .fn() .mockImplementationOnce(async () => [MOCKED_USER]); - const { getByText, queryByTestId, getByTestId, getByRole } = render( + const { getByText, queryByTestId, getByRole } = render( wrapInTestApp( From a965a70548ae0f409df5994b76187f129fb73896 Mon Sep 17 00:00:00 2001 From: Juan Lulkin Date: Thu, 25 Feb 2021 13:47:28 +0100 Subject: [PATCH 16/20] Removes unused makeStyles --- .../src/components/SplunkOnCallCard.tsx | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/plugins/splunk-on-call/src/components/SplunkOnCallCard.tsx b/plugins/splunk-on-call/src/components/SplunkOnCallCard.tsx index ae6b0420b0..1e28be1483 100644 --- a/plugins/splunk-on-call/src/components/SplunkOnCallCard.tsx +++ b/plugins/splunk-on-call/src/components/SplunkOnCallCard.tsx @@ -25,8 +25,6 @@ import { } from '@backstage/core'; import { Entity } from '@backstage/catalog-model'; import { - Button, - makeStyles, Card, CardHeader, Divider, @@ -43,22 +41,6 @@ import { TriggerDialog } from './TriggerDialog'; import { MissingApiKeyOrApiIdError } from './Errors/MissingApiKeyOrApiIdError'; import { User } from './types'; -const useStyles = makeStyles({ - triggerAlarm: { - paddingTop: 0, - paddingBottom: 0, - fontSize: '0.7rem', - textTransform: 'uppercase', - fontWeight: 600, - letterSpacing: 1.2, - lineHeight: 1.5, - '&:hover, &:focus, &.focus': { - backgroundColor: 'transparent', - textDecoration: 'none', - }, - }, -}); - export const SPLUNK_ON_CALL_TEAM = 'splunk.com/on-call-team'; export const MissingTeamAnnotation = () => ( @@ -83,7 +65,6 @@ type Props = { }; export const SplunkOnCallCard = ({ entity }: Props) => { - const classes = useStyles(); const config = useApi(configApiRef); const api = useApi(splunkOnCallApiRef); const [showDialog, setShowDialog] = useState(false); From bf169d0320a39d60ae5a0861a3fbadd789d9a358 Mon Sep 17 00:00:00 2001 From: Juan Lulkin Date: Tue, 2 Mar 2021 16:06:07 +0100 Subject: [PATCH 17/20] Adds more explanation in the changeset about the action and onClick props --- .changeset/sour-insects-marry.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/.changeset/sour-insects-marry.md b/.changeset/sour-insects-marry.md index 9ede485869..fe35fda82e 100644 --- a/.changeset/sour-insects-marry.md +++ b/.changeset/sour-insects-marry.md @@ -6,3 +6,22 @@ Adds onClick and other props to IconLinkVertical Allows TriggerButton component to render when pager duty key is missing Refactors TriggerButton and PagerDutyCard not to have shared state +Removes the `action` prop of the IconLinkVertical component while adding `onClick`. So instead of having an action including a button with onClick, now the whole component can be clickable making it easier to implement and having a better UX. Before: + +```ts +const myLink : IconLinkVerticalProps = { + label: 'Click me', + action: