Merge pull request #20777 from is343/chore/header-link-spacing

chore: fix spacing of header labels for entity layout

.changeset/famous-plums-sit.md

@@ -0,0 +1,6 @@
+---
+'@backstage/core-components': patch
+'@backstage/plugin-catalog': patch
+---
+
+Fix spacing inconsistency with links and labels in headers

packages/core-components/src/layout/HeaderLabel/HeaderLabel.test.tsx

@@ -51,4 +51,29 @@ describe('<HeaderLabel />', () => {
     expect(rendered.getByText('Value')).toBeInTheDocument();
     expect(anchor.href).toBe('http://localhost/test');
   });
+
+  it('should use a `p` tag if the provided value is a string', async () => {
+    const rendered = await renderInTestApp(
+      <HeaderLabel label="Label" value="Value" />,
+    );
+    expect(rendered.getByText('Value').tagName).toBe('P');
+  });
+
+  it('should use a `span` tag if the provided value is not a string', async () => {
+    const rendered = await renderInTestApp(
+      <HeaderLabel label="Label" value={<>Value</>} />,
+    );
+    expect(rendered.getByText('Value').tagName).toBe('SPAN');
+  });
+
+  it('should use the correct custom typography root component', async () => {
+    const rendered = await renderInTestApp(
+      <HeaderLabel
+        label="Label"
+        value="Value"
+        contentTypograpyRootComponent="tr"
+      />,
+    );
+    expect(rendered.container.querySelector('tr')).toBeInTheDocument();
+  });
 });

packages/core-components/src/layout/HeaderLabel/HeaderLabel.tsx

@@ -49,12 +49,19 @@ const useStyles = makeStyles<BackstageTheme>(
 type HeaderLabelContentProps = PropsWithChildren<{
   value: React.ReactNode;
   className: string;
+  typographyRootComponent?: keyof JSX.IntrinsicElements;
 }>;
 
-const HeaderLabelContent = ({ value, className }: HeaderLabelContentProps) => {
+const HeaderLabelContent = ({
+  value,
+  className,
+  typographyRootComponent,
+}: HeaderLabelContentProps) => {
   return (
     <Typography
-      component={typeof value === 'string' ? 'p' : 'span'}
+      component={
+        typographyRootComponent ?? (typeof value === 'string' ? 'p' : 'span')
+      }
       className={className}
     >
       {value}
@@ -65,6 +72,7 @@ const HeaderLabelContent = ({ value, className }: HeaderLabelContentProps) => {
 type HeaderLabelProps = {
   label: string;
   value?: HeaderLabelContentProps['value'];
+  contentTypograpyRootComponent?: HeaderLabelContentProps['typographyRootComponent'];
   url?: string;
 };
 
@@ -75,12 +83,13 @@ type HeaderLabelProps = {
  *
  */
 export function HeaderLabel(props: HeaderLabelProps) {
-  const { label, value, url } = props;
+  const { label, value, url, contentTypograpyRootComponent } = props;
   const classes = useStyles();
   const content = (
     <HeaderLabelContent
       className={classes.value}
       value={value || '<Unknown>'}
+      typographyRootComponent={contentTypograpyRootComponent}
     />
   );
   return (

plugins/catalog/src/components/EntityLayout/EntityLayout.test.tsx

@@ -233,4 +233,35 @@ describe('EntityLayout', () => {
     expect(screen.queryByText('tabbed-test-title-2')).not.toBeInTheDocument();
     expect(screen.getByText('tabbed-test-title-3')).toBeInTheDocument();
   });
+
+  it('renders the owner links inside `p` tags', async () => {
+    const mockTargetRef = 'my:target/ref';
+    const ownerEntity = {
+      ...mockEntity,
+      relations: [{ type: 'ownedBy', targetRef: mockTargetRef }],
+    };
+    await renderInTestApp(
+      <ApiProvider apis={mockApis}>
+        <EntityProvider entity={ownerEntity}>
+          <EntityLayout>
+            <EntityLayout.Route path="/" title="tabbed-test-title">
+              <div>tabbed-test-content</div>
+            </EntityLayout.Route>
+          </EntityLayout>
+        </EntityProvider>
+      </ApiProvider>,
+      {
+        mountedRoutes: {
+          '/catalog/:namespace/:kind/:name': entityRouteRef,
+        },
+      },
+    );
+
+    const ownerLink = screen.getByText(mockTargetRef).closest('a');
+    expect(ownerLink).toBeInTheDocument();
+    expect(ownerLink?.tagName).toBe('A');
+    const linkParent = ownerLink?.parentElement;
+    expect(linkParent).toBeInTheDocument();
+    expect(linkParent?.tagName).toBe('P');
+  });
 });

plugins/catalog/src/components/EntityLayout/EntityLayout.tsx

@@ -119,6 +119,7 @@ function EntityLabels(props: { entity: Entity }) {
       {ownedByRelations.length > 0 && (
         <HeaderLabel
           label="Owner"
+          contentTypograpyRootComponent="p"
           value={
             <EntityRefLinks
               entityRefs={ownedByRelations}