From 9473c5c355ad3cf1cbd6367a3c71a8ec0690ab41 Mon Sep 17 00:00:00 2001 From: Vishali Sanghishetty <95239214+vishsanghishetty@users.noreply.github.com> Date: Fri, 17 Oct 2025 08:52:58 -0400 Subject: [PATCH 1/2] feat: Add onRowClick handler for interactive table rows - Add optional onRowClick prop to TableWrapper interface - Enable click handler for drill-down interactions - Add visual feedback with cursor pointer and hover state - Backward compatible - existing tables unchanged - Uses PatternFly's isHoverable for consistent styling - Tested in OCM Genie POC with 100+ row tables This feature enables common use cases like: - Navigating to detail pages - Opening modals/drawers - Selecting items for actions - Expanding inline details --- src/components/TableWrapper.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/components/TableWrapper.tsx b/src/components/TableWrapper.tsx index c3c567f..74e4c57 100644 --- a/src/components/TableWrapper.tsx +++ b/src/components/TableWrapper.tsx @@ -23,10 +23,11 @@ interface TableWrapperProps { id: string; fields: FieldData[]; className?: string; + onRowClick?: (rowData: Record) => void; } const TableWrapper = (props: TableWrapperProps) => { - const { title, id, fields, className } = props; + const { title, id, fields, className, onRowClick } = props; // Check for missing or invalid data const hasNoFields = !fields || fields.length === 0; @@ -102,7 +103,13 @@ const TableWrapper = (props: TableWrapperProps) => { {rows.map((row, rowIndex) => ( - + onRowClick?.(row)} + style={onRowClick ? { cursor: 'pointer' } : undefined} + isHoverable={!!onRowClick} + > {columns.map((col, colIndex) => ( {row[col.key]} ))} From 837c14be0e408facad9ba30c4c03771822451dc2 Mon Sep 17 00:00:00 2001 From: Vishali Sanghishetty <95239214+vishsanghishetty@users.noreply.github.com> Date: Sun, 26 Oct 2025 14:16:15 -0400 Subject: [PATCH 2/2] test: Add comprehensive unit tests for clickable table rows feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add 14 new test cases for onRowClick functionality - Test row click handler with correct data passing - Test visual feedback (cursor pointer, hover state) - Test edge cases: null values, arrays, large tables (150 rows) - Test backward compatibility (tables without onRowClick) - Test multiple clicks and data-testid attributes - All 24 tests passing ✓ --- src/test/components/TableWrapper.test.tsx | 260 +++++++++++++++++++++- 1 file changed, 259 insertions(+), 1 deletion(-) diff --git a/src/test/components/TableWrapper.test.tsx b/src/test/components/TableWrapper.test.tsx index 52b344c..87cb222 100644 --- a/src/test/components/TableWrapper.test.tsx +++ b/src/test/components/TableWrapper.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from "@testing-library/react"; +import { render, screen, fireEvent } from "@testing-library/react"; import TableWrapper from "../../components/TableWrapper"; @@ -238,4 +238,262 @@ describe("TableWrapper Component", () => { expect(screen.getByText("123")).toBeInTheDocument(); expect(screen.getByText("true")).toBeInTheDocument(); }); + + // ========== onRowClick Feature Tests ========== + + describe("onRowClick functionality", () => { + const mockOnRowClick = vitest.fn(); + + beforeEach(() => { + mockOnRowClick.mockClear(); + }); + + it("should call onRowClick handler when row is clicked", () => { + render(); + + const firstRow = screen.getByTestId("row-0"); + fireEvent.click(firstRow); + + expect(mockOnRowClick).toHaveBeenCalledTimes(1); + expect(mockOnRowClick).toHaveBeenCalledWith({ + Title: "Toy Story", + Year: "1995", + Runtime: "81", + "IMDB Rating": "8.3", + Revenue: "373554033", + Countries: "USA", + }); + }); + + it("should call onRowClick with correct data for each row", () => { + const multiRowData = { + ...mockFieldsData, + fields: [ + { + name: "Name", + data_path: "user.name", + data: ["Alice", "Bob", "Charlie"], + }, + { + name: "Age", + data_path: "user.age", + data: [25, 30, 35], + }, + ], + }; + + render(); + + // Click second row + const secondRow = screen.getByTestId("row-1"); + fireEvent.click(secondRow); + + expect(mockOnRowClick).toHaveBeenCalledWith({ + Name: "Bob", + Age: "30", + }); + }); + + it("should apply pointer cursor style when onRowClick is provided", () => { + render(); + + const firstRow = screen.getByTestId("row-0"); + expect(firstRow).toHaveStyle({ cursor: "pointer" }); + }); + + it("should not apply pointer cursor style when onRowClick is not provided", () => { + render(); + + const firstRow = screen.getByTestId("row-0"); + expect(firstRow).not.toHaveStyle({ cursor: "pointer" }); + }); + + it("should make rows hoverable when onRowClick is provided", () => { + const { container } = render( + + ); + + const firstRow = container.querySelector('[data-testid="row-0"]'); + // Check that isHoverable prop is applied (PatternFly adds hover class) + expect(firstRow).toBeInTheDocument(); + }); + + it("should handle multiple row clicks", () => { + const multiRowData = { + ...mockFieldsData, + fields: [ + { + name: "Item", + data_path: "item", + data: ["A", "B", "C"], + }, + ], + }; + + render(); + + fireEvent.click(screen.getByTestId("row-0")); + fireEvent.click(screen.getByTestId("row-1")); + fireEvent.click(screen.getByTestId("row-2")); + + expect(mockOnRowClick).toHaveBeenCalledTimes(3); + expect(mockOnRowClick).toHaveBeenNthCalledWith(1, { Item: "A" }); + expect(mockOnRowClick).toHaveBeenNthCalledWith(2, { Item: "B" }); + expect(mockOnRowClick).toHaveBeenNthCalledWith(3, { Item: "C" }); + }); + + it("should handle row click with array data", () => { + const fieldsWithArray = { + ...mockFieldsData, + fields: [ + { + name: "Name", + data_path: "name", + data: ["Test"], + }, + { + name: "Tags", + data_path: "tags", + data: [["tag1", "tag2", "tag3"]], + }, + ], + }; + + render(); + + fireEvent.click(screen.getByTestId("row-0")); + + expect(mockOnRowClick).toHaveBeenCalledWith({ + Name: "Test", + Tags: "tag1, tag2, tag3", // Array should be joined + }); + }); + + it("should handle row click with null values", () => { + const fieldsWithNull = { + ...mockFieldsData, + fields: [ + { + name: "Field1", + data_path: "field1", + data: ["value1"], + }, + { + name: "Field2", + data_path: "field2", + data: [null], + }, + ], + }; + + render(); + + fireEvent.click(screen.getByTestId("row-0")); + + expect(mockOnRowClick).toHaveBeenCalledWith({ + Field1: "value1", + Field2: "", // null should become empty string + }); + }); + + it("should not break when onRowClick is undefined", () => { + render(); + + const firstRow = screen.getByTestId("row-0"); + + // Should not throw error when clicking without handler + expect(() => fireEvent.click(firstRow)).not.toThrow(); + }); + + it("should work with single row tables", () => { + render(); + + const row = screen.getByTestId("row-0"); + fireEvent.click(row); + + expect(mockOnRowClick).toHaveBeenCalledTimes(1); + }); + + it("should work with large tables (100+ rows)", () => { + const largeData = { + ...mockFieldsData, + fields: [ + { + name: "ID", + data_path: "id", + data: Array.from({ length: 150 }, (_, i) => i + 1), + }, + { + name: "Value", + data_path: "value", + data: Array.from({ length: 150 }, (_, i) => `value-${i + 1}`), + }, + ], + }; + + render(); + + // Click a row in the middle + fireEvent.click(screen.getByTestId("row-75")); + + expect(mockOnRowClick).toHaveBeenCalledWith({ + ID: "76", + Value: "value-76", + }); + }); + + it("should maintain backward compatibility with tables without onRowClick", () => { + const { container } = render(); + + const rows = container.querySelectorAll('[data-testid^="row-"]'); + expect(rows.length).toBeGreaterThan(0); + + // Should render normally without onRowClick + expect(screen.getByText("Toy Story")).toBeInTheDocument(); + }); + + it("should pass all row data fields to click handler", () => { + const complexData = { + ...mockFieldsData, + fields: [ + { name: "Name", data_path: "name", data: ["Test"] }, + { name: "Age", data_path: "age", data: [25] }, + { name: "City", data_path: "city", data: ["NYC"] }, + { name: "Active", data_path: "active", data: [true] }, + { name: "Score", data_path: "score", data: [95.5] }, + ], + }; + + render(); + + fireEvent.click(screen.getByTestId("row-0")); + + expect(mockOnRowClick).toHaveBeenCalledWith({ + Name: "Test", + Age: "25", + City: "NYC", + Active: "true", + Score: "95.5", + }); + }); + + it("should have data-testid on all rows", () => { + const multiRowData = { + ...mockFieldsData, + fields: [ + { + name: "Item", + data_path: "item", + data: ["A", "B", "C", "D", "E"], + }, + ], + }; + + render(); + + for (let i = 0; i < 5; i++) { + expect(screen.getByTestId(`row-${i}`)).toBeInTheDocument(); + } + }); + }); });