Repository navigation
Acceptance follow-ups: product documents, dead CSS, and a server-rendered changelog - #258
Merged
Merged
Conversation
Follow-ups from an acceptance pass over the site. The acceptance gate went from BLOCK to CONDITIONAL: the three required documents now exist, and what remains unevaluated is structural -- acceptance ran in the builder's own context, and the frontend domain checker is not installed here. PRODUCT.md, design-direction.md and ux-walkthrough.md are reconstructed from what the repository already evidences, with the user ordering and success criteria supplied by the maintainer. Every claim that is inferred rather than evidenced is marked, so the guesses are correctable instead of looking settled. One thing surfaced by writing them: the README and the website both lead with the agent/MCP story, but agent-driven users rank last of four. The code's history explains the emphasis; PRODUCT.md records the current priority and says where the two disagree. The CSS change removes 67 of the 126 declarations css-shadowing.mjs proves can never take effect, and lowers the ratchet from 126 to 59. The budget was sitting exactly at its cap, so the next shadowed declaration anyone added would have failed the check. The remaining 59 were left alone because the removal script could not match them unambiguously; it reports what it skipped rather than guessing. Verified by computed style: a fresh load of /docs/install/ at 1280x900 hashes identically before and after across 15 selectors and 21 properties, and the a11y suite still reports no violations in either theme.
The page shipped "Loading release notes..." and painted the notes in from script. Everything it needed was already known at build time -- the entries come from this repository's own CHANGELOG.md -- so a crawler or a reader without JavaScript got an empty page whose entire purpose is release notes. Static HTML went from 66 visible words to 4,111. The build runs the same renderer the browser runs, under linkedom, rather than gaining a second server-side one. The card markup sits on top of about 400 lines of markdown-to-DOM conversion, and two copies of that would drift apart quietly -- this repo has lost days to exactly that kind of drift in its stylesheets. The one thing the build render skips is the disclosure that collapses long cards: it measures scrollHeight, which is 0 without layout. The client attaches it on load instead. That also means the no-JS rendering shows every entry in full, which is the better direction to fail in. A failed GitHub request no longer replaces the notes with an error. The build-time render is already on screen and is accurate; it just has not been told which tags are published, and throwing it away because a metadata request failed loses the thing the reader came for. Also narrows parseLocalChangelog's filter to a type guard. `.filter(Boolean)` does not narrow the null out, which only surfaced once the result was passed to a typed function instead of into an untyped define:vars. Not verified here: the collapse itself. requestAnimationFrame never fires in the automation pane -- document.hidden is true -- so no rAF-scheduled code can be exercised through it, on this build or the one before it.
Contributor
|
Site preview: https://pr-258.netscli-site-preview.pages.dev Built from 1b77963 with Production is unaffected: netscli.com is served from GitHub Pages via |
design-tokens.json extracts the two themes from apps/netscli-gui/src/styles/tokens.css so the frontend gate can measure contrast. tokens.css stays the file the app reads; this is a description of it, kept by hand, and worth nothing if it drifts. Adding it moves the gate from CONDITIONAL to BLOCK, which is the point of adding it. --text-muted fails the 4.5:1 body-text bar in both themes -- 3.43:1 dark, 3.49:1 light -- and it is not a decorative token: 43 rules use it for placeholders, form labels and detail-pane text. WCAG allows 3:1 only at 18pt or 14pt bold, and a token carries no size, so the body-text bar is the only safe one to hold it to. Nothing here changes the colours. Fixing the contrast alters how the app looks, which is a design decision rather than a mechanical one; the failing pairs are recorded so that decision is taken deliberately instead of the token quietly passing because nobody measured it.
--text-muted was 3.43:1 on the dark base and 3.49:1 on the light one, against a 4.5:1 body-text bar. It is not a decorative token: 43 rules use it for placeholders, form labels and detail-pane text, and WCAG allows 3:1 only at 18pt or 14pt bold. A token carries no size, so whichever component renders it at 13px decides whether the pair was ever legible. Dark #667085 -> #7a8499 (4.54:1), light #7b8797 -> #687484 (4.54:1). Both are about 20/255 per channel, chosen as the smallest step along the existing colour that clears the bar rather than a new grey -- the token is meant to be quiet, and this keeps it quiet while making it readable. Checked in the running app, not only by ratio: the setting descriptions, the empty detail pane and the traffic units still read as secondary to the text beside them. The frontend gate now reports 4 pairs >= 4.5:1 across 2 themes, which takes the acceptance verdict back from BLOCK to CONDITIONAL.
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
1 participant
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 four items from the site acceptance pass. The gate moved BLOCK → CONDITIONAL. Nothing fails; the verdict is capped by two unevaluated checks.
A-independentis structural — acceptance ran in the builder's own context, so SHIP is unreachable from here.D-frontenddoes run, and itself returns CONDITIONAL on two open items:F-dual-framework("no package.json readable" — it looks at the repo root, which is a Cargo workspace; the JS lives insite/andapps/netscli-gui/) andF-tokens-contrast(nodesign-tokens.json, so colour contrast cannot be evaluated). An earlier revision of this description said that checker was not installed; that was wrong — it is installed, and the run reporting otherwise had simply failed to vendor it.1. The changelog page is no longer empty without JavaScript
It shipped
Loading release notes...and painted the notes in from script, even though the entries come from this repo's ownCHANGELOG.mdand were already computed at build time. Static HTML: 66 visible words → 4,111.The build runs the same renderer the browser runs, under
linkedom, rather than gaining a second server-side one — the card markup sits on ~400 lines of markdown-to-DOM conversion, and two copies would drift. This repo has lost days to exactly that kind of drift in its stylesheets.The build render skips only the disclosure that collapses long cards, which measures
scrollHeightand so needs layout; the client attaches it on load. No-JS therefore shows every entry in full — the better direction to fail in.A failed GitHub request no longer replaces the notes with an error message. The build-time render is accurate; it just has not been told which tags are published.
2. CSS shadowing budget had zero headroom
Removed 67 of 126 declarations
css-shadowing.mjsproves can never take effect, and lowered the ratchet 126 → 59. The remaining 59 were left alone because the removal script could not match them unambiguously — it reports what it skipped rather than guessing.Verified by computed style, not by eye: a fresh load of
/docs/install/at 1280×900 hashes identically before and after across 15 selectors and 21 properties.3. Two versions showing "Not yet released" — no change made
This is correct behaviour. The badge appears only once GitHub has confirmed a tag is not a published release, and the card deliberately omits the link rather than pointing at a 404. Both 0.3.0 (draft) and 0.3.1 genuinely are unreleased. Hiding them would make the changelog less accurate. It resolves on publish.
4. Product documents
PRODUCT.md,design-direction.mdandux-walkthrough.md, reconstructed from what the repo evidences, with user ordering and success criteria supplied by the maintainer. Every inferred claim is marked[inferred]so guesses are correctable rather than looking settled.Writing them surfaced a tension worth recording: the README and the website both lead with the agent/MCP story, but agent-driven users rank last of four.
PRODUCT.mdrecords the current priority and says where the two disagree.Not verified here
The disclosure collapse.
requestAnimationFramenever fires in the automation pane —document.hiddenistrue— so no rAF-scheduled code can be exercised through it, on this build or the one before it. I briefly concluded the collapse was broken and "fixed" it; that conclusion came from the harness, not the app, and the change was reverted. Worth a human glance on a long release card.