Public: anyone can read this; the crew can change it.

The theme picker in the layout draws §5's field and button, and .theme-option, .theme-apply leave the markup #4738

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

The theme picker in `src/web/layout.ts` now draws §5's field and button, and `theme-option` and `theme-apply` are gone from the markup. The only remaining references are the unused rules in `src/web/style/components.ts` and the line in `docs/DESIGN.md`, which the issue leaves for the follow-up.

shipped production is serving it deploy eb43263

opened this

The theme picker in src/web/layout.ts now draws §5's field and button, and theme-option and theme-apply are gone from the markup. The only remaining references are the unused rules in src/web/style/components.ts and the line in docs/DESIGN.md, which the issue leaves for the follow-up.

The design lint and the tests for design-lint, web and flow-pipeline pass. bun x tsc --noEmit could not run here (it reports a missing bun type definition even after bun install), so the forge's check will be its first run. docs/design/baseline.json is unchanged because the lint counts did not move.

  • Markup: themeForm() is now a fieldset.field with the legend "Theme". Each choice is a label.check holding a radio and the existing swatch. "Apply" is a plain btn("Apply", { size: "sm" }).
  • Without JavaScript: the form still posts to /theme.
  • Client script: the script in src/web/client.ts finds the form by .theme-form, the button by button[type=submit] and the radios by .theme-form input. It still hides Apply and applies the theme on change.
  • Tests: the themes test in test/web.test.ts asserts the choices, the Apply button and that the old classes are absent. test/flow-pipeline.test.ts follows the selector change.

I dropped the check-mark .tick. The native radio now shows the selection, and the old stylesheet rule that drew the tick would otherwise leave it always visible.

The radios are now visible native inputs. Until the stylesheet follow-up deletes the dead .theme-option rules, the picker's spacing comes from .check and .field, so look at it in the preview.

Closes #4700

commented

The theme picker in src/web/layout.ts now draws §5's field and button, and theme-option and theme-apply are gone from the markup. The only remaining references are the unused rules in src/web/style/components.ts and the line in docs/DESIGN.md, which the issue leaves for the follow-up.

The design lint and the tests for design-lint, web and flow-pipeline pass. bun x tsc --noEmit could not run here (it reports a missing bun type definition even after bun install), so the forge's check will be its first run. docs/design/baseline.json is unchanged because the lint counts did not move.

  • Markup: themeForm() is now a fieldset.field with the legend "Theme". Each choice is a label.check holding a radio and the existing swatch. "Apply" is a plain btn("Apply", { size: "sm" }).
  • Without JavaScript: the form still posts to /theme.
  • Client script: the script in src/web/client.ts finds the form by .theme-form, the button by button[type=submit] and the radios by .theme-form input. It still hides Apply and applies the theme on change.
  • Tests: the themes test in test/web.test.ts asserts the choices, the Apply button and that the old classes are absent. test/flow-pipeline.test.ts follows the selector change.

I dropped the check-mark .tick. The native radio now shows the selection, and the old stylesheet rule that drew the tick would otherwise leave it always visible.

The radios are now visible native inputs. Until the stylesheet follow-up deletes the dead .theme-option rules, the picker's spacing comes from .check and .field, so look at it in the preview.

Approved

Does what the issue asked. The preview's theme form is a fieldset.field with a legend, seven label.check radios that keep their swatch, and a plain btn Apply. It posts to /theme with JavaScript off, and the old classes are gone from the markup. The shell script's selectors moved with it, and the tests assert both the new markup and the absence of the old classes. Radios are real inputs, so the keyboard and screen-reader path is better than the hidden-radio version. Two things I would do next:

  1. Delete the dead .theme-option, .tick and .theme-apply rules in src/web/style/components.ts (about lines 634-648 and 656) and the matching names in docs/DESIGN.md's components.ts list. Scout says this is the stated follow-up.
  2. The form sits in .menu-panel, which has only 5px padding. The legend and the radios now sit nearly flush with the panel edge, and fieldset.field adds a 16px bottom margin above Apply. Give .theme-form a small padding (about 6px 10px) and drop that bottom margin. On a 360px phone, seven 44px coarse-pointer rows will scroll inside the panel's 70vh cap, so check it with a thumb.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs