docs: review document for #180

Issue: #180
User-Visible: no

docs/reviews/CODE-REVIEW-180-r2.md

@@ -0,0 +1,148 @@
+# Код-ревью #180 — r2
+
+Issue: [#180](https://github.com/Matysh/houseplan-card/issues/180)
+ТЗ: `docs/specs/180-all-color-call-sites.md`, ревью ТЗ: `docs/reviews/SPEC-REVIEW-180-r1.md` (зелёное, High 0/Medium 0)
+Предыдущий цикл: `docs/reviews/CODE-REVIEW-180-r1.md` — красный, High-1 (перекрытие подписей в строке "Цвет пульсации активности" / "Размер пульсации активности" на узком экране, `device-ripple-color-popover-mobile-ru`, 390px).
+Диапазон: `git diff origin/dev...HEAD`, коммиты `3540d24`, `f69ac71`, `fcee724`, `1bf90ee`, `9bde4b1`. Правка этого цикла — `9bde4b1` («Prevent ripple color label overlap»), реакция на r1.
+Вердикт: **зелёный** · цикл r2/4 · High: 0 · Medium: 0 → нет · Документ: `docs/reviews/CODE-REVIEW-180-r2.md`
+
+## Скоуп
+
+#180 переводит пять оставшихся source-шаблонов `<input type="color">` (15 полей:
+11 палитр общих настроек, глобальный статичный фон, marker activity/ripple color,
+цвет комнаты пространства, фон пространства) на общий `hp-color-opacity` из #57.
+Модель данных, backend, i18n-ключи не меняются. Этот цикл проверяет только
+дельту `9bde4b1` поверх уже проверенного в r1 диапазона: остальные AC1-6, AC8
+были разобраны и подтверждены в r1, здесь они не пересматриваются заново, кроме
+точечной перепроверки, что фикс не задел соседние места.
+
+Правка `9bde4b1`: строка marker-ripple разбита на два независимых flex-блока
+`.ripple-colorrow` (picker на всю ширину) и `.ripple-sizerow` (`.opl` с
+`min-width: 0`), вместо одной строки `.colorrow`, где `hp-color-opacity.label`
+(`white-space: nowrap`) конкурировал за место с соседним `.opl` в узком vieport.
+Добавлены: source/unit-регрессия в `test/color-picker.test.mjs`, геометрическая
+DOM-проверка в `demo/smoke_color_picker_consumers.mjs` (390×1000, реальный
+`getBoundingClientRect()` двух `.label`), обновление уже существующего
+`demo/smoke_bg_color.mjs` (селектор `input[type=color]`, который правка #180
+устранила, был мёртвым с `fcee724` и здесь наконец приведён в соответствие),
+записи в `docs/CHANGELOG.md`/`.ru.md` и `docs/TESTING.md`.
+
+## Как проверялось
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| Typecheck | `npx tsc --noEmit` | зелёный |
+| Unit | `npm test` | 897/897 зелёный (было 896 в r1, +1 новый regression-тест) |
+| Build + сверка бандлов | `npm run build && cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js && cmp dist/houseplan-card.js demo/srv/assets/houseplan-card.js` | зелёный, `git status` после сборки пуст — три закоммиченные копии бандла уже актуальны |
+| Целевой smoke (AC1-AC7, изменён этим циклом) | `node demo/smoke_color_picker_consumers.mjs` | зелёный, все 12 проверок `true`, включая новую `rippleLabelsDoNotOverlapOnMobile: true` |
+| Целевой smoke (тронут этим циклом) | `node demo/smoke_bg_color.mjs` | зелёный, все проверки `true`, включая обновлённую `dialogHasBgRow: true` |
+| Целевой smoke (общий контракт #57, не тронут этим циклом, но зависим) | `node demo/smoke_color_picker.mjs` | зелёный, все 16 проверок `true` |
+| Дисциплина «тест умеет падать» — DOM-смок | revert правки `9bde4b1` в `src/houseplan-card.ts`/`src/styles.ts` (`git apply -R`), пересборка, `cp dist → demo/srv/assets`, повторный прогон `smoke_color_picker_consumers.mjs` | `rippleLabelsDoNotOverlapOnMobile` упал в `false`, остальные 11 проверок остались `true` — ассерт специфичен именно к найденной регрессии, не к чему-то ещё; правка и бандлы возвращены на место сразу после (`git apply`, пересборка, `cmp` подтвердил три копии идентичны, `git status` пуст) |
+| Дисциплина «тест умеет падать» — unit | тот же revert, `node --test test/color-picker.test.mjs` | новый тест `activity color and ripple size keep independent readable rows` упал (`assert.match` на `class="colorrow ripple-sizerow"` не находит совпадения), остальные 5 тестов файла прошли — восстановление подтверждено тем же коммитом патча обратно |
+| Независимая геометрическая проверка (закрытый popover) | ad hoc Playwright-скрипт (`launch` из `demo/serve.mjs`), измерение `getBoundingClientRect()` `.ripple-colorrow` и `.ripple-sizerow` на 390×1000 без открытия picker | `colorRow` `y: 2077.5..2117.5`, `sizeRow` `y: 2123.5..2143.5` — строки идут одна под другой на всю ширину (332.6px каждая), зазор 6px, горизонтального соседства (источника прежнего перекрытия) больше не существует структурно, а не только по счастливой ширине текста |
+| Целевой golden capture (AC7, ровно сценарий, на котором в r1 было найдено High) | `npm run golden:capture -- --scenario=device-ripple-color-popover-mobile-ru` | рендерится без runtime-ошибки; `missing-baseline` ожидаемо (эталон не принят, pre-beta шаг); визуальный осмотр `artifacts/golden/actual/device-ripple-color-popover-mobile-ru.png` — открытый popover занимает большую часть узкого viewport, что затрудняет визуальную оценку текста под ним; решающим доказательством для AC7 считаю не скриншот, а прямой геометрический замер строкой выше, где popover не открывался |
+| `process-gate.mjs` (офлайн, без `--issues`) | `node scripts/process-gate.mjs` | «гейт пройден, предупреждений 0» на диапазоне `origin/dev..HEAD`, 5 коммитов — трейлеры, имя ветки, changelog при `User-Visible: yes`, лимит документов ревью (≤4) соблюдены |
+| `golden:verify` (полный) | не прогонялся | не нужен для ревью: для трёх новых сценариев эталонов нет по дизайну; в r1 уже прочитан `src/styles.ts` и подтверждено, что новые классы `.ripple-colorrow`/`.ripple-sizerow`/старые `.gsrow`/`.editor-secondary` не пересекаются; правка этого цикла добавляет ещё два новых, ещё более узких по охвату класса — тот же вывод остаётся в силе |
+| `python -m pytest tests_backend -q` | не прогонялся | диапазон не трогает `custom_components/**/*.py` |
+| Performance-профили | не прогонялись | ни AC, ни diff не называют performance-чувствительный путь; правка `9bde4b1` не добавляет новых observers/подписок, только реструктурирует существующий render (подтверждено чтением диффа) |
+| Полный набор `demo/smoke_*.mjs` (127 шт.) | не прогонялся | диапазон касается только color-picker consumers; остальные смоки не относятся к затронутым поверхностям (PROCESS.md §8) |
+
+## Находки
+
+Нет. High-1 из r1 устранён и подтверждён и чтением, и исполнением (DOM-смок,
+мутационная проверка, независимый геометрический замер). Новых находок не
+обнаружено.
+
+## Что проверено и корректно
+
+- **Устранение High-1.** Строка `.colorrow` с двумя текстовыми label заменена на
+  два отдельных полноширинных блока `.ripple-colorrow` (picker) и
+  `.ripple-sizerow` (`.opl` + range + значение). Это устраняет саму причину
+  перекрытия (горизонтальную конкуренцию `white-space: nowrap` label с соседним
+  `.opl` в одном flex-ряду), а не маскирует симптом уменьшением текста или
+  локальным `font-size`. Подтверждено: (а) мутационным тестом — откат правки
+  воспроизводит ровно исходный дефект и ничего больше; (б) независимым замером
+  `getBoundingClientRect()` при закрытом popover, где обе строки идут друг под
+  другом на всю ширину без горизонтального соседства.
+- **AC7 (визуальный/layout-контракт) для marker activity mobile.** Ранее
+  проваленный сценарий теперь доказан по коду и геометрией; остаточный визуальный
+  осмотр через golden не противоречит (popover открыт и закрывает собой большую
+  часть узкого экрана, что ожидаемо для 390px и не является новой проблемой —
+  тот же порядок открытия используется во всех golden-сценариях picker).
+- **AC5 (независимость ripple color/size) не нарушена структурной правкой.**
+  `rippleChangeLeavesSizeAndAlphaModelAlone` зелёный; `card._markerDialog.rippleSize`
+  не читается и не пишется в новом коде, только layout-обёртка вокруг
+  существующего `_rangeInput` вызова — подтверждено построчным чтением диффа
+  `src/houseplan-card.ts`.
+- **`demo/smoke_bg_color.mjs` приведён в соответствие с #180, а не оставлен
+  сломанным.** Старый ассерт `dialogHasBgRow` проверял
+  `.colorrow input[type=color]`, который правка #180 (ещё в `fcee724`) убрала —
+  проверка стала бы бессмысленной (всегда `false`→`falsy`, но интерпретировалась
+  как «нет строки фона» вместо «есть строка с picker»). Новая версия ищет
+  `hp-color-opacity` с нужным `label`, проверяет `showOpacity === false` и явно
+  подтверждает отсутствие native input — это восстанавливает содержательность
+  теста, а не просто гасит его. Подтверждено прогоном: все проверки файла
+  зелёные, включая `dialogHasBgRow: true`.
+- **Regression-тест в `test/color-picker.test.mjs` специфичен, а не
+  тавтологичен.** Мутационная проверка (откат правки → красный) показала, что
+  тест действительно завязан на структуру `.ripple-colorrow`/`.ripple-sizerow` и
+  порядок текста, а не на что-то тривиально всегда истинное.
+- **Трейлеры и changelog.** Коммит `9bde4b1`: `Issue: #180`, `User-Visible: yes`;
+  `docs/CHANGELOG.md` и `docs/CHANGELOG.ru.md` правятся в этом же коммите,
+  формулировка («на узких экранах цвет и размер пульсации находятся в отдельных
+  строках, поэтому обе подписи остаются читаемыми») соответствует факту диффа.
+  `docs/TESTING.md` обновлён тем же коммитом.
+- **`process-gate.mjs` офлайн-проверка зелёная** на весь диапазон `origin/dev..HEAD`
+  (5 коммитов), включая эту правку — трейлеры, имя ветки, лимит документов ревью,
+  обязательность changelog при `User-Visible: yes`.
+- **Дерево осталось чистым после всех локальных экспериментов.** Ревью включало
+  умышленный откат и восстановление продуктового кода для проверки, что тесты
+  умеют падать; после каждого шага `git status --porcelain` пуст, три копии
+  бандла идентичны (`cmp`), `npm test`/`npm run build` зелёные на итоговом дереве.
+- **Остальные AC (1-4, 6, 8), не тронутые этим циклом.** Уже разобраны и
+  подтверждены в `docs/reviews/CODE-REVIEW-180-r1.md` (раздел «Что проверено и
+  корректно»); точечно перепроверено здесь, что правка `9bde4b1` не касается
+  `_renderColorRow()`, global/space background, room color — диффом подтверждено,
+  что изменены только `src/houseplan-card.ts:19332-19343`-регион (marker dialog) и
+  два новых CSS-правила, специфичных новым классам.
+
+## Чего не проверял
+
+- **`golden:verify` (полный прогон, включая существующие baseline-сравнения).**
+  Не прогонял — та же причина, что в r1: для новых сценариев эталонов нет по
+  дизайну, полный набор — pre-beta гейт (PROCESS.md §8). Точечно перечитан
+  `src/styles.ts`: новые правила `.ripple-colorrow > hp-color-opacity` и
+  `.ripple-sizerow > .opl` используют классы, введённые этой же правкой и больше
+  нигде не встречающиеся, поэтому не пересекаются с существующими
+  golden-сценариями.
+- **`python -m pytest tests_backend -q`.** Не запускал — диапазон не касается
+  `custom_components/**/*.py`.
+- **Performance-профили.** Не запускал — ни AC, ни diff не называют
+  performance-чувствительный путь; правка не добавляет новых observers,
+  подписок или сетевых вызовов (подтверждено чтением диффа).
+- **Остальные 125 browser-смоков.** Не прогонялись — диапазон касается только
+  color-picker consumers; смоки других поверхностей не пересекаются с диффом.
+- **Реальный экран/устройство, скринридер, реальный multi-touch.** Как и в r1 —
+  вне обычного объёма гейтов код-ревью; структурная правка (два flex-блока
+  вместо одного) не меняет доступные имена/роли элементов, только их
+  геометрический контейнер, что подтверждено чтением диффа (никаких
+  атрибутов не тронуто).
+- **Визуальная приёмка golden-скриншота человеком/Linux CI.** Как и в r1, это
+  pre-beta шаг (`npm run golden:accept -- --reviewed`); я использовал захват
+  сценария как дополнительную, но не решающую проверку, потому что открытый
+  popover занимает большую часть узкого вьюпорта и не даёт чистой визуальной
+  оценки текста самой строки — решающим доказательством стал прямой
+  геометрический замер при закрытом picker.
+
+## Итог
+
+High-1 из r1 устранён структурно (разделение строки на два независимых
+полноширинных блока), а не косметически. Фикс подтверждён двумя независимыми
+методами измерения (DOM-смок с открытым popover, ad hoc замер с закрытым) и
+дисциплиной «тест умеет падать» — как для нового DOM-смока, так и для нового
+unit-теста источника. Побочно поднятая правка `smoke_bg_color.mjs` устраняет
+тест, обессмысленный ещё в `fcee724`, а не появившийся в этом цикле — исполнитель
+сам это заметил и исправил, что соответствует общему духу задачи (полное
+покрытие), не расширяя её AC. Новых находок нет, все AC ТЗ #180, ранее
+подтверждённые в r1 и переподтверждённые здесь после структурной правки,
+выполнены. Готово к `S8-merged`.