12 KiB
CODE-REVIEW-686-r1
Issue: #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— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
329767ba99a70c7f7df3a291f4e80cfbef72b6cbgit log --all --format='%H %T' | grep 329767ba99a7 - Тело issue:
feb343fa507c3f87355317c9df029f27fd3f9957111add77d89b7c2e176c42f1 - Вердикт конвейера:
green· High 0