|
| 1 | +--- |
| 2 | +phase: 260508-d7k |
| 3 | +plan: 01 |
| 4 | +subsystem: FastSenseCompanion/applyThemeToChildren_ |
| 5 | +tags: [theme-walker, dark-mode, uilistbox, companion, tdd] |
| 6 | +dependency_graph: |
| 7 | + requires: [] |
| 8 | + provides: [expanded-walker-coverage] |
| 9 | + affects: [TagCatalogPane, CompanionSettingsDialog, FastSenseCompanion] |
| 10 | +tech_stack: |
| 11 | + added: [] |
| 12 | + patterns: [TDD-red-green, recursive-walker, switch-case-expansion] |
| 13 | +key_files: |
| 14 | + created: |
| 15 | + - tests/test_companion_apply_theme_walker.m |
| 16 | + modified: |
| 17 | + - libs/FastSenseCompanion/private/applyThemeToChildren_.m |
| 18 | +decisions: |
| 19 | + - "Button-like StateButton and ToggleButton use WidgetBorderColor (neutral) for BackgroundColor — same rule as the existing Button case — so the active-state repaint owned by each pane's setTheme override is not overwritten by the walker." |
| 20 | + - "CheckBox and RadioButton receive FontColor only; no BackgroundColor because these controls inherit their background from the parent container in MATLAB's UIFigure system." |
| 21 | + - "ButtonGroup receives BorderColor in addition to BackgroundColor because it renders a visible border like Panel." |
| 22 | + - "Prophylactic cases (StateButton, ToggleButton, RadioButton, NumericEditField) are added unconditionally to the walker switch even though some MATLAB versions do not support those constructors — a never-matched switch string is free and future-proofs the walker." |
| 23 | +metrics: |
| 24 | + duration: "< 5 min" |
| 25 | + completed: "2026-05-08" |
| 26 | + tasks_completed: 1 |
| 27 | + files_modified: 2 |
| 28 | +--- |
| 29 | + |
| 30 | +# Phase 260508-d7k Plan 01: Expand Companion theme walker (uilistbox + 7 prophylactic classes) Summary |
| 31 | + |
| 32 | +Expanded `applyThemeToChildren_` to cover `uilistbox` (fixing the reported tag-catalog dark-mode regression) and 7 prophylactic widget classes, with a TDD regression test that asserts the dark->light->dark colour flip. |
| 33 | + |
| 34 | +## What Was Built |
| 35 | + |
| 36 | +### Root Cause |
| 37 | + |
| 38 | +`applyThemeToChildren_` used a `switch class(ch)` block with an `otherwise` silent-skip branch. The `matlab.ui.control.ListBox` class (used by `uilistbox()`) had no explicit `case`, so every theme switch left the tag list (`TagCatalogPane` Row 7) on whatever palette it was constructed with (white by default). The user symptom — "tag catalog stays light when switching to dark" — maps directly to this missing case. |
| 39 | + |
| 40 | +### Widget Class Coverage Audit |
| 41 | + |
| 42 | +| Widget class | Constructor | Status before | Status after | |
| 43 | +|---|---|---|---| |
| 44 | +| `matlab.ui.container.Panel` | `uipanel` | covered | covered | |
| 45 | +| `matlab.ui.container.GridLayout` | `uigridlayout` | covered | covered | |
| 46 | +| `matlab.ui.container.ButtonGroup` | `uibuttongroup` | NOT covered | covered (NEW) | |
| 47 | +| `matlab.ui.control.Label` | `uilabel` | covered | covered | |
| 48 | +| `matlab.ui.control.EditField` | `uieditfield` | covered | covered | |
| 49 | +| `matlab.ui.control.DropDown` | `uidropdown` | covered | covered | |
| 50 | +| `matlab.ui.control.Spinner` | `uispinner` | covered | covered | |
| 51 | +| `matlab.ui.control.Button` | `uibutton` | covered | covered | |
| 52 | +| `matlab.ui.control.Table` | `uitable` | covered | covered | |
| 53 | +| `matlab.ui.control.ListBox` | `uilistbox` | NOT covered (BUG) | covered (NEW) | |
| 54 | +| `matlab.ui.control.TextArea` | `uitextarea` | NOT covered | covered (NEW) | |
| 55 | +| `matlab.ui.control.CheckBox` | `uicheckbox` | NOT covered | covered (NEW) | |
| 56 | +| `matlab.ui.control.NumericEditField` | `uinumericeditfield` | NOT covered | covered (NEW) | |
| 57 | +| `matlab.ui.control.StateButton` | `uistatebutton` | NOT covered | covered (NEW) | |
| 58 | +| `matlab.ui.control.ToggleButton` | `uitogglebutton` | NOT covered | covered (NEW) | |
| 59 | +| `matlab.ui.control.RadioButton` | `uiradiobutton` | NOT covered | covered (NEW) | |
| 60 | + |
| 61 | +### Files Modified |
| 62 | + |
| 63 | +**`libs/FastSenseCompanion/private/applyThemeToChildren_.m`** |
| 64 | +- Added `matlab.ui.container.ButtonGroup` case (recurses into children like Panel/GridLayout) |
| 65 | +- Added `matlab.ui.control.ListBox` case: `BackgroundColor = WidgetBackground`, `FontColor = ForegroundColor` |
| 66 | +- Added `matlab.ui.control.TextArea` case: same as ListBox |
| 67 | +- Added `matlab.ui.control.CheckBox` case: `FontColor = ForegroundColor` only (no BackgroundColor — inherits from parent) |
| 68 | +- Added `matlab.ui.control.NumericEditField` case: same as EditField |
| 69 | +- Added `matlab.ui.control.StateButton` case: `BackgroundColor = WidgetBorderColor`, `FontColor = ForegroundColor` |
| 70 | +- Added `matlab.ui.control.ToggleButton` case: same as StateButton |
| 71 | +- Added `matlab.ui.control.RadioButton` case: `FontColor = ForegroundColor` only (no BackgroundColor — inherits from parent) |
| 72 | +- Updated function header comment with expanded coverage inventory |
| 73 | + |
| 74 | +**`tests/test_companion_apply_theme_walker.m`** |
| 75 | +- New Octave-style function test (MATLAB-only, skips on Octave) |
| 76 | +- Builds a hidden `uifigure('Visible','off')` with all covered widget classes |
| 77 | +- Prophylactic widgets (`uistatebutton`, `uinumericeditfield`) wrapped in `tryMakeWidget` for version portability |
| 78 | +- `uitogglebutton` and `uiradiobutton` parented to separate `uibuttongroup` instances (MATLAB requirement) |
| 79 | +- Asserts dark->light->dark colour flip for all available widget classes (18 assertions on current MATLAB version) |
| 80 | +- `onCleanup(@() delete(hFig))` prevents figure leak on test failure |
| 81 | +- Local `add_companion_private_path()` helper mirrors the pattern in `add_fastsense_private_path.m` with R2025b+ proxy fallback |
| 82 | + |
| 83 | +## TDD Execution |
| 84 | + |
| 85 | +| Step | Commit | Result | |
| 86 | +|---|---|---| |
| 87 | +| RED: failing test for uilistbox + 7 prophylactic classes | `93312b8` | FAIL: uilistbox BackgroundColor was [1 1 1], expected [0.09 0.13 0.24] | |
| 88 | +| GREEN: expand walker cases | `101ab9c` | PASS: All 18 tests passed | |
| 89 | + |
| 90 | +## Deviations from Plan |
| 91 | + |
| 92 | +### Auto-fixed Issues |
| 93 | + |
| 94 | +**1. [Rule 1 - Bug] Wrapped additional widget constructors in tryMakeWidget** |
| 95 | +- **Found during:** RED step |
| 96 | +- **Issue:** `uistatebutton` and `uinumericeditfield` are not available in the local MATLAB version (Home License). The test crashed at widget construction before reaching the uilistbox assertion. |
| 97 | +- **Fix:** Added `tryMakeWidget` wrappers for all prophylactic widgets (`uistatebutton`, `uitextarea`, `uicheckbox`, `uinumericeditfield`, `uitogglebutton`, `uiradiobutton`, `uibuttongroup`). Assertions for unavailable widgets are skipped. `uilistbox`, `uieditfield`, `uilabel`, etc. are constructed directly since they are used in production code today and confirmed available. |
| 98 | +- **Files modified:** `tests/test_companion_apply_theme_walker.m` |
| 99 | +- **Commit:** included in RED commit `93312b8` |
| 100 | + |
| 101 | +**2. [Rule 1 - Bug] uitogglebutton/uiradiobutton require ButtonGroup parent** |
| 102 | +- **Found during:** RED exploration |
| 103 | +- **Issue:** `uitogglebutton` and `uiradiobutton` throw "Parent must be a ButtonGroup" when parented to a GridLayout. The plan's try/catch guidance assumed they might be unavailable, but the real constraint is parent type. |
| 104 | +- **Fix:** Created dedicated `uibuttongroup(hFig)` instances as parents for each. `hBgToggle` parents `uitogglebutton`; `hBgRadio` parents `uiradiobutton`. Both groups wrapped in `tryMakeWidget` so Octave or versions without these classes still work. |
| 105 | +- **Files modified:** `tests/test_companion_apply_theme_walker.m` |
| 106 | +- **Commit:** included in RED commit `93312b8` |
| 107 | + |
| 108 | +## Known Drift Risk (Out of Scope) |
| 109 | + |
| 110 | +The two log tables in `FastSenseCompanion.m` (lines 745-749 and 787-789) hardcode the same dark/light RGB pairs that `applyThemeToChildren_` already derives from `theme.DashboardBackground`. This duplication is NOT the source of the reported bug — the walker already overwrites the tables on each theme apply. However, if someone refines the `tableBg` derivation logic inside the walker without updating these construction-time hardcodes, the tables will show the wrong palette briefly during first construction before the first `applyTheme` call. |
| 111 | + |
| 112 | +**Recommended follow-up:** A small DRY-up task to replace the hardcoded pairs at lines 745-749 and 787-789 with `CompanionTheme.get(preset).DashboardBackground`-derived values at construction time. Backlog item suggested. |
| 113 | + |
| 114 | +## Self-Check: PASSED |
| 115 | + |
| 116 | +- `tests/test_companion_apply_theme_walker.m` exists and passes (18 tests). |
| 117 | +- `libs/FastSenseCompanion/private/applyThemeToChildren_.m` exists and contains `case 'matlab.ui.control.ListBox'`. |
| 118 | +- RED commit `93312b8` confirmed in `git log`. |
| 119 | +- GREEN commit `101ab9c` confirmed in `git log`. |
0 commit comments