From 0f0f8bd6af3153c4203ceb515abbf553b2aca7db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 5 Jan 2023 16:09:07 +0100 Subject: [PATCH] minor updates to the entity peek-ahead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../CardActionComponents/EmailCardAction.tsx | 8 +- .../EntityCardActions.tsx | 11 +- .../CardActionComponents/GroupCardActions.tsx | 12 +- .../CardActionComponents/UserCardActions.tsx | 12 +- .../CardActionComponents/index.ts | 1 + .../EntityPeekAheadPopover.stories.tsx | 20 +-- .../EntityPeekAheadPopover.test.tsx | 21 ++-- .../EntityPeekAheadPopover.tsx | 116 ++++++++---------- 8 files changed, 81 insertions(+), 120 deletions(-) diff --git a/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/EmailCardAction.tsx b/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/EmailCardAction.tsx index 2fcb89eed1..16804426fb 100644 --- a/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/EmailCardAction.tsx +++ b/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/EmailCardAction.tsx @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { IconButton } from '@material-ui/core'; import EmailIcon from '@material-ui/icons/Email'; import React from 'react'; @@ -23,14 +24,13 @@ import { Link } from '@backstage/core-components'; * * @private */ -export const EmailCardAction = ({ email }: { email: string }) => { +export const EmailCardAction = (props: { email: string }) => { return ( diff --git a/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/EntityCardActions.tsx b/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/EntityCardActions.tsx index fc8b5f4007..3f8f67c668 100644 --- a/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/EntityCardActions.tsx +++ b/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/EntityCardActions.tsx @@ -13,12 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { entityRouteRef } from '../../../routes'; import { IconButton } from '@material-ui/core'; import InfoIcon from '@material-ui/icons/Info'; import React from 'react'; import { useRouteRef } from '@backstage/core-plugin-api'; -import { Entity } from '@backstage/catalog-model'; +import { Entity, getCompoundEntityRef } from '@backstage/catalog-model'; import { Link } from '@backstage/core-components'; /** @@ -26,7 +27,7 @@ import { Link } from '@backstage/core-components'; * * @private */ -export const EntityCardActions = ({ entity }: { entity: Entity }) => { +export const EntityCardActions = (props: { entity: Entity }) => { const entityRoute = useRouteRef(entityRouteRef); return ( @@ -34,11 +35,7 @@ export const EntityCardActions = ({ entity }: { entity: Entity }) => { component={Link} aria-label="Show" title="Show details" - to={entityRoute({ - name: entity.metadata.name, - namespace: entity.metadata.namespace || 'default', - kind: entity.kind.toLocaleLowerCase('en-US'), - })} + to={entityRoute(getCompoundEntityRef(props.entity))} > diff --git a/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/GroupCardActions.tsx b/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/GroupCardActions.tsx index fca0763703..9bc840c112 100644 --- a/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/GroupCardActions.tsx +++ b/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/GroupCardActions.tsx @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { EmailCardAction } from './EmailCardAction'; import React from 'react'; import { GroupEntity } from '@backstage/catalog-model'; @@ -22,12 +23,7 @@ import { GroupEntity } from '@backstage/catalog-model'; * * @private */ -export const GroupCardActions = ({ entity }: { entity: GroupEntity }) => { - return ( - <> - {entity.spec.profile?.email && ( - - )} - - ); +export const GroupCardActions = (props: { entity: GroupEntity }) => { + const email = props.entity.spec.profile?.email; + return email ? : null; }; diff --git a/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/UserCardActions.tsx b/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/UserCardActions.tsx index 57801497f6..cdf0eea385 100644 --- a/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/UserCardActions.tsx +++ b/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/UserCardActions.tsx @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { EmailCardAction } from './EmailCardAction'; import React from 'react'; import { UserEntity } from '@backstage/catalog-model'; @@ -22,12 +23,7 @@ import { UserEntity } from '@backstage/catalog-model'; * * @private */ -export const UserCardActions = ({ entity }: { entity: UserEntity }) => { - return ( - <> - {entity.spec.profile?.email && ( - - )} - - ); +export const UserCardActions = (props: { entity: UserEntity }) => { + const email = props.entity.spec.profile?.email; + return email ? : null; }; diff --git a/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/index.ts b/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/index.ts index 75959026f7..ba2114d73d 100644 --- a/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/index.ts +++ b/plugins/catalog-react/src/components/EntityPeekAheadPopover/CardActionComponents/index.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + export { EntityCardActions } from './EntityCardActions'; export { GroupCardActions } from './GroupCardActions'; export { UserCardActions } from './UserCardActions'; diff --git a/plugins/catalog-react/src/components/EntityPeekAheadPopover/EntityPeekAheadPopover.stories.tsx b/plugins/catalog-react/src/components/EntityPeekAheadPopover/EntityPeekAheadPopover.stories.tsx index 132042452b..bee5587ea1 100644 --- a/plugins/catalog-react/src/components/EntityPeekAheadPopover/EntityPeekAheadPopover.stories.tsx +++ b/plugins/catalog-react/src/components/EntityPeekAheadPopover/EntityPeekAheadPopover.stories.tsx @@ -33,12 +33,8 @@ import { Table, TableColumn } from '@backstage/core-components'; import { EntityRefLink } from '../EntityRefLink'; const mockCatalogApi = { - getEntityByRef: async (entityRef: CompoundEntityRef) => { - if ( - entityRef.namespace === 'default' && - entityRef.name === 'playback' && - entityRef.kind === 'component' - ) { + getEntityByRef: async (entityRef: string) => { + if (entityRef === 'component:default/playback') { return { kind: 'Component', metadata: { @@ -48,11 +44,7 @@ const mockCatalogApi = { }, }; } - if ( - entityRef.namespace === 'default' && - entityRef.name === 'fname.lname' && - entityRef.kind === 'user' - ) { + if (entityRef === 'user:default/fname.lname') { return { kind: 'User', metadata: { @@ -66,11 +58,7 @@ const mockCatalogApi = { }, }; } - if ( - entityRef.namespace === 'default' && - entityRef.name === 'slow.catalog.item' && - entityRef.kind === 'component' - ) { + if (entityRef === 'component:default/slow.catalog.item') { await new Promise(resolve => setTimeout(resolve, 3000)); return { kind: 'Component', diff --git a/plugins/catalog-react/src/components/EntityPeekAheadPopover/EntityPeekAheadPopover.test.tsx b/plugins/catalog-react/src/components/EntityPeekAheadPopover/EntityPeekAheadPopover.test.tsx index 2fc630313f..f665ca6c00 100644 --- a/plugins/catalog-react/src/components/EntityPeekAheadPopover/EntityPeekAheadPopover.test.tsx +++ b/plugins/catalog-react/src/components/EntityPeekAheadPopover/EntityPeekAheadPopover.test.tsx @@ -14,26 +14,21 @@ * limitations under the License. */ -import { fireEvent, screen } from '@testing-library/react'; +import { screen } from '@testing-library/react'; +import user from '@testing-library/user-event'; import React from 'react'; import { EntityPeekAheadPopover } from './EntityPeekAheadPopover'; import { ApiProvider } from '@backstage/core-app-api'; import { TestApiRegistry, renderInTestApp } from '@backstage/test-utils'; import { catalogApiRef } from '../../api'; -import { CompoundEntityRef, Entity } from '@backstage/catalog-model'; +import { Entity } from '@backstage/catalog-model'; import { CatalogApi } from '@backstage/catalog-client'; import { Button } from '@material-ui/core'; import { entityRouteRef } from '../../routes'; const catalogApi: Partial = { - getEntityByRef: async ( - entityRef: CompoundEntityRef, - ): Promise => { - if ( - entityRef.name === 'service1' && - entityRef.namespace === 'default' && - entityRef.kind === 'component' - ) { + getEntityByRef: async (entityRef: string): Promise => { + if (entityRef === 'component:default/service1') { return { apiVersion: '', kind: 'Component', @@ -71,14 +66,14 @@ describe('', () => { ); expect(screen.getByText('s1')).toBeInTheDocument(); expect(screen.queryByText('service1')).toBeNull(); - fireEvent.mouseOver(screen.getByTestId('popover1')); + user.hover(screen.getByTestId('popover1')); expect(await screen.findByText('service1')).toBeInTheDocument(); expect(screen.getByText('s2')).toBeInTheDocument(); expect(screen.queryByText('service2')).toBeNull(); - fireEvent.mouseOver(screen.getByTestId('popover2')); + user.hover(screen.getByTestId('popover2')); expect( - await screen.findByText('Error: service2 was not found'), + await screen.findByText('Error: component:default/service2 not found'), ).toBeInTheDocument(); }); }); diff --git a/plugins/catalog-react/src/components/EntityPeekAheadPopover/EntityPeekAheadPopover.tsx b/plugins/catalog-react/src/components/EntityPeekAheadPopover/EntityPeekAheadPopover.tsx index 05a818fa1f..dbdbe01a50 100644 --- a/plugins/catalog-react/src/components/EntityPeekAheadPopover/EntityPeekAheadPopover.tsx +++ b/plugins/catalog-react/src/components/EntityPeekAheadPopover/EntityPeekAheadPopover.tsx @@ -13,9 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import useAsyncFn from 'react-use/lib/useAsyncFn'; import { catalogApiRef } from '../../api'; -import React, { PropsWithChildren, useEffect, useState } from 'react'; +import React, { PropsWithChildren, useEffect, useMemo, useState } from 'react'; import HoverPopover from 'material-ui-popup-state/HoverPopover'; import { bindHover, @@ -33,11 +34,7 @@ import { Typography, } from '@material-ui/core'; import { useApiHolder } from '@backstage/core-plugin-api'; -import { - isGroupEntity, - isUserEntity, - parseEntityRef, -} from '@backstage/catalog-model'; +import { isGroupEntity, isUserEntity } from '@backstage/catalog-model'; import { Progress, ResponseErrorPanel } from '@backstage/core-components'; import { EntityCardActions, @@ -58,9 +55,6 @@ export type EntityPeekAheadPopoverProps = PropsWithChildren<{ const useStyles = makeStyles(() => { return { - trigger: { - display: 'inline-block', - }, popoverPaper: { width: '30em', }, @@ -90,27 +84,24 @@ export const EntityPeekAheadPopover = (props: EntityPeekAheadPopoverProps) => { variant: 'popover', popupId: 'entity-peek-ahead', }); - const compoundEntityRef = parseEntityRef(entityRef); const [isHovered, setIsHovered] = useState(false); - const debouncedHandleMouseEnter = debounce( - () => setIsHovered(true), - delayTime, + const debouncedHandleMouseEnter = useMemo( + () => debounce(() => setIsHovered(true), delayTime), + [delayTime], ); const [{ loading, error, value: entity }, load] = useAsyncFn(async () => { const catalogApi = apiHolder.get(catalogApiRef); if (catalogApi) { - const retrievedEntity = await catalogApi.getEntityByRef( - compoundEntityRef, - ); + const retrievedEntity = await catalogApi.getEntityByRef(entityRef); if (!retrievedEntity) { - throw new Error(`${compoundEntityRef.name} was not found`); + throw new Error(`${entityRef} not found`); } return retrievedEntity; } return undefined; - }, [apiHolder, compoundEntityRef]); + }, [apiHolder, entityRef]); const handleOnMouseLeave = () => { setIsHovered(false); @@ -146,61 +137,58 @@ export const EntityPeekAheadPopover = (props: EntityPeekAheadPopoverProps) => { }} onMouseLeave={handleOnMouseLeave} > - <> - {error && } - + + + {error && } {loading && } - - - {entity && ( - <> - - {compoundEntityRef.namespace} - - - {compoundEntityRef.name} - - {entity.kind} + {entity && ( + <> + + {entity.metadata.namespace} + + + {entity.metadata.name} + + + {entity.kind} + + {entity.metadata.description && ( {entity.metadata.description} - {entity.spec?.type} - - {(entity.metadata.tags || []) - .slice(0, maxTagChips) - .map(tag => { - return ; - })} - {entity.metadata.tags?.length && - entity.metadata.tags?.length > maxTagChips && ( - - - - )} - - - )} - - {!error && ( - - {entity && ( - <> - {isUserEntity(entity) && ( - - )} - {isGroupEntity(entity) && ( - - )} - - )} - + {entity.spec?.type} + + {(entity.metadata.tags || []) + .slice(0, maxTagChips) + .map(tag => { + return ; + })} + {entity.metadata.tags?.length && + entity.metadata.tags?.length > maxTagChips && ( + + + + )} + + )} - - + + {!error && entity && ( + + <> + {isUserEntity(entity) && } + {isGroupEntity(entity) && ( + + )} + + + + )} + )}