test(plugin-detail): pin the viewport in the #8350 dedupe pin, and cover the mobile branch - #8445
Merged
Merged
Conversation
…ver the mobile branch (objectui#8399) `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
os-justin
marked this pull request as ready for review
September 7, 2026 23:49
os-justin
enabled auto-merge
September 7, 2026 23:49
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8399
record-details.dedupeEmptinessTrims-8350.test.tsxargued that a missing row could only be the dedupe's doing, because every fixture stayed belowDetailSection's auto-hide minimum. Both halves of that argument are viewport-dependent, and the file pinned neither value. It was green because happy-dom reportsinnerWidth1024.The margin, recounted on this base (
ca3942729), not taken from the cardRendered rows after the dedupe, and how many of them are empty today (post-#8396), measured at a pinned 1280 by counting rows in the container and
No valueplaceholders by title:CONTRACT_FIELDSACTIVITY_FIELDSPLAIN_FIELDSNON-REGRESSIONThe card said "3 rendered rows, 2 of them empty" for the ACTIVITY fixture and that is confirmed. Two things the card did not say, both of which come out of the recount:
AUTO_HIDE_MIN_FIELDS = isMobile ? 3 : 4,AUTO_HIDE_RATIO = isMobile ? 0.2 : 0.25(DetailSection.tsx:258-260).The hazard is open — it is not a hypothetical, and it did not close
Ablation from the committed implementation, mutation proven on disk by
git hash-objectand restored by state (git diff HEADempty and the hash back togit rev-parse HEAD:PATH), trap onEXIT INT TERMwith absolute paths:Leg 1 — pin the existing describe to the mobile branch (1280 to 375):
Exactly the predicted failure, and note what the message says: nothing about auto-hide. That one line is the whole reason this card exists.
Leg 2 — pin the new mobile describe to desktop (375 to 1280):
The new cases are not vacuous, and the contrast between the two legs is itself the argument for reading presence through
queryByTextintoexpect(value, message).What changed
DESKTOP_WIDTHfor the existing describe, with a comment saying why: these cases assert which row the dedupe drops, and that reading is only clean while no second mechanism can remove a row — which at these fixture sizes is true only on the desktop branch. Pinning desktop is the decision, not the accident.DetailSection's emptiness test "is raw — a whitespace-only value counts as FILLED there". fix(plugin-detail): DetailSection shares ONE definition of emptiness, and it trims #8396 made that definition trim, which is precisely what turned this file's margin from comfortable into zero. The stale sentence is replaced by the measured table above and by the statement that the file now pins the viewport.MOBILE HALF 1shows both blank rows hidden withDetailSectionofferingShow 2 empty fields,MOBILE HALF 2clicks that toggle, gets both rows back, and asserts thatAcme Corporation— the row the dedupe removed — does not come back.Checks
pnpm exec vitest run packages/plugin-detail/src/renderers/__tests__/record-details.dedupeEmptinessTrims-8350.test.tsx—Test Files 1 passed (1) / Tests 10 passed (10)(was 8).pnpm --filter @object-ui/plugin-detail type-check— exit 0. The test file is genuinely in that program:tsc -p tsconfig.test.json --listFileslists it (1 hit, withDetailSection.tsxas the lit control).pnpm exec eslint .inpackages/plugin-detail— exit 0, 0 errors, 901 pre-existing warnings; the two on this file are the pre-existingno-explicit-anyonrenderBody.node scripts/check-changeset-presence.mjsverdict:1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/issue-8399-dedupe-pin-pins-viewport.md.Every one of them has an EMPTY frontmatter — declared as releasing nothing, which is the explicit exemption and a complete answer to this gate.check-changeset-no-major.mjsalso exit 0.grep -naPfor the control ranges, zero hits, with a lit control that fires on a file containing one).Out of scope, filed separately
#8444 — forcing
useIsMobileto answer "mobile" turns 21 of 137 test files in this package red (77 of 1233 tests). Same class, one layer out. Not touched here.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code