From 67ac9981cfa2613c84472bb3dfd03afcabfbf81c Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 05:52:09 +0000 Subject: [PATCH] docs: review document for #686 Issue: #686 User-Visible: no --- docs/reviews/CODE-REVIEW-686-r1.md | 68 ++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-686-r1.md diff --git a/docs/reviews/CODE-REVIEW-686-r1.md b/docs/reviews/CODE-REVIEW-686-r1.md new file mode 100644 index 00000000..c31b73a5 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-686-r1.md @@ -0,0 +1,68 @@ +# CODE-REVIEW-686-r1 + +**Issue:** [#686](https://github.com/Matysh/houseplan-card/issues/686) — «Лестница в View: при фокусе вокруг неё рисуется чёрно-белая рамка браузера» +**Материал:** `c455e7c82dfd358377eca2f60c38184c42acc79d` (ровно этот SHA, рабочая копия — на нём; проверено `git rev-parse HEAD`) +**Трек:** trivial · **Заход:** r1 · блокирующих циклов израсходовано 0 из 2 + +## Скоуп + +Один коммит поверх `dev@03a3dd99` (кандидат beta.7): `.hp-stair:focus, .hp-stair:focus-visible { outline: none; }` в `src/styles/plan.styles.ts`, свидетель в `demo/smoke_stairs.mjs`, правка `docs/STAIRS.md`, оба changelog. Чисто CSS-фикс: убрать браузерную рамку фокуса у лестницы-перехода в View, не трогая фокусируемость (Tab/Enter/Space, #676) и hover/выделение в редакторе (#663/#676). Работа обслуживает J1 SCOPE.md (корректность рендера уже принятой поверхности View), нового продуктового контракта не создаёт — решение владельца зафиксировано в теле issue. + +## Как проверялось + +Прочитаны: `docs/SCOPE.md`, `docs/process/REVIEWER.md`, `AGENTS.md`, тело issue #686 и его комментарии (оценка автора, взятие в работу, отчёт о реализации), `docs/STAIRS.md`, канонический код (`src/stairs-view.ts`, `src/stairs-editor.ts`, `src/space-render.ts`). + +Разобран код правила и его область действия: `tabindex="0"`/`role="link"` в `stairs-view.ts:92` выставляются только когда `active` (View + `targetState === 'active'`); в редакторе (`stairs-editor.ts:517`) тот же класс `.hp-stair` рендерится с `role="img"` без `tabindex` — не фокусируется вообще, значит новое правило `outline: none` физически не может задеть `.selected` (AC3 подтверждён и структурно, не только диффом). + +Гейты, которые прогнал сам (не полагаясь только на Validate, потому что дельта — единственный `src`-файл, и это дёшево): + +| Гейт | Команда | Результат | +|---|---|---| +| typecheck+build (через bundle:sync) | `npm run bundle:sync` | OK, `tsc --noEmit` и rollup прошли чисто | +| Целевой smoke AC1/AC2 | `node demo/smoke_stairs.mjs` (после `bundle:sync`) | `OK`, все 66 полей true, включая 4 новых: `focusProbeHasActiveStair`, `keyboardFocusReachesStair`, `focusedStairPaintsNoFrame`, `focusedStairEnterNavigates` | +| **Мутация (тест умеет падать)** | вручную вырезал правило `.hp-stair:focus, .hp-stair:focus-visible { outline: none; }` из `plan.styles.ts`, пересобрал (`bundle:sync`), перезапустил смок | смок покраснел ровно на заявленном месте: `FAILED (1): focusedStairPaintsNoFrame: expected true, got false` — остальные 65 полей не пострадали | +| Восстановление | `cp` бэкапа исходника обратно, `bundle:sync`, `git checkout -- dist && git clean -fd dist` | `git status` пуст, рабочая копия снова на `c455e7c8` без остаточных изменений | +| `check-docs.mjs --screenshots=warn` | `node scripts/check-docs.mjs --screenshots=warn` | `WARN` про несвежий отпечаток скриншотов (#479, сдвигается любой правкой `src/**`, не относится к этой задаче) + `Documentation checks passed` | +| `smoke-select.mjs` | `node scripts/smoke-select.mjs --base 03a3dd99 --head c455e7c8` | «НЕОПРЕДЕЛЁННОСТЬ» (диф исполняемый, но 0 символов проекта на изменённых строках CSS) — ожидаемо для чистого CSS-правила, автор верно закрыл её явным прогоном именованного в AC смока | +| `git log -1 --format=%B` | — | трейлеры `Issue: #686`, `User-Visible: yes` на месте; `docs/CHANGELOG.md` и `docs/CHANGELOG.ru.md` правлены в том же коммите | +| Golden | не гонял, обосновано чтением | `grep` по `demo/golden/**` не находит ни одной сцены, комбинирующей `stair` и `focus`; правка не может задеть golden-снимки | + +Не прогонял и почему: `npx tsc --noEmit`/`npm test`/`npm run build` со сверкой трёх копий бандла отдельно — Validate на этом SHA уже зелёный (run 36382924353), а `tsc --noEmit`+build я всё равно исполнил через `bundle:sync` при мутационном тесте (тот же компилятор, тот же rollup); отдельный `npm test` не гонял — диф не трогает ничего, что покрыто unit-тестами (`test/*.test.mjs` не содержит `hp-stair`/`plan.styles`), а typecheck+build уже дважды подтверждён (до и после мутации). `npm run invariants` — не применимо, диф не трогает геометрию модели. `python -m pytest tests_backend` — не применимо, Python не тронут. Performance-профили — не названы в AC. Полные наборы намеренно не гонял (это предрелизный гейт, не гейт ревью, §8). + +## Находки + +Ничего блокирующего или требующего правки не найдено. + +**Low (снято ревьюером, без правки):** `docs/STAIRS.md` — новое предложение про фокус вставлено в середину существующего абзаца, и последняя строка получившегося абзаца («…follow it. Pan, pinch, long press, swipe and pointer cancellation do not navigate.») заметно длиннее соседних строк (файл обычно перевёрнут на ~78–80 знаков). Чисто оформление источника markdown, на отрендеренный текст не влияет (мягкий перенос абзаца), поэтому не поднимаю до Medium и не возвращаю автору. + +## Что проверено и корректно + +- **AC1** (фокус ничего не рисует): доказано смоком `focusedStairPaintsNoFrame` — побайтовое совпадение снимков области лестницы без фокуса и с клавиатурным `:focus-visible`-фокусом (запас 8px на кольцо). Сам прогнал смок на `c455e7c8` — зелёный; сам вырезал правило и убедился, что этот же смок падает именно на этом поле — оракул реален, а не тавтология. Ветка «фокус без `:focus-visible`» (клик/тап) доказана чтением: то же правило покрывает голый `:focus`; автор верно объяснил отсутствие исполняемого отрицательного свидетеля (Chromium не рисует кольцо по клику даже без правила) — это не защитный AC в смысле §2.7 (не валидация/гард/лимит/отказ/инвариант), а чисто визуальный CSS-фикс, так что допустимо разобрать чтением. +- **AC2** (переход работает как прежде): доказано смоком — новое поле `focusedStairEnterNavigates` (Enter на сфокусированной лестнице переключает вкладку) плюс все прежние 58 проверок `smoke_stairs.mjs` остались зелёными (клик, спираль, 2.5D, подавление жестов, инертность недействительных/фиксированных целей). +- **AC3** (hover и выделение в редакторе не меняются): доказано и чтением диффа (правила `:hover`/`.selected` не тронуты), и структурно — элемент `.hp-stair` в редакторе (`stairs-editor.ts:517`) не имеет `tabindex`, то есть физически не может получить фокус, на который реагирует новое правило. +- Трейлеры коммита (`Issue: #686`, `User-Visible: yes`) на месте, оба changelog правлены в этом же коммите тем же числом-ссылкой на issue. +- Один источник числа: единственная новая цифра в диффе — margin `8` (px, запас на кольцо в смоке) — используется один раз, дублирования нет. +- Риск (не видно, что фокус на лестнице с клавиатуры) — явно принят владельцем в теле issue и в комментарии-оценке, не является скрытым продуктовым регрессом. + +## Чего не проверял + +- Полные наборы `golden:verify`, весь `npm test`, `demo/performance/*`, `pytest tests_backend`, `npm run invariants` — не применимо к этому диффу (обоснование выше) и являются предрелизным, а не ревью-гейтом. +- Реальное поведение в HA companion app / старых WebView (комбинированный селектор `:focus, :focus-visible` в одном списке теоретически невалиден целиком в UA без поддержки `:focus-visible`) — не проверял на реальных старых движках; риск считаю пренебрежимым, так как кодовая база уже повсеместно полагается на `:focus-visible` без резервных правил (`chrome.styles.ts:316`, `devices.styles.ts:347` и др.) — это не новый риск, вносимый этим диффом, а существующее допущение проекта. + +## Вердикт + +Зелёный. AC1–AC3 доказаны (два — исполняемым смоком с подтверждённой мутацией, один — чтением с структурным подтверждением), трейлеры и changelog в порядке, гейты, применимые к диффу, прогнаны и зелёные, единственная находка — Low, снята без правки. + +--- + + + +## Материал раунда + +- Ветка: `issue/686-stair-focus-frame`, коммит `c455e7c82dfd` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `329767ba99a70c7f7df3a291f4e80cfbef72b6cb` + ``` + git log --all --format='%H %T' | grep 329767ba99a7 + ``` +- Тело issue: `feb343fa507c3f87355317c9df029f27fd3f9957111add77d89b7c2e176c42f1` +- Вердикт конвейера: `green` · High 0