Skip to content

OXY-154: harden ColorModePicker into a standalone Light/Dark/System component - #303

Open
Kalin-Rudnicki wants to merge 2 commits into
OXY-154from
OXY-154-colormode
Open

OXY-154: harden ColorModePicker into a standalone Light/Dark/System component#303
Kalin-Rudnicki wants to merge 2 commits into
OXY-154from
OXY-154-colormode

Conversation

@Kalin-Rudnicki

Copy link
Copy Markdown
Owner

Summary

Hardens ColorModePicker (from OXY-154) into a first-class, standalone Light / Dark / System control. It is now fully independent of the theme-pack machinery — it depends only on the ColorMode service and stands on its own even if ThemePicker didn't exist. ThemePicker is unchanged.

Stacks on #296 (OXY-154). This PR is based on the OXY-154 branch so the diff shows only the new work. Merge/rebase #296 first.

What changed

  • Config-builder widget (à la ToggleThumb / HorizontalRadio), so instances are droppable widgets and fluently configurable. Two variants:
    • Segmented (role=radiogroup, default)
    • Compact — a single icon button that cycles modes (top-bar friendly)
  • Config knobs: .small/.medium/.large, .label(..)/.noLabel, .includeSystem(..)/.lightDarkOnly, .withIcons/.noIcons, .idPrefix(..). Backward-compatible apply() / apply(label = ...) retained — existing call sites unchanged.
  • a11y hardening: roving tabindex, role=radio + aria-checked per option, and keyboard nav (Left/Up = prev, Right/Down = next, Home/End = first/last with wrap, Space/Enter select) that moves selection and DOM focus together. Compact is a native <button> with a descriptive aria-label/title.
  • Cross-tab / programmatic live updates: new opt-in ColorModePicker.syncAcrossTabs subscribes to the Broadcast color-mode channel and re-highlights every mounted picker (addresses the OXY-154 "does not re-highlight on cross-tab changes" caveat). Off by default — no bootstrap behavior change.
  • Pure decision logic extracted into ColorModePicker.Logic and unit-tested (6 new DOM-free tests).
  • Showcase: ThemePage now demos the variants. Docs: docs/docs/ui/builders.md updated.

Decoupling note

ColorModePicker never had a code-level dependency on ThemePicker (they only shared a pattern). Confirmed and kept its shared state seeded via ColorMode.storageKey / ColorMode.parse — no reference to Theme / OxygenThemes / ThemePicker.

A native <select>/dropdown "form" variant was intentionally not added: labeled form contexts are already served by the generic HorizontalRadio.form[ColorMode.Mode].

Verification

  • oxygen-ui-web/compile
  • example-ui-web/compile
  • oxygen-ui-web/test ✅ (51 tests, incl. 6 new ColorModePicker.Logic tests)
  • sbt fmt applied. No regressions to ColorMode / Theme / ColorTheme / ThemePicker or existing call sites.

🤖 Generated with Claude Code

https://claude.ai/code/session_011YxWKdsz97QT9BD7AdpSq6

…omponent

Rework the reusable color-mode picker into a first-class, standalone widget,
fully independent of the theme-pack machinery (depends only on the ColorMode
service).

- Config-builder widget (à la ToggleThumb / HorizontalRadio) with two variants:
  Segmented (role=radiogroup, default) and Compact (single icon cycle button).
- Config knobs: small/medium/large, label/noLabel, includeSystem/lightDarkOnly,
  withIcons/noIcons, idPrefix. Backward-compatible apply()/apply(label=...).
- a11y: roving tabindex, role=radio + aria-checked per option, and keyboard
  navigation (arrows / Home / End / Space / Enter) that moves selection and DOM
  focus together; compact is a native button with descriptive aria-label/title.
- Extracted pure decision logic (ColorModePicker.Logic) and unit-tested it.
- Opt-in ColorModePicker.syncAcrossTabs subscribes to the Broadcast color-mode
  channel and re-highlights every mounted picker on cross-tab / programmatic
  changes.
- Showcase (ThemePage) demos the variants; docs updated.

No changes to ColorMode / Theme / ColorTheme / ThemePicker behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011YxWKdsz97QT9BD7AdpSq6
@Kalin-Rudnicki

Copy link
Copy Markdown
Owner Author

🤖 Automated multi-agent review (confirmed findings)

Medium

Keyboard nav focuses wrong picker when multiple segmented pickers share default idPrefix
modules/ui/web/src/main/scala/oxygen/ui/web/component/ColorModePicker.scala:207
Every segmented picker built without an explicit .idPrefix(...) emits option elements with the same ids (default prefix "oxygen-color-mode"), so getElementById in focusOption always resolves to the first picker. On ThemePage three segmented pickers share one page — tabbing into the 2nd/3rd and pressing ArrowRight/Left/Home/End moves focus to the 1st picker's option, breaking roving-tabindex nav (and producing invalid duplicate-id HTML).

Low

iconFor test never pins System→monitor, so a wrong-but-distinct System icon passes
modules/ui/web/src/test/scala/oxygen/ui/web/style/OxygenColorSystemSpec.scala:476
The test asserts names.size == 3 plus Light == "sun" and Dark == "moon", but never asserts iconFor(System).name == "monitor". Changing iconFor(System) to a distinct-but-wrong glyph (e.g. gear) keeps the set size at 3 and both name assertions green, so the wrong System icon ships undetected. Add assertTrue(iconFor(System).name == "monitor").


These are AI-generated, adversarially-verified findings and may still contain false positives — please confirm before acting.

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.

1 participant