From 51194c54afcaa7b52663bd3ad6949985394e0102 Mon Sep 17 00:00:00 2001 From: Elliot Date: Sun, 19 Jul 2026 13:14:24 -0500 Subject: [PATCH] fix: viewer affordance for text/blob/json and overflow-detected cells (#216) TruncatedCell had three problems compounding: 1. Sliced text at 47 chars + '...' in JS at render time, so dragging a column wider never revealed more of the value. 2. Offered no viewer affordance for short text/blob/json values, even though those types are inherently multi-line / structured and benefit from the formatted viewer regardless of length. 3. Used a character-length heuristic (>50) to decide whether the cell was 'truncated', which didn't reflect actual visible overflow. Fix: - Render the full value via CSS truncation (text-overflow: ellipsis). Cell reflows as the column resizes. Closes #216. - Show a viewer affordance (Maximize2 icon button) on every cell where it is useful. Icon appears when EITHER: (a) dataType matches /text|blob|json|mediumtext|longtext/i and value length >= 20, OR (b) actual visual overflow is detected by ResizeObserver. Icon disappears the moment the column widens enough (ResizeObserver re-checks after each resize). - Icon click opens the viewer and stops propagation so row selection is unaffected. Double-click on the cell is kept as a fallback affordance for users without an icon shown (e.g. non-truncated non-text values). - ResizeObserver + rAF debounce so a flurry of resize events collapse to a single scrollWidth/clientWidth check per frame. - TruncatedCell now takes an optional dataType prop. ResultsGrid passes col.data_type from the result set. Tests: - 19 cases covering NULL, primitive types, full-text render, no-icon for short non-text, icon for each text/blob/json variant, overflow via ResizeObserver, hide-on-widen, icon click triggers viewer, stopPropagation, double-click fallback on/off. --- src/components/grid/ResultsGrid.tsx | 1 + src/components/grid/TruncatedCell.tsx | 92 ++++--- .../grid/__tests__/TruncatedCell.test.tsx | 227 +++++++++++++++--- 3 files changed, 255 insertions(+), 65 deletions(-) diff --git a/src/components/grid/ResultsGrid.tsx b/src/components/grid/ResultsGrid.tsx index 80ce312..36f3149 100644 --- a/src/components/grid/ResultsGrid.tsx +++ b/src/components/grid/ResultsGrid.tsx @@ -367,6 +367,7 @@ export function ResultsGrid() { { setCellViewer({ isOpen: true, diff --git a/src/components/grid/TruncatedCell.tsx b/src/components/grid/TruncatedCell.tsx index f259708..2d7421d 100644 --- a/src/components/grid/TruncatedCell.tsx +++ b/src/components/grid/TruncatedCell.tsx @@ -1,11 +1,15 @@ +import { Maximize2 } from "lucide-react"; +import { useEffect, useRef, useState } from "react"; + interface Props { value: unknown; columnName: string; + dataType?: string; onViewFull: (content: string | null, columnName: string) => void; } -const MAX_DISPLAY_LENGTH = 50; -const TRUNCATE_LENGTH = 47; // Leaves room for "..." +const TEXT_TYPE_MIN_LENGTH = 20; +const TEXT_TYPE_PATTERN = /text|blob|json|mediumtext|longtext/i; function formatValue(val: unknown): string { if (val === null || val === undefined) return "NULL"; @@ -15,40 +19,74 @@ function formatValue(val: unknown): string { return String(val); } -function shouldTruncate(val: unknown): boolean { - if (val === null || val === undefined) return false; - const formatted = formatValue(val); - return formatted.length > MAX_DISPLAY_LENGTH; -} - -function getTruncatedDisplay(val: unknown): string { - if (val === null || val === undefined) return "NULL"; - const formatted = formatValue(val); - if (formatted.length > MAX_DISPLAY_LENGTH) { - return formatted.slice(0, TRUNCATE_LENGTH) + "..."; - } - return formatted; +function isLongTextType(dataType?: string): boolean { + if (!dataType) return false; + return TEXT_TYPE_PATTERN.test(dataType); } export function TruncatedCell({ value, columnName, + dataType, onViewFull, }: Props) { - const isTruncated = shouldTruncate(value); - const displayText = getTruncatedDisplay(value); + const formatted = formatValue(value); + const isTextType = isLongTextType(dataType); + const showForTextType = isTextType + && value !== null + && value !== undefined + && formatted.length >= TEXT_TYPE_MIN_LENGTH; + + const [isOverflowing, setIsOverflowing] = useState(false); + const textRef = useRef(null); + + useEffect(() => { + const el = textRef.current; + if (!el) return; + let scheduled = false; + const check = () => setIsOverflowing(el.scrollWidth > el.clientWidth); + const ro = new ResizeObserver(() => { + if (scheduled) return; + scheduled = true; + requestAnimationFrame(() => { + scheduled = false; + check(); + }); + }); + ro.observe(el); + check(); + return () => ro.disconnect(); + }, [formatted, columnName, dataType]); + + const showIcon = showForTextType || isOverflowing; + const openViewer = () => { + onViewFull(value === null || value === undefined ? null : formatted, columnName); + }; return ( -
{ - if (isTruncated) { - onViewFull(value === null || value === undefined ? null : formatValue(value), columnName); - } - }} - title={isTruncated ? "Double-click to view full content" : ""} - > - {displayText} +
+
+ {formatted} +
+ {showIcon && ( + + )}
); } diff --git a/src/components/grid/__tests__/TruncatedCell.test.tsx b/src/components/grid/__tests__/TruncatedCell.test.tsx index 9d4d742..ae7c82d 100644 --- a/src/components/grid/__tests__/TruncatedCell.test.tsx +++ b/src/components/grid/__tests__/TruncatedCell.test.tsx @@ -1,7 +1,49 @@ -import { fireEvent, render, screen } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; +import { act, fireEvent, render } from "@testing-library/react"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { TruncatedCell } from "../TruncatedCell"; +class FakeResizeObserver { + static instances: FakeResizeObserver[] = []; + callback: ResizeObserverCallback; + observed: Element[] = []; + constructor(callback: ResizeObserverCallback) { + this.callback = callback; + FakeResizeObserver.instances.push(this); + } + observe(el: Element) { + this.observed.push(el); + } + unobserve() {} + disconnect() { + FakeResizeObserver.instances = FakeResizeObserver.instances.filter((i) => i !== this); + } + trigger() { + this.callback([], this as unknown as ResizeObserver); + } +} + +beforeAll(() => { + (globalThis as unknown as { ResizeObserver: typeof ResizeObserver }).ResizeObserver = + FakeResizeObserver as unknown as typeof ResizeObserver; + globalThis.requestAnimationFrame = ((cb: FrameRequestCallback) => { + cb(0); + return 0; + }) as typeof globalThis.requestAnimationFrame; + globalThis.cancelAnimationFrame = (() => {}) as typeof globalThis.cancelAnimationFrame; +}); + +beforeEach(() => { + FakeResizeObserver.instances = []; +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function getIconButton(container: HTMLElement) { + return container.querySelector("button[aria-label=\"View full content\"]"); +} + describe("TruncatedCell", () => { it("renders NULL for null value", () => { const { container } = render( @@ -45,70 +87,179 @@ describe("TruncatedCell", () => { expect(container.textContent).toBe("42"); }); - it("truncates long string values", () => { + it("renders full long string values (visual truncation handled by CSS, #216)", () => { const longText = "a".repeat(100); const { container } = render( , ); - const text = container.textContent ?? ""; - expect(text.length).toBeLessThan(100); - expect(text.endsWith("...")).toBe(true); + expect(container.textContent).toContain("aaa"); + expect(container.textContent?.length).toBe(longText.length); }); - it("does not truncate short string values", () => { - const shortText = "a".repeat(30); + it("applies truncate class to the text element", () => { const { container } = render( - , + , ); - expect(container.textContent).toBe(shortText); + const text = container.querySelector(".truncate"); + expect(text).not.toBeNull(); }); - it("calls onViewFull on double click when truncated", () => { - const onViewFull = vi.fn(); - const longText = "a".repeat(100); - render( - , + it("does not show icon for short non-text values", () => { + const { container } = render( + , + ); + expect(getIconButton(container)).toBeNull(); + }); + + it("does not show icon for short numeric values", () => { + const { container } = render( + , ); + expect(getIconButton(container)).toBeNull(); + }); + + it("does not show icon for null values", () => { + const { container } = render( + , + ); + expect(getIconButton(container)).toBeNull(); + }); + + it("does not show icon for text types with below-threshold values", () => { + const { container } = render( + , + ); + expect(getIconButton(container)).toBeNull(); + }); - const cell = screen.getByText((content) => content.endsWith("...")); - fireEvent.doubleClick(cell); - expect(onViewFull).toHaveBeenCalledWith(longText, "myCol"); + it("shows icon for text/blob/json types with non-trivial values", () => { + const longText = "a".repeat(100); + for (const dataType of ["text", "TEXT", "tinytext", "mediumtext", "longtext", "blob", "BLOB", "json", "JSON"]) { + const { container, unmount } = render( + , + ); + expect(getIconButton(container)).not.toBeNull(); + unmount(); + } }); - it("does not call onViewFull on double click when not truncated", () => { + it("shows icon when actual overflow is detected via ResizeObserver (#216)", async () => { const onViewFull = vi.fn(); - render( - , + const { container } = render( + , ); + expect(getIconButton(container)).toBeNull(); - fireEvent.doubleClick(screen.getByText("short")); - expect(onViewFull).not.toHaveBeenCalled(); + const textEl = container.querySelector(".truncate") as HTMLElement; + Object.defineProperty(textEl, "scrollWidth", { configurable: true, value: 500 }); + Object.defineProperty(textEl, "clientWidth", { configurable: true, value: 100 }); + await act(async () => { + FakeResizeObserver.instances[0]?.trigger(); + }); + + expect(getIconButton(container)).not.toBeNull(); }); - it("null values do not trigger onViewFull on double click (not truncated)", () => { + it("hides icon when overflow ceases after column widens", async () => { + const { container } = render( + , + ); + const textEl = container.querySelector(".truncate") as HTMLElement; + + Object.defineProperty(textEl, "scrollWidth", { configurable: true, value: 500 }); + Object.defineProperty(textEl, "clientWidth", { configurable: true, value: 100 }); + await act(async () => { + FakeResizeObserver.instances[0]?.trigger(); + }); + expect(getIconButton(container)).not.toBeNull(); + + Object.defineProperty(textEl, "scrollWidth", { configurable: true, value: 50 }); + Object.defineProperty(textEl, "clientWidth", { configurable: true, value: 100 }); + await act(async () => { + FakeResizeObserver.instances[0]?.trigger(); + }); + expect(getIconButton(container)).toBeNull(); + }); + + it("icon click opens the viewer", () => { const onViewFull = vi.fn(); - render( - , + const longText = "a".repeat(100); + const { container } = render( + , ); + const button = getIconButton(container) as HTMLButtonElement; + fireEvent.click(button); + expect(onViewFull).toHaveBeenCalledWith(longText, "payload"); + }); - fireEvent.doubleClick(screen.getByText("NULL")); - expect(onViewFull).not.toHaveBeenCalled(); + it("icon click stops propagation so row selection is not triggered", () => { + const onViewFull = vi.fn(); + const parentClick = vi.fn(); + const longText = "a".repeat(100); + const { container } = render( +
+ +
, + ); + const button = getIconButton(container) as HTMLButtonElement; + fireEvent.click(button); + expect(onViewFull).toHaveBeenCalledTimes(1); + expect(parentClick).not.toHaveBeenCalled(); }); - it("shows hover title when truncated", () => { + it("double-clicking the text also opens the viewer when icon is shown", () => { + const onViewFull = vi.fn(); const longText = "a".repeat(100); - render( - , + const { container } = render( + , ); - expect(screen.getByTitle("Double-click to view full content")).toBeInTheDocument(); + const textEl = container.querySelector(".truncate") as HTMLElement; + fireEvent.doubleClick(textEl); + expect(onViewFull).toHaveBeenCalledWith(longText, "payload"); }); - it("has no hover title when not truncated", () => { - render( - , + it("double-click does not open the viewer when no icon is shown", () => { + const onViewFull = vi.fn(); + const { container } = render( + , ); - expect( - screen.queryByTitle("Double-click to view full content"), - ).not.toBeInTheDocument(); + const textEl = container.querySelector(".truncate") as HTMLElement; + fireEvent.doubleClick(textEl); + expect(onViewFull).not.toHaveBeenCalled(); }); });