Skip to content

OCPBUGS-76558: Fix Add page getting started cards not clickable #16041

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sg00dwin wants to merge 1 commit into from
Closed

OCPBUGS-76558: Fix Add page getting started cards not clickable #16041

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 35 additions & 17 deletions frontend/packages/dev-console/src/components/add/layout/Masonry.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { ReactElement, FC } from 'react';
import { useState, Children, useRef, useEffect } from 'react';
import { useState, Children, useRef, useEffect, useCallback } from 'react';
import { css } from '@patternfly/react-styles';
import './MasonryLayout.scss';

Expand All @@ -8,12 +8,36 @@ type MasonryProps = {
children: ReactElement[];
};

interface MeasuredItemProps {
item: ReactElement;
itemKey: string;
onMeasure: (key: string, height: number) => void;
}

// Stable component definition - prevents unmount/remount cycles
const MeasuredItem: FC<MeasuredItemProps> = ({ item, itemKey, onMeasure }) => {
const measureRef = useRef<HTMLDivElement>(null);
const lastHeightRef = useRef<number>(0);

useEffect(() => {
const newHeight = measureRef.current?.getBoundingClientRect().height;
if (newHeight && lastHeightRef.current !== newHeight) {
lastHeightRef.current = newHeight;
onMeasure(itemKey, newHeight);
}
}, [itemKey, onMeasure]);
Comment on lines +17 to +28
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find Masonry usage to assess if children can resize after mount
echo "=== Masonry usages ===" 
rg -n --type=tsx --type=ts '\bMasonry\b' -C 3

echo -e "\n=== ResizeObserver availability ==="
rg -n 'ResizeObserver' -S

Repository: openshift/console

Length of output: 6356

🏁 Script executed:

#!/bin/bash
# Check the actual Masonry.tsx file
echo "=== Masonry.tsx content ==="
cat -n frontend/packages/dev-console/src/components/add/layout/Masonry.tsx | head -60

echo -e "\n=== MasonryLayout.tsx for comparison ==="
cat -n frontend/packages/dev-console/src/components/add/layout/MasonryLayout.tsx | head -60

echo -e "\n=== Check if Masonry is imported/used in MasonryLayout ==="
rg -n 'import.*Masonry' frontend/packages/dev-console/src/components/add/layout/

Repository: openshift/console

Length of output: 5464

Individual item height changes after mount won't update column balancing

MeasuredItem measures height only on mount (dependency array never triggers re-measure since onMeasure is memoized and itemKey is stable). If Add cards can change height after mount—e.g., async images, lazy-loaded content, or responsive resizing within items—column heights become stale and balancing drifts. If cards are guaranteed static, this is fine; otherwise, observe size changes.

🔧 Suggested fix using ResizeObserver 
 useEffect(() => {
-  const newHeight = measureRef.current?.getBoundingClientRect().height;
-  if (newHeight && lastHeightRef.current !== newHeight) {
-    lastHeightRef.current = newHeight;
-    onMeasure(itemKey, newHeight);
-  }
+  const element = measureRef.current;
+  if (!element) return;
+
+  const updateHeight = (height: number) => {
+    if (lastHeightRef.current !== height) {
+      lastHeightRef.current = height;
+      onMeasure(itemKey, height);
+    }
+  };
+
+  // Initial measure
+  updateHeight(element.getBoundingClientRect().height);
+
+  const observer = new ResizeObserver((entries) => {
+    const height = entries[0]?.contentRect.height ?? 0;
+    updateHeight(height);
+  });
+  observer.observe(element);
+  return () => observer.disconnect();
 }, [itemKey, onMeasure]);

ResizeObserver is already available in the codebase and used in similar measurement patterns (e.g., MasonryLayout for container resizing). Confirm whether Add items can realistically resize—if so, this is a good safeguard.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@frontend/packages/dev-console/src/components/add/layout/Masonry.tsx` around
lines 17 - 28, MeasuredItem currently only measures height once (useEffect with
[itemKey, onMeasure]) so late changes (async images, lazy content, responsive
shifts) won't update column balancing; modify MeasuredItem to observe size
changes by attaching a ResizeObserver to measureRef (use the existing
ResizeObserver utility or the pattern from MasonryLayout), compare against
lastHeightRef, and call onMeasure(itemKey, newHeight) whenever height changes;
ensure observer is cleaned up on unmount and that measureRef/null checks are
handled.


return <div ref={measureRef}>{item}</div>;
};

export const Masonry: FC<MasonryProps> = ({ columnCount, children }) => {
const [heights, setHeights] = useState<Record<string, number>>({});
const columns = columnCount || 1;
const setHeight = (key: string, height: number) => {

// Memoize setHeight to prevent useEffect re-runs in MeasuredItem
const setHeight = useCallback((key: string, height: number) => {
setHeights((old) => ({ ...old, [key]: height }));
};
}, []);
const groupedColumns = Array.from({ length: columns }, () => ({
height: 0,
items: [] as ReactElement[],
Expand All @@ -22,22 +46,16 @@ export const Masonry: FC<MasonryProps> = ({ columnCount, children }) => {
let added = false;
let allRendered = true;
Children.forEach(children, (item: ReactElement, itemIndex) => {
const MeasuredItem = () => {
const measureRef = useRef<HTMLDivElement>(null);
useEffect(() => {
const newHeight = measureRef.current.getBoundingClientRect().height;
if (heights[item.key as string] !== newHeight) {
setHeight(item.key as string, newHeight);
}
}, []);
return <div ref={measureRef}>{item}</div>;
};
// Use consistent key for both React reconciliation and height tracking
const effectiveKey = (item.key ?? itemIndex).toString();

const measuredItem = <MeasuredItem key={item.key ?? itemIndex} />;
const measuredItem = (
<MeasuredItem key={effectiveKey} item={item} itemKey={effectiveKey} onMeasure={setHeight} />
);

// Fill first row directly
if (itemIndex < columns) {
groupedColumns[itemIndex].height += heights[item.key as string] || 0;
groupedColumns[itemIndex].height += heights[effectiveKey] || 0;
groupedColumns[itemIndex].items.push(measuredItem);
return;
}
Expand All @@ -49,8 +67,8 @@ export const Masonry: FC<MasonryProps> = ({ columnCount, children }) => {
);

// Add column which height is already known
if (item.key && heights[item.key]) {
column.height += heights[item.key];
if (heights[effectiveKey]) {
column.height += heights[effectiveKey];
column.items.push(measuredItem);
return;
}
Expand Down