mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-28 19:01:34 +00:00
claude[bot]
@@ -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, снята без правки.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/686-stair-focus-frame`, коммит `c455e7c82dfd` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `329767ba99a70c7f7df3a291f4e80cfbef72b6cb`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 329767ba99a7
|
||||
```
|
||||
- Тело issue: `feb343fa507c3f87355317c9df029f27fd3f9957111add77d89b7c2e176c42f1`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user