diff --git a/docs/reviews/SPEC-REVIEW-362-r1.md b/docs/reviews/SPEC-REVIEW-362-r1.md new file mode 100644 index 00000000..d9909933 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-362-r1.md @@ -0,0 +1,117 @@ +# SPEC-REVIEW-362-r1 + +Issue: #362 — «Редактор подложки: устройства перехватывают hover и действия инструментов» +Трек: `small` (лёгкий) · заход r1 · блокирующих циклов ревью ТЗ 0 из 2 +Материал: тело issue #362 на момент разбора (createdAt/updatedAt 2026-08-29), последний +комментарий автора «ТЗ готово, передача на ревью (small track, r1)». +Дерево кода: ветка `dev`, HEAD `156be645` (issue ещё не в коде — ревью читает только +описание поведения и сверяет его с фактическим состоянием `dev`, из которого фикс будет +писаться). + +## Скоуп + +Баг в редакторе Background (Подложка): видимая часть device layer (иконка, 44px +псевдо-область, shell/frame, капсула, value/LQI/бейджи, пульсы, спутники +opening-lock) остаётся pointer target и общими device-обработчиками, хотя по +`docs/DECOR-EDITOR.md` устройства в Background — чисто контекстный ориентир на +35% opacity. Из-за этого устройство перехватывает hover/tooltip, может открыть +карточку, выполнить toggle/run или начать drag устройства вместо действия +активного инструмента Подложки (Line/Rectangle/Text/Furniture), и мешает +поставить декоративный элемент в нужную точку. + +Продуктовая рамка (`docs/SCOPE.md`): задача в J4 («GUI-only редактирование +плана») и J6 («drag/resize, два редактора») — восстанавливает уже +задокументированный (не новый) UX-контракт Background, а также защищает +инвариант «никакой lock/alarm-экшен не срабатывает по тапу на плане» той же +секции SCOPE.md, поскольку сейчас клик по устройству в Подложке может +инициировать реальный HA-service-call. Правка входит в скоуп, новой персоны или +поверхности не создаёт. + +## Как проверялось + +Ревью ТЗ на `small`-треке не предполагает файла в `docs/specs/` — проверено, +файла `docs/specs/362-*.md` в дереве нет, как и требуется §5 PROCESS.md. + +Каждое техническое утверждение ТЗ («Подтверждение по коду») сверено чтением +актуального `dev` (`156be645`), а не принято на слово автора: + +| Утверждение ТЗ | Файл : строка | Результат сверки | +|---|---|---| +| `.stage.mode-decor .devlayer { pointer-events: none }` уже есть, но перекрывается | `src/styles/plan.styles.ts:856` | подтверждено — правило есть | +| `.dev`, `.dev::before`, `.device-shell-frame` явно `pointer-events: auto` | `src/styles/devices.styles.ts:144,182,213` | подтверждено дословно | +| `_renderDevice()` вешает click/contextmenu/pointerover/pointermove/pointerdown/up без учёта Background | `src/houseplan-card.ts:12017-12029` | подтверждено; `role`/`tabindex` действительно ограничены `interactive = view\|devices` (:11980, :12012-13) | +| `_clickDevice()` блокирует только `plan`, Background проваливается в общий action-путь | `src/houseplan-card.ts:5179-5182` | подтверждено — только `if (this._mode === 'plan') return;`, ветки `devices` и общий tap-action идут дальше | +| `_pointerDown()` блокирует `plan`, отдельно обрабатывает `view`, всё остальное (включая Background) идёт в drag-путь устройства | `src/houseplan-card.ts:6337-6361` | подтверждено — после `plan`/`view` сразу `this._drag = {...}` | +| `_showTip()` не проверяет режим вовсе | `src/houseplan-card.ts:6549-6561` | подтверждено — единственная проверка `hoverEnabled`/`_drag`, режим не участвует | +| `_ctxDevice()` (правый клик) уже fail-closed вне View | `src/houseplan-card.ts:5170-5177` | **не баг** — `if (this._mode !== 'view') return;` уже безопасен для Background; ТЗ верно требует лишь сохранить это регрессионной проверкой, не выдаёт это за новый фикс | +| `_selId` (device selection) выставляется только в `_pointerUp` при `moved` | `src/houseplan-card.ts:6388-6396` (единственное присваивание — `houseplan-card.ts:6394`) | подтверждено — фикс `_pointerDown` автоматически закрывает и «device selection» без отдельного гварда | +| `:hover`-покраска CSS не зависит от JS-обработчиков, а только от того, является ли элемент pointer target | `src/styles/devices.styles.ts:329-407` (`:host([data-pointer-hover]) .dev:hover` и т. п.) | подтверждено — это обосновывает контрактный пункт 6 («CSS явно перекрывает descendants, JS-гварды одни pointer-events не заменяют») как техническую необходимость, а не избыточность | +| Терминология «Подложка» / «Просмотр» / «Устройства» | `docs/USER-GUIDE.ru.md:56,109,120,198,1090` и др. | совпадает с интерфейсом, не изобретена | +| Ссылки на дубликаты #101, #213, #266 | `gh issue view 101/213/266` | #213 — расширение hover/action до всей капсулы (источник «капсулы» в ТЗ), #266 — чисто структурный CSS-рефакторинг, #101 — переход View↔редакторы/фон; ни один не дублирует маршрутизацию событий Background, ссылки корректны | +| `demo/smoke_decor.mjs` существует, есть куда добавлять AC1/AC4 | `ls demo/smoke_decor.mjs` | подтверждено | + +Гейты кода (typecheck/test/build/смоки) не запускались: на этапе ревью ТЗ кода +ещё нет, проверке подлежит только описание поведения и его соответствие +текущему `dev`. + +## Находки + +Нет. High — 0, Medium — 0, Low — 0. + +Рассмотренные и отклонённые кандидаты на находку: + +- *AC5 не называет конкретные файлы existing device/decor smokes.* Отклонено: + AC5 — регрессионная, а не основная проверка; ТЗ явно допускает альтернативное + доказательство «чтение mode guards на код-ревью», выбор конкретных смоков — + техническое решение реализации, не продуктовый вопрос, ревьюер не вправе его + требовать на этом треке. +- *Контракт требует сохранить fail-closed для `_ctxDevice`, хотя эта ветка уже + безопасна.* Отклонено как находка: ТЗ не выдаёт существующее поведение за + новый фикс, а включает его в перечень side-effects, которые обязана держать + регрессия — это корректная практика, не догадка. +- *Не назван отдельный вопрос владельцу.* Отклонено: ни один открытый пункт не + требует продуктового решения (что видит/делает человек) — контракт нигде не + меняет видимое поведение сверх уже описанного в `DECOR-EDITOR.md` (35% + opacity, устройства вне magnet targets), только устраняет утечку + интерактивности. Догадок, выданных за факт, не обнаружено — каждое + поведенческое утверждение либо сверено с кодом (таблица выше), либо с + `DECOR-EDITOR.md`/`UX-MODES.md`. + +## Что проверено и корректно + +- Обязательные разделы лёгкого трека (§5 PROCESS.md): Проблема · Контракт · + AC1…AC6 с указанным способом доказательства · Откат — все присутствуют в теле + issue. +- Каждый AC — проверяемое утверждение с явным способом доказательства + (`smoke_decor.mjs`, browser smoke со spies/counters, DOM/computed-style + smoke, diff review + typecheck/unit/build). AC3 отдельно требует, чтобы + мутация (возврат `pointer-events:auto` или снятие mode guard) роняла smoke — + дисциплина «тест умеет падать» заложена уже в ТЗ. +- Скоуп: одна поверхность (Background), нет миграции конфига, нет новых + compatibility/i18n-полей, нет влияния на touch-контракт и performance — + критерии `small` (§5 PROCESS.md) выполнены одновременно, трек выбран верно. +- Контракт не вводит новый UX, а восстанавливает уже задокументированный в + `docs/DECOR-EDITOR.md` (35% opacity, устройства исключены из magnet targets) + и `docs/UX-MODES.md` (Background — desktop-first editor, View/Devices не + меняются). +- Откат описан конкретно и с явным запретом частичного отката (CSS без guards + или наоборот), что верно устраняет риск оставить один из двух путей + случайного действия. +- Технические утверждения о текущем состоянии кода все подтверждены прямым + чтением `dev`@`156be645` (таблица выше) — ни одно не оказалось устаревшим или + неточным. + +## Чего не проверял + +- Сам фикс не написан — код-ревью впереди, эта проверка не про реализацию, а + про план работы. +- Не проверялись гейты (`typecheck`/`test`/`build`/смоки) — на этапе ТЗ кода + нет, гонять их не над чем. +- Не проверялся live browser-стенд — вне скоупа ревью ТЗ. + +## Вердикт + +Зелёный. ТЗ полное, однозначное, каждый AC проверяем и привязан к способу +доказательства, технические утверждения подтверждены чтением кода, догадок, +выданных за факт, не найдено. Продуктовых вопросов владельцу нет и не +требуется.