OCPBUGS-77054: Fix Add page issues with PF 6.4.0 and masonry layout

rhamilto · claude · rhamilto · commit 978074792db2 · 2026-02-18T17:03:51.000-05:00
This fixes multiple issues on the Add page:

1. Console accessibility errors for Card components (PF 6.4.0)
2. Add page cards not appearing on first load
3. Add page cards not clickable (PF 6.4.0)
4. Infinite re-rendering when browser DevTools is open

PatternFly 6.4.0 compatibility changes:
- Remove unnecessary isClickable/isSelectable props from GettingStartedExpandableGrid
- Add isControlled={false} to SimpleList (PF 6.4.0 changed default to true)
- Replace navigateTo with useNavigate hook and add e.preventDefault()

Masonry layout fixes:
- Move MeasuredItem component outside render function to prevent re-creation
- Add requestAnimationFrame for initial measurement timing
- Add debouncing and proper cleanup for resize handling
- Wrap components in memo for performance optimization

Co-Authored-By: Claude Sonnet 4.5 &lt;noreply@anthropic.com&gt;
diff --git a/frontend/packages/console-shared/src/components/getting-started/GettingStartedExpandableGrid.tsx b/frontend/packages/console-shared/src/components/getting-started/GettingStartedExpandableGrid.tsx
@@ -68,8 +68,6 @@ export const GettingStartedExpandableGrid: FC<GettingStartedExpandableGridProps>
       className="ocs-getting-started-expandable-grid"
       variant="secondary"
       data-test="getting-started"
-      isClickable
-      isSelectable
       isExpanded={isOpen}
     >
       <CardHeader
diff --git a/frontend/packages/dev-console/src/components/add/AddCard.tsx b/frontend/packages/dev-console/src/components/add/AddCard.tsx
@@ -1,5 +1,5 @@
 import type { ReactNode, FC } from 'react';
-import { isValidElement } from 'react';
+import { isValidElement, memo } from 'react';
 import { Card, SimpleList, Title } from '@patternfly/react-core';
 import { ResolvedExtension, AddAction } from '@console/dynamic-plugin-sdk';
 import { CodeRef } from '@console/dynamic-plugin-sdk/src/types';
@@ -16,7 +16,7 @@ type AddCardProps = {
   icon?: CodeRef<ReactNode> | string;
 };
 
-const AddCard: FC<AddCardProps> = ({ id, title, items, namespace, icon }) => {
+const AddCard: FC<AddCardProps> = memo(({ id, title, items, namespace, icon }) => {
   const isTitleFromItem: boolean = items?.length === 1 && items[0].properties.label === title;
   const actionIcon = (): JSX.Element => {
     if (typeof icon === 'string') {
@@ -46,12 +46,12 @@ const AddCard: FC<AddCardProps> = ({ id, title, items, namespace, icon }) => {
           {title}
         </Title>
       )}
-      <SimpleList>
+      <SimpleList isControlled={false}>
         {items.map((item) => (
           <AddCardItem key={item.properties.id} namespace={namespace} action={item} />
         ))}
       </SimpleList>
     </Card>
   ) : null;
-};
+});
 export default AddCard;
diff --git a/frontend/packages/dev-console/src/components/add/AddCardItem.tsx b/frontend/packages/dev-console/src/components/add/AddCardItem.tsx
@@ -1,10 +1,11 @@
-import type { FC, SyntheticEvent } from 'react';
-import { isValidElement } from 'react';
+import type { FC } from 'react';
+import { isValidElement, memo } from 'react';
 import { SimpleListItem, Title, Content } from '@patternfly/react-core';
+import { useNavigate } from 'react-router-dom-v5-compat';
 import { ResolvedExtension, AddAction } from '@console/dynamic-plugin-sdk';
 import { useToast } from '@console/shared/src';
 import { useTelemetry } from '@console/shared/src/hooks/useTelemetry';
-import { navigateTo, resolvedHref } from '../../utils/add-page-utils';
+import { resolvedHref } from '../../utils/add-page-utils';
 import { useShowAddCardItemDetails } from './hooks/useShowAddCardItemDetails';
 import './AddCardItem.scss';
 
@@ -13,69 +14,80 @@ type AddCardItemProps = {
   namespace: string;
 };
 
-const AddCardItem: FC<AddCardItemProps> = ({
-  action: {
-    properties: { id, label, icon, href, callback, description },
-  },
-  namespace,
-}) => {
-  const fireTelemetryEvent = useTelemetry();
-  const [showDetails] = useShowAddCardItemDetails();
-  const toast = useToast();
+const AddCardItem: FC<AddCardItemProps> = memo(
+  ({
+    action: {
+      properties: { id, label, icon, href, callback, description },
+    },
+    namespace,
+  }) => {
+    const navigate = useNavigate();
+    const fireTelemetryEvent = useTelemetry();
+    const [showDetails] = useShowAddCardItemDetails();
+    const toast = useToast();
 
-  const actionIcon = (): JSX.Element => {
-    if (typeof icon === 'string') {
-      return (
-        <img
-          className="odc-add-card-item__icon odc-add-card-item__img-icon"
-          src={icon}
-          alt={label}
-          aria-hidden="true"
-          data-test="add-card-icon"
-        />
-      );
-    }
-    if (typeof icon !== 'string' && isValidElement(icon)) {
-      return (
-        <span className="odc-add-card-item__icon" aria-hidden="true" data-test="add-card-icon">
-          {icon}
-        </span>
-      );
-    }
-    return null;
-  };
+    const actionIcon = (): JSX.Element => {
+      if (typeof icon === 'string') {
+        return (
+          <img
+            className="odc-add-card-item__icon odc-add-card-item__img-icon"
+            src={icon}
+            alt={label}
+            aria-hidden="true"
+            data-test="add-card-icon"
+          />
+        );
+      }
+      if (typeof icon !== 'string' && isValidElement(icon)) {
+        return (
+          <span className="odc-add-card-item__icon" aria-hidden="true" data-test="add-card-icon">
+            {icon}
+          </span>
+        );
+      }
+      return null;
+    };
+    return (
+      <SimpleListItem
+        component="a"
+        componentProps={{
+          'data-test': `item ${id}`,
+        }}
+        href={href ? resolvedHref(href, namespace) : null}
+        onClick={(e) => {
+          const isModifiedClick =
+            'metaKey' in e && (e.metaKey || e.ctrlKey || e.shiftKey || e.altKey || e.button === 1);
+          fireTelemetryEvent('Add Item Selected', {
+            id,
+            name: label,
+          });
 
-  return (
-    <SimpleListItem
-      component="a"
-      componentProps={{
-        'data-test': `item ${id}`,
-      }}
-      href={href ? resolvedHref(href, namespace) : null}
-      onClick={(e: SyntheticEvent) => {
-        fireTelemetryEvent('Add Item Selected', {
-          id,
-          name: label,
-        });
-        if (href) {
-          navigateTo(e, resolvedHref(href, namespace));
-        } else if (callback) {
-          callback({ namespace, toast });
-        }
-      }}
-      className="odc-add-card-item"
-    >
-      <Title headingLevel="h3" size="md" className="odc-add-card-item__title" data-test="title">
-        {actionIcon()}
-        {label}
-      </Title>
-      {showDetails && (
-        <Content component="p" className="odc-add-card-item__description" data-test="description">
-          {description}
-        </Content>
-      )}
-    </SimpleListItem>
-  );
-};
+          // Allow modified clicks to open in new tab/window
+          if (href && isModifiedClick) {
+            return;
+          }
+
+          e.preventDefault();
+          if (href) {
+            navigate(resolvedHref(href, namespace));
+          } else if (callback) {
+            callback({ namespace, toast });
+          }
+        }}
+        className="odc-add-card-item"
+      >
+        <Title headingLevel="h3" size="md" className="odc-add-card-item__title" data-test="title">
+          {actionIcon()}
+          {label}
+        </Title>
+        {showDetails && (
+          <Content component="p" className="odc-add-card-item__description" data-test="description">
+            {description}
+          </Content>
+        )}
+      </SimpleListItem>
+    );
+  },
+);
 
 export default AddCardItem;
diff --git a/frontend/packages/dev-console/src/components/add/AddCardSection.tsx b/frontend/packages/dev-console/src/components/add/AddCardSection.tsx
@@ -1,4 +1,5 @@
 import type { FC, ReactElement } from 'react';
+import { useMemo } from 'react';
 import { AddActionGroup, ResolvedExtension, AddAction } from '@console/dynamic-plugin-sdk';
 import type { LoadedExtension } from '@console/dynamic-plugin-sdk/src/types';
 import { orderExtensionBasedOnInsertBeforeAndAfter } from '@console/shared/';
@@ -28,10 +29,7 @@ const AddCardSection: FC<AddCardSectionProps> = ({
   loadingFailed,
   accessCheckFailed,
 }) => {
-  if (loadingFailed || accessCheckFailed) {
-    return <AddCardSectionEmptyState accessCheckFailed={!loadingFailed && accessCheckFailed} />;
-  }
-  const getAddCards = (): ReactElement[] => {
+  const addCards = useMemo((): ReactElement[] => {
     if (!extensionsLoaded) {
       return [];
     }
@@ -44,7 +42,11 @@ const AddCardSection: FC<AddCardSectionProps> = ({
     return addGroups.map(({ id, name, items, icon }) => (
       <AddCard key={id} id={id} title={name} items={items} namespace={namespace} icon={icon} />
     ));
-  };
+  }, [extensionsLoaded, addActionExtensions, addActionGroupExtensions, namespace]);
+
+  if (loadingFailed || accessCheckFailed) {
+    return <AddCardSectionEmptyState accessCheckFailed={!loadingFailed && accessCheckFailed} />;
+  }
 
   return (
     <div data-test="add-cards">
@@ -53,7 +55,7 @@ const AddCardSection: FC<AddCardSectionProps> = ({
         loading={!extensionsLoaded}
         LoadingComponent={AddCardSectionSkeleton}
       >
-        {getAddCards()}
+        {addCards}
       </MasonryLayout>
     </div>
   );
diff --git a/frontend/packages/dev-console/src/components/add/__tests__/MasonryLayout.spec.tsx b/frontend/packages/dev-console/src/components/add/__tests__/MasonryLayout.spec.tsx
@@ -1,4 +1,4 @@
-import { act } from '@testing-library/react';
+import { act, waitFor } from '@testing-library/react';
 import { renderWithProviders } from '@console/shared/src/test-utils/unit-test-utils';
 import AddCardSectionSkeleton from '../AddCardSectionSkeleton';
 import { MasonryLayout } from '../layout/MasonryLayout';
@@ -11,7 +11,7 @@ describe('Masonry Layout', () => {
       } as DOMRect);
   };
 
-  it('should render loading component if loading is true and LoadingComponent is defined', () => {
+  it('should render loading component if loading is true and LoadingComponent is defined', async () => {
     setWidth(1400);
 
     const { container } = renderWithProviders(
@@ -24,9 +24,11 @@ describe('Masonry Layout', () => {
       </MasonryLayout>,
     );
 
-    // Should show 4 columns (Math.floor(1400 / 300))
-    const columns = container.querySelectorAll('.odc-masonry-layout__column');
-    expect(columns).toHaveLength(4);
+    // Wait for width measurement and rendering to complete
+    await waitFor(() => {
+      const columns = container.querySelectorAll('.odc-masonry-layout__column');
+      expect(columns).toHaveLength(4);
+    });
 
     // Should render 4 skeleton placeholders (one per column)
     const skeletons = container.querySelectorAll(
@@ -35,7 +37,7 @@ describe('Masonry Layout', () => {
     expect(skeletons).toHaveLength(4);
   });
 
-  it('should render children if loading is false', () => {
+  it('should render children if loading is false', async () => {
     setWidth(1400);
 
     const { container } = renderWithProviders(
@@ -48,16 +50,18 @@ describe('Masonry Layout', () => {
       </MasonryLayout>,
     );
 
-    // Should show 4 columns (Math.floor(1400 / 300))
-    const columns = container.querySelectorAll('.odc-masonry-layout__column');
-    expect(columns).toHaveLength(4);
+    // Wait for width measurement and rendering to complete
+    await waitFor(() => {
+      const columns = container.querySelectorAll('.odc-masonry-layout__column');
+      expect(columns).toHaveLength(4);
+    });
 
     // Should render all children
     const children = container.querySelectorAll('div.child');
     expect(children).toHaveLength(5);
   });
 
-  it('should change columns if a resize event exceeds threshold', () => {
+  it('should change columns if a resize event exceeds threshold', async () => {
     setWidth(1200);
 
     const { container } = renderWithProviders(
@@ -70,21 +74,25 @@ describe('Masonry Layout', () => {
       </MasonryLayout>,
     );
 
-    // Should show 4 columns initially
-    let columns = container.querySelectorAll('.odc-masonry-layout__column');
-    expect(columns).toHaveLength(4);
+    // Wait for initial width measurement and rendering to complete
+    await waitFor(() => {
+      const columns = container.querySelectorAll('.odc-masonry-layout__column');
+      expect(columns).toHaveLength(4);
+    });
 
     act(() => {
       setWidth(900);
       window.dispatchEvent(new Event('resize'));
     });
 
-    // Should show 3 columns now (Math.floor(900 / 300))
-    columns = container.querySelectorAll('.odc-masonry-layout__column');
-    expect(columns).toHaveLength(3);
+    // Wait for resize to be processed and re-render to complete
+    await waitFor(() => {
+      const columns = container.querySelectorAll('.odc-masonry-layout__column');
+      expect(columns).toHaveLength(3);
+    });
   });
 
-  it('should not change columns if a resize event does not exceed threshold', () => {
+  it('should not change columns if a resize event does not exceed threshold', async () => {
     setWidth(1200);
 
     const { container } = renderWithProviders(
@@ -97,17 +105,24 @@ describe('Masonry Layout', () => {
       </MasonryLayout>,
     );
 
-    // Should show 4 columns initially
-    let columns = container.querySelectorAll('.odc-masonry-layout__column');
-    expect(columns).toHaveLength(4);
+    // Wait for initial width measurement and rendering to complete
+    await waitFor(() => {
+      const columns = container.querySelectorAll('.odc-masonry-layout__column');
+      expect(columns).toHaveLength(4);
+    });
 
     // Should still show 4 columns because new width does not exceed threshold
     act(() => {
       setWidth(1190);
       window.dispatchEvent(new Event('resize'));
     });
 
-    columns = container.querySelectorAll('.odc-masonry-layout__column');
+    // Wait for debounce timeout to ensure resize handler completes
+    await act(async () => {
+      await new Promise((resolve) => setTimeout(resolve, 150));
+    });
+
+    const columns = container.querySelectorAll('.odc-masonry-layout__column');
     expect(columns).toHaveLength(4);
   });
 });
diff --git a/frontend/packages/dev-console/src/components/add/layout/Masonry.tsx b/frontend/packages/dev-console/src/components/add/layout/Masonry.tsx
diff --git a/frontend/packages/dev-console/src/components/add/layout/MasonryLayout.tsx b/frontend/packages/dev-console/src/components/add/layout/MasonryLayout.tsx

Original file line number	Diff line number	Diff line change
`@@ -68,8 +68,6 @@ export const GettingStartedExpandableGrid: FC<GettingStartedExpandableGridProps>`
`68`	`68`	`className="ocs-getting-started-expandable-grid"`
`69`	`69`	`variant="secondary"`
`70`	`70`	`data-test="getting-started"`
`71`		`- isClickable`
`72`		`- isSelectable`
`73`	`71`	`isExpanded={isOpen}`
`74`	`72`	`>`
`75`	`73`	`<CardHeader`