From 7d14401b7a75121d820ae46db29695ea43366dd5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 23:38:09 +0000 Subject: [PATCH] test(plugin-detail): pin the viewport in the #8350 dedupe pin, and cover the mobile branch (objectui#8399) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `record-details.dedupeEmptinessTrims-8350.test.tsx` kept every fixture below `DetailSection`'s auto-hide minimum so that a missing row could only be the dedupe's doing. Both halves of that argument are viewport-dependent (`AUTO_HIDE_MIN_FIELDS` 4/3, `AUTO_HIDE_RATIO` 0.25/0.2, chosen by `useIsMobile`), and the file pinned neither value — it was green because happy-dom reports `innerWidth` 1024, i.e. the desktop branch. That became load-bearing when objectui#8376 made `DetailSection`'s emptiness test TRIM: the ACTIVITY fixture's 3 rendered rows are now 2/3 empty, which sits exactly at the mobile minimum. Measured on this base, pinning the viewport to 375 reddens `WHITESPACE AT THE DERIVATION RUNG — HALF 2` with a bare "Unable to find ... contract_no" that names nothing about auto-hide. - pin `window.innerWidth` to 1280 for the existing describe, and say in a comment WHY desktop is the branch these cases mean to exercise; - correct the docblock claim that `DetailSection`'s emptiness test is raw (objectui#8376 changed that) and record the measured per-fixture margins; - add a second describe covering the mobile branch — not a substitute — which pins the distinction the desktop choice rests on: an auto-hidden row comes back through the "Show 2 empty fields" toggle, a deduped row never does. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .../issue-8399-dedupe-pin-pins-viewport.md | 6 + ...details.dedupeEmptinessTrims-8350.test.tsx | 158 +++++++++++++++++- 2 files changed, 156 insertions(+), 8 deletions(-) create mode 100644 .changeset/issue-8399-dedupe-pin-pins-viewport.md diff --git a/.changeset/issue-8399-dedupe-pin-pins-viewport.md b/.changeset/issue-8399-dedupe-pin-pins-viewport.md new file mode 100644 index 0000000000..a355605fec --- /dev/null +++ b/.changeset/issue-8399-dedupe-pin-pins-viewport.md @@ -0,0 +1,6 @@ +--- +--- + +Test-only change (objectui#8399): `record-details.dedupeEmptinessTrims-8350.test.tsx` now pins +`window.innerWidth` instead of inheriting happy-dom's default, and covers the mobile branch of +`DetailSection`'s auto-hide as a second case. No published behaviour changes. diff --git a/packages/plugin-detail/src/renderers/__tests__/record-details.dedupeEmptinessTrims-8350.test.tsx b/packages/plugin-detail/src/renderers/__tests__/record-details.dedupeEmptinessTrims-8350.test.tsx index 67beb0a137..56991ac1fc 100644 --- a/packages/plugin-detail/src/renderers/__tests__/record-details.dedupeEmptinessTrims-8350.test.tsx +++ b/packages/plugin-detail/src/renderers/__tests__/record-details.dedupeEmptinessTrims-8350.test.tsx @@ -64,23 +64,72 @@ * is therefore asserted by its LABEL, which for a bare-string entry is the * field name itself; a row that must drop is asserted by its VALUE. * - * ## Why the whitespace rows are still on screen at all + * ## Why the whitespace rows are still on screen — and why this file now pins + * the viewport (objectui#8399) * - * `DetailSection`'s auto-hide heuristic needs a section of at least 4 fields, - * and its own emptiness test is raw — a whitespace-only value counts as FILLED - * there. Both facts are measured, not assumed: every fixture below renders 3 - * rows after the dedupe, so auto-hide cannot fire and cannot be the reason a - * row is missing. (That `DetailSection` has a THIRD, un-trimmed spelling of - * emptiness is a real and separate divergence; it is not this card.) + * A row may only be missing below because the dedupe dropped it. `DetailSection` + * can also remove a row, through its auto-hide heuristic, and that heuristic is + * decided by TWO viewport-dependent constants (`DetailSection.tsx`, read through + * `useIsMobile`, breakpoint 768): + * + * desktop — `AUTO_HIDE_MIN_FIELDS` 4, `AUTO_HIDE_RATIO` 0.25 + * mobile — `AUTO_HIDE_MIN_FIELDS` 3, `AUTO_HIDE_RATIO` 0.2 + * + * Measured on this base, after the dedupe, per fixture (rendered rows / of which + * empty): CONTRACT 2/1 · ACTIVITY 3/2 · PLAIN 2/1 · NON-REGRESSION 2/0. So on + * the DESKTOP branch nothing here can reach the 4-row minimum and auto-hide + * cannot fire — which is the condition every case below needs. + * + * ⚠️ This file used to state a second reason that is no longer true, and that + * is why the pin exists. `DetailSection`'s own emptiness test WAS raw, so a + * whitespace-only value counted as FILLED there and the ACTIVITY fixture read + * 0 empty rows. objectui#8376 made that definition TRIM — the same definition + * the dedupe ladder reads — so those two rows are EMPTY here now. The ACTIVITY + * fixture consequently sits EXACTLY at the mobile minimum (3 rows) with 2 of + * them empty (0.67 ≥ 0.2): on the mobile branch auto-hide fires and + * `WHITESPACE AT THE DERIVATION RUNG — HALF 2` reddens, reporting only that + * `contract_no` could not be found. + * + * The green was therefore resting on an ambient value this file never stated: + * happy-dom reports `innerWidth` 1024 (measured), which is the desktop branch. + * It is pinned below — and the branch is chosen, not inherited. DESKTOP is the + * branch these cases mean to exercise, because they assert WHICH ROW THE DEDUPE + * DROPS and that reading is only clean while no second mechanism can remove a + * row. The mobile branch is not swept under the rug by that choice: it is + * covered as a SECOND describe at the bottom of this file, never as a + * substitute, and what it pins is the distinction the choice above depends on — + * an auto-hidden row comes back, a deduped row does not. */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { render, screen, cleanup } from '@testing-library/react'; +import { render, screen, cleanup, fireEvent } from '@testing-library/react'; import * as React from 'react'; import { getRecordDisplayName } from '@object-ui/core'; import { RecordContextProvider } from '@object-ui/react'; import { RecordDetailsRenderer } from '../record-details'; +/** + * Viewport, pinned rather than inherited (objectui#8399) — see the docblock. + * + * `Object.defineProperty` rather than assignment: happy-dom's `innerWidth` is a + * getter, so `window.innerWidth = n` is silently a no-op. `configurable: true` + * is what lets the second describe re-pin it. + */ +const DESKTOP_WIDTH = 1280; +const MOBILE_WIDTH = 375; +const pinViewport = (width: number) => () => { + Object.defineProperty(window, 'innerWidth', { configurable: true, value: width }); +}; + +/** + * Presence read as a VALUE, so the `expect` message survives into the CI + * summary. `getByText` throws before `expect` runs, and its one-line summary + * ("Unable to find an element with the text: contract_no") is exactly the + * message that sent this card's reader to the dedupe ladder instead of to the + * auto-hide heuristic. + */ +const shown = (text: string) => screen.queryByText(text) !== null; + /** * Declared pointer `contract_no`, held blank-but-not-empty on the record. * `deriveTitleField` lands on `name` (NAME_ISH_EXACT, first entry), which is @@ -179,6 +228,11 @@ afterEach(() => { }); describe('record:details dedupe — emptiness is the header\'s definition (#8350)', () => { + // DESKTOP on purpose: at these fixture sizes it is the only branch on which + // `DetailSection` auto-hide cannot fire, so a missing row can only be the + // dedupe's doing. See the docblock; the mobile branch is covered below. + beforeEach(pinViewport(DESKTOP_WIDTH)); + /** * The two halves are separate cases ON PURPOSE. Inside one `it` the first * failing expectation short-circuits the rest, so an ablation of the read @@ -291,3 +345,91 @@ describe('record:details dedupe — emptiness is the header\'s definition (#8350 expect(screen.getByText('42')).toBeInTheDocument(); }); }); + +/** + * The MOBILE branch — an ADDITION, never a substitute (objectui#8399). + * + * The describe above pins DESKTOP because that is the branch its cases need. + * That choice is only honest if the other branch is stated rather than avoided, + * and here the other branch is not a null case: the ACTIVITY fixture's 3 + * rendered rows sit EXACTLY at the mobile `AUTO_HIDE_MIN_FIELDS` (3) with 2 of + * them empty (0.67 ≥ `AUTO_HIDE_RATIO` 0.2), so `DetailSection` auto-hide + * fires. Measured, not argued: with the viewport at `MOBILE_WIDTH` and no other + * change, `WHITESPACE AT THE DERIVATION RUNG — HALF 2` above goes red. + * + * ## What these two cases pin, and why it is not just "the same rows again" + * + * A row the dedupe dropped and a row auto-hide hid are BOTH "not on screen". + * The describe above reads every absence as the dedupe's doing, and only the + * desktop thresholds make that reading sound. These cases pin the difference + * the reading rests on, on the branch where both mechanisms are live: + * + * - auto-hide HIDES — `DetailSection` says so with `Show 2 empty fields`, and + * the toggle brings both rows back; + * - the dedupe REMOVES — `Acme Corporation`'s row is gone from the section + * and no toggle resurrects it. + * + * ⇒ if auto-hide ever reaches the desktop branch too (a threshold change, a + * fixture change, another shift in what counts as empty), the describe above + * reddens with a bare "Unable to find … contract_no" while MOBILE HALF 2 stays + * green — and that pair of readings names auto-hide instead of the ladder. + */ +describe('record:details dedupe — on mobile the blank rows are HIDDEN, not deduped (#8399)', () => { + beforeEach(pinViewport(MOBILE_WIDTH)); + + it('MOBILE HALF 1 — auto-hide removes both blank rows, and DetailSection reports them as empty', () => { + renderBody(WHITESPACE_BOTH_RUNGS_RECORD, activitySchema, ACTIVITY_FIELDS); + + expect( + shown('contract_no'), + 'mobile auto-hide must fire on this fixture (3 rendered rows meets AUTO_HIDE_MIN_FIELDS 3, 2/3 empty clears AUTO_HIDE_RATIO 0.2) — this row is absent because it was HIDDEN, not because the dedupe took it', + ).toBe(false); + expect( + shown('activity_name'), + 'the second blank row is hidden by the same heuristic, for the same reason', + ).toBe(false); + + // CONTROL — the grid rendered at all. Every negative above is trivially + // true of a document that rendered nothing. + expect(shown('4'), 'the one filled row must still render').toBe(true); + + // The count is the other half of the reading: a RAW emptiness test would + // see 0 empty rows here and offer no toggle at all, so this line also pins + // that `DetailSection` reads whitespace as empty (objectui#8376). + const toggle = screen.queryByRole('button', { name: /empty fields/i }); + expect( + toggle === null ? 'NO TOGGLE RENDERED' : toggle.textContent, + 'auto-hide must offer the reveal toggle, counting BOTH whitespace-only rows as empty', + ).toContain('Show 2 empty fields'); + }); + + it('MOBILE HALF 2 — the toggle brings the hidden rows back; the DEDUPED row never comes back', () => { + renderBody(WHITESPACE_BOTH_RUNGS_RECORD, activitySchema, ACTIVITY_FIELDS); + + const toggle = screen.queryByRole('button', { name: /empty fields/i }); + expect( + toggle, + 'without the reveal toggle there is nothing to reveal and this case measures nothing', + ).not.toBeNull(); + fireEvent.click(toggle as HTMLElement); + + expect( + shown('contract_no'), + 'revealing empty fields must bring back a row that auto-hide hid', + ).toBe(true); + expect( + shown('activity_name'), + 'revealing empty fields must bring back a row that auto-hide hid', + ).toBe(true); + + // ⭐ The line between the two mechanisms. `label` carries `Acme Corporation`, + // the value this record's H1 shows; the dedupe removed that field from the + // section entirely, so no toggle can resurrect it. If this ever goes green, + // the rows above were auto-hidden rather than deduped and every absence + // asserted in the describe above is measuring the wrong mechanism. + expect( + shown('Acme Corporation'), + 'the dedupe REMOVES the duplicate row — revealing empty fields must not resurrect it', + ).toBe(false); + }); +});