From 19e92e0cc026cd0beda2e00e98b48cbb7335f3e3 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Wed, 19 Aug 2026 09:03:56 +0000 Subject: [PATCH] docs: review document for #180 Issue: #180 User-Visible: no --- docs/reviews/CODE-REVIEW-180-r2.md | 148 +++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-180-r2.md diff --git a/docs/reviews/CODE-REVIEW-180-r2.md b/docs/reviews/CODE-REVIEW-180-r2.md new file mode 100644 index 00000000..fabe7db6 --- /dev/null +++ b/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-шаблонов `` (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`.