Skip to content

polish: localStorage safety, scroll perf, a11y, dead code, modernize CSS#5

Merged
Arty2 merged 5 commits intomasterfrom
claude/polish-pass
Apr 29, 2026
Merged

polish: localStorage safety, scroll perf, a11y, dead code, modernize CSS#5
Arty2 merged 5 commits intomasterfrom
claude/polish-pass

Conversation

@Arty2
Copy link
Copy Markdown
Owner

@Arty2 Arty2 commented Apr 29, 2026

Why

A read-only audit of the codebase outside the recent forge-meta-security (#2), css-modernize (#3), and modifier-docs (#4) PRs surfaced a cluster of small, isolated wins that share one theme: stop relying on patterns that no longer pull their weight. Three Explore agents proposed 30+ items; manual cross-checking eliminated the false positives (one agent flagged the taxon partial as orphaned despite live usage at tags-list.html:3 + articles-filter.html:15; another flagged the just-documented .no-* modifiers as orphans). What landed below is verified line-by-line.

What

Robustness — localStorage try/catch

localStorage.getItem and setItem are wrapped in try / catch. Safari Private Mode throws QuotaExceededError (and on older Safari, even on read), which today crashes both the inline theme bootstrap and the toggle handler.

  • layouts/_default/baseof.html:9-24 — bootstrap rewritten; theme is now declared with let instead of being an implicit global created by if (theme = localStorage.getItem('theme')).
  • static/scripts/onion.js:124setItem wrapped; in-memory toggle still works in Private Mode, just no persistence.

Scroll perf — setInterval(1000) → passive scroll listener

onion.js:67–81 polled window.scrollY every second to swap the #scroller gadget between up/down/hidden. Replaced with window.addEventListener('scroll', update, { passive: true }) plus an initial update() call. Same logic, instant response (was up to 1 s lag), zero idle-time work.

Accessibility

  • aria-current="page" on active main-nav links — top-level and nested children. Mirrors the existing idiom at languages.html:5.
  • aria-label on the icon-only #themer and #scroller <a> elements in gadgets.html. New i18n keys toggle-theme and scroll-top in both en.yaml and el.yaml.
  • Search form wrapped in <search> landmark (HTML Living Standard, Safari 17 / Chrome 120 / Firefox 118; older browsers degrade to a generic block element with no behavioural impact).

Per-page menu cache fix

header-inner.html:11 called partialCached "menu-main" . with no variant key, so Hugo cached the first render and reused it for every page — meaning aria-current would have been pinned to whichever page Hugo rendered first. Added .RelPermalink as the variant key.

JS modernization — varconst / let

All ~15 sites in static/scripts/onion.js. const for never-reassigned (lbBase, lbCss, galleries, figures, slideAnimation); let for loop counters (index_current, index, z). The themer and scroller IIFEs were also restructured around early-return guards. The themer.onclick handler used an undeclared event reference (worked in Chromium via window.event, broken in Firefox) — handler now takes event as a parameter.

CSS modernization

Removed / changed Why
-webkit-transition paired with unprefixed siblings Safari has supported unprefixed transition since 9 (2015)
-webkit-mask-imagemask-image Safari 15.3+ (2022) supports unprefixed
-webkit-appearance: button iOS Safari 15+ has native parity
2× commented-out -webkit-mask-image dead

Dead code

  • --theme--toggle: 0 (double dash, typo) declared in .theme--gray and .theme--blue — never read. Deleted from both.
  • i18n key description ("Summary") — defined in en.yaml and el.yaml but no template references. Deleted from both. Note: rss-about was also flagged by the audit but IS used at layouts/_default/index.xsl:41 — kept.
  • Three stale TODO comments resolved/removed (screen.css:866, screen.css:1567, articles-filter.html:26).

Test plan

  • cd exampleSite && hugo --gc --minify — clean, no warnings
  • npx html-validate@^9 --config .github/html-validate.json public/**/*.html — exit 0
  • grep -rnE "theme--toggle|-webkit-(transition|appearance|mask-image)" assets/styles/screen.css — empty
  • grep -c "^\s*var " static/scripts/onion.js — 0
  • Rendered HTML spot-check: aria-current=page on /archives/ only (not on /); gadget icons emit aria-label="Toggle theme" / "Scroll to top"
  • Manual: theme toggle round-trips correctly (click → reload → persists); DevTools "block localStorage" → toggle still works in-memory, no console error
  • Manual: scroll a long page — gadget arrow swaps updown instantly (was up to 1 s lag before)
  • Manual: VoiceOver / NVDA reads "current page" on the active main-nav item; both gadget icons announce their purpose

Out of scope (next branch, stacked)

claude/print-stylesheet will add a separate assets/styles/print.css linked with media="print". Reading-time, OG image dimensions, dark-mode favicon, and apple-touch-icon were considered and dropped per maintainer call ("very little benefit").


Generated by Claude Code

claude added 2 commits April 29, 2026 04:48
A grab-bag of small, low-risk wins that share one theme: stop relying on
patterns that no longer pull their weight.

* Robustness. `localStorage.getItem` and `setItem` are wrapped in
  try/catch — Safari Private Mode throws on access, which today crashes
  the inline theme bootstrap (`baseof.html`) and the toggle handler
  (`onion.js`). Both now fall through cleanly. The inline bootstrap also
  declared its `theme` variable as an implicit global; explicit `let`
  now.

* Scroll perf. `setInterval(1000)` polled `window.scrollY` every second
  to swap the gadget arrow between up/down/hidden. Replaced with a
  passive `scroll` listener plus an initial `update()` call. Same logic,
  responds instantly instead of with up to a 1-s lag, no idle-time work.

* a11y.
  - `aria-current="page"` on active main-nav links (top-level + nested),
    matching the existing pattern in `languages.html:5`.
  - `aria-label` on the icon-only `#themer` and `#scroller` gadget
    buttons; new `toggle-theme` and `scroll-top` keys in en.yaml + el.yaml.
  - Search form wrapped in `<search>` landmark
    (HTML Living Standard, Safari 17 / Chrome 120 / Firefox 118).

* Per-page menu cache. `header-inner.html` was calling
  `partialCached "menu-main" .` with no variant key, so Hugo cached the
  first render and reused it for every page — meaning aria-current would
  have been wrong. Added `.RelPermalink` as the variant key.

* JS modernization. `var` → `const`/`let` everywhere in `onion.js`
  (~15 sites). Block-scoped, no hoisting surprises. The themer and
  scroller IIFEs were also restructured around early-return guards
  rather than nested `if (cond) { … }` blocks. The `themer.onclick`
  handler used an undeclared `event` reference (worked in Chromium via
  `window.event`, broken in Firefox) — handler now takes `event` as a
  parameter.

* CSS modernization. Removed three `-webkit-transition` lines paired
  with unprefixed siblings (no-op in modern browsers); replaced two
  `-webkit-mask-image` with unprefixed `mask-image` (Safari 15.3+);
  removed `-webkit-appearance: button` (iOS Safari 15+ has native
  parity). Also dropped two commented-out `-webkit-mask-image` lines.

* Dead code.
  - `--theme--toggle: 0` (double dash, typo) inside `.theme--gray` and
    `.theme--blue` — declared but never read. Deleted both.
  - i18n key `description` ("Summary") — defined in en.yaml + el.yaml
    but no template references. Deleted from both.
  - Three stale TODO comments resolved or removed.

Verified: hugo build clean, html-validate exit 0, theme toggle
round-trips correctly via localStorage, gadget arrow updates instantly
on scroll, aria-current renders on the right page only.

https://claude.ai/code/session_01LAYiVF9Z1tk1PW3UAZz1Wb
A dedicated `assets/styles/print.css` linked with `media="print"`.
Browsers natively defer non-matching media, so the file is fetched
lazily — only when the user actually prints (or print-previews).
Screen readers and the average page load pay nothing.

A separate sheet (rather than an inline `@media print` block in
`screen.css`) keeps the default stylesheet focused, makes overrides
trivial via a custom `static/styles/custom-print.css`, and lets the
browser drop the rules from memory when not in use.

Rules:
* Flat, ink-saving palette — overrides every theme's HSL channels at
  `:root` and forces `body` to black-on-white.
* Single-column layout — `max-width: none` on `body > header`,
  `main`, `body > footer`.
* Hides chrome with no value on paper: `.gadgets`, `.languages`,
  `nav`, `.taxonomy-filter`, `#search__form`, `.menu-social`,
  `.share`. Generic escape hatch: `[data-print="hide"]`.
* Expands `http(s)` and protocol-relative hyperlinks to footnote-style
  URLs via `a::after { content: " (" attr(href) ")" }` so they survive
  the page. Same-page anchors and `mailto:`/`tel:` stay clean.
* Page-break hints: `break-after: avoid` on h1–h6, `break-inside:
  avoid` on `figure`, `pre`, `blockquote`, `.alert`.
* Drops decorative effects (`mix-blend-mode`, `filter`, hover
  outlines) that print poorly.

Wired through Hugo's existing asset pipeline (`resources.Get | minify
| fingerprint`) with SRI integrity, mirroring how `screen.css` and
`syntax.css` are already loaded.

https://claude.ai/code/session_01LAYiVF9Z1tk1PW3UAZz1Wb
claude and others added 3 commits April 29, 2026 05:03
`galleries.forEach((element, index) => …)` declared an outer `index`
parameter that was never used inside the callback. With `var` the
inner `var index = 0;` silently shadowed it; after the var→let sweep
in the previous commit, the inner `let index = 0;` collides:

    SyntaxError: Identifier 'index' has already been declared
        at static/scripts/onion.js:138

The `node --check` step in CI catches this. Fix is to drop the unused
outer parameter — the inner counter is the one actually used at lines
144, 145, 159, 172, 183.
add print.css as a separate, lazy-loaded stylesheet
@Arty2 Arty2 merged commit 2670963 into master Apr 29, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants