fix: BROWSE validation message was clipped away; refresh docs + screenshots#55
Merged
Merged
Conversation
…nshots The per-cell validation error (#45) was never visible. Grid cells set overflow:hidden for the ellipsis on long values, which clipped the absolutely-positioned tooltip to nothing — an invalid edit showed a red border and no explanation of what was wrong. The e2e tests passed the whole time: toContainText and toBeVisible do not account for clipping by an ancestor's overflow. The assertion now uses toBeInViewport(), which is backed by an IntersectionObserver and does. Verified it fails against the unfixed CSS (viewport ratio 0). Found by looking at a screenshot of the feature rather than trusting a green test — now recorded in CLAUDE.md's test-discipline notes. Also regenerates every screenshot and the README demo GIF against v1.2.0 (they still showed the v1.1.1 banner), and adds a new screenshot-grid-validation.png plus its capture step and a README section for it.
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
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.
The bug
The
BROWSEper-cell validation message shipped in #45 was never visible to anyone.#grid-table tdsetsoverflow: hidden(to ellipsis long values), which clipped the absolutely-positioned.cell-errortooltip to nothing. An invalid edit showed a red border and no reason at all.Why the tests didn't catch it
toContainText()andtoBeVisible()do not account for clipping by an ancestor'soverflow. The element was in the DOM, haddisplay: block; visibility: visible, and a real bounding box — Playwright happily reportedisVisible: true. It was simply painted nowhere.The assertion now uses
toBeInViewport(), which is backed by an IntersectionObserver and therefore does account for ancestor clipping. I verified it fails against the unfixed CSS:The fix is one line:
#grid-table td.cell-invalid { overflow: visible; }.How I found it: not from a test — from looking at a screenshot of the feature while regenerating the docs. That lesson is now in
CLAUDE.md→ Test discipline, alongside thetoContain-can't-see-absence and untested-WS-surface notes.Docs (DoD steps 5–7)
docs/screenshots/*.pngand the READMEdemo.gif— they still showed thev1.1.1banner, and the splash now advertisesDO overtime.docs/screenshots/screenshot-grid-validation.png(the feature actually working — the message reads "STARTTIME: minutes must be a multiple of 15"), a matching capture step inscripts/capture-screenshots.mjs, and a README section for it.CHANGELOG.mdentry under Fixed.Test plan
npx tsc --noEmitclean.npx playwright test tests/grid-validation.spec.ts tests/assistant.spec.ts tests/overtime.spec.ts— 35/35.