Add Omarchy-style navigation and animated hero announcement - #203
HANCORE-linux wants to merge 3 commits into
Conversation
|
Reviewed by Claude Opus 5 at high effort and by Codex at xhigh reasoning as an independent second reviewer. Two verdicts, kept apart: whether the site's chrome should look like this is a design decision for the maintainer and nothing below tries to make it; what follows is what the rewrite does mechanically, and one defect that has been fixed on this branch. Nothing was lost in the -675. Both trees were built on a disposable VM and compared file for file. 2647 HTML pages on each side and an identical file set — the only difference is the hashed One behaviour did change, and it is the maintainer's call. Deleting The animation checks out. Under emulated Keyboard and contrast. The tab order runs skip link, home, News, Manual, Plugins, Themes, search, theme, RSS, GitHub, Install, and every stop has a visible 2px ring. The pause control is real: it is tab stop 14, Fixed and pushed as Still open, reported rather than fixed because the remedy is a design choice: a focused current-page nav link draws the same inset brand ring over the Three loose ends the deletions left, none with any effect on screen: No supply-chain or privacy change. No conflict with the theme gallery PRs (#187, #188, #189, #190, #191, #193, #196, #197, #200). Those touch On a worker VM built from the worktree: Codex agreed with the conclusions already reached on the deletions, the animation lifecycle and the no-JS path; its independence is not currently guaranteed, so that agreement is worth less than it looks. The focus-ring defect is the exception and stands on its own: it is a mechanism this review had not reasoned about, and it is credited on the commit. Waiting on the maintainer, for the design. |
The bar's controls are no taller than its row, so their ring is drawn 2px inside the box. Applying that to every a and button in the header reached the mobile menu's Install too, which carries the brand fill: a brand outline drawn inside a brand fill is the colour of the fill, so the ring measured 1.00:1 in every theme and the primary call to action had no visible focus at all. Naming the bar's own controls leaves Install, RSS, GitHub and the music control with the ring the shared button already ships, outside their box. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Codex XHigh <noreply@openai.com>
1ce7533 to
571a46b
Compare
|
Re-reviewed at The open finding is closed
All 22 themes were computed, not only those four, because a shared token moves every theme at once and this is exactly the change where a dark-theme fix costs a light one. Nothing was traded: the floor across all 22 is everforest at 4.91:1, which is the same ratio the label text already had on that fill — the ring now reuses the ink that was already proven to read there, which is the neat part of the fix. The ordinary non-current ring is untouched, still 3.14:1 or better everywhere with rose-pine the floor. The cascade works out too: Two other things that were verified rather than assumedThe mobile Install fix survived the rebase intact. Because the branch was rebased, the commit we pushed is no longer an ancestor of this head — it came back as The named-class scoping leaves nothing unringed. Every focusable element inside Three small things, none blocking1. rose-pine, hovered and keyboard-focused at once. A non-current bar control that is both hovered and keyboard-focused draws the brand ring against the 8% hover fill at 2.797:1, under the same 3:1 the fix above just cleared. rose-pine only; the next lowest is miasma at 3.26:1. Reachable by tabbing to a control and then moving the mouse over it. Current-page links are safe — the later 2. 3. The highlighted amount now reaches 27 more locales. The new fallback in Still open from the previous pass and unchanged: the dead What was checked, and by whomReduced motion, layout shift and the no-JS path were all re-verified at this head rather than carried forward from the last review, since the animation changed: Codex at xhigh confirmed the anchors it was given against Nothing was pushed this run. The accessibility objection that was left open is genuinely closed; what remains is the maintainer's, and it has not moved: whether the chrome should be this bar at all, and whether an indefinitely repeating hero animation belongs on the front page. Worth their attention alongside it: #244 is a competing fix for the duplicate hero navigation this rewrite removes outright, and #266 styles a header wordmark this PR deletes, so those two need a merge order rather than a rebase. |
2 participants
Summary
$15.5 millionamount, using the registry PR #10 glyph pool. Resolve the text over approximately 1.6 seconds and repeat every six seconds while visible.Motion and accessibility
Verification
npm run lintnpm run typechecknpm test— 31 Node tests and 26 Python tests passednpm run check:translations— no pending UI messagesnpm run parityagainst the complete working-tree exportThe unchanged upstream site already emits
AbortError: Transition was skippedduring client navigation. The same warnings were reproduced on both builds; this PR introduces no additional navigation errors and leaves that upstream behavior untouched.Base
PR #198 has merged. This branch is rebased onto
masterat83dbfa5748527737c37e468b1ee6eef5817e982a, retaining the current localization, workstation, branding, content, download, and navigation updates.