mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,191 @@
|
||||
# SPEC-REVIEW-578-r1
|
||||
|
||||
- Issue: #578 — «Touch: второй палец pinch на устройстве запускает long-press и HA more-info»
|
||||
- Этап: spec (PROCESS.md §2.4), заход r1, блокирующих циклов израсходовано 0 из 4
|
||||
- Материал: тело issue #578, раздел `## ТЗ` (сборка 14 разделов, план автотестов, принятые предположения), состояние `S4-spec-review`, комментариев к issue нет — это первый заход
|
||||
- Проверено на `dev` @ `ad4000f95a152d2cdd0fc41f3cb1707df121a47e` (рабочая копия уже на этом SHA)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Ревью ТЗ, не кода: продукт ещё не изменён. Вопрос — выполнимо ли и однозначно ли
|
||||
ТЗ, доказуем ли каждый AC, не выдана ли догадка за факт, укладывается ли трек
|
||||
(полный) и не требуется ли продуктовый вопрос владельцу.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитан `docs/SCOPE.md` — задача чинит нарушение touch-контракта в View/kiosk
|
||||
(персоны «Household members», «Guests/kiosk», §«View mode is the product»);
|
||||
лежит внутри уже принятой рамки, нового job не открывает и не расширяет её.
|
||||
2. Прочитаны `AGENTS.md` и разделы PROCESS.md §2.4, §2.9/2.10 (не применимо — r1),
|
||||
§4, §7.1, §7.2.
|
||||
3. Прочитано тело issue #578 целиком (`## Проблема` → `## ТЗ`, 14 разделов).
|
||||
Комментариев нет.
|
||||
4. Прочитан `docs/TOUCH-SUPPORT.md` (контракт View/kiosk, разделы «Product
|
||||
contract», «What "fully supported View" means», «Safety floor»).
|
||||
5. Прочитан `docs/USER-GUIDE.ru.md` (табл. §на строках 1009/1132/1133/1147) —
|
||||
термины «долгое нажатие, 600 мс», «внутренняя карточка House Plan», «HA
|
||||
more-info», «правый клик» использованы в ТЗ так же, как в интерфейсной
|
||||
документации; не изобретены.
|
||||
6. **Технический разбор аналитики сверен с кодом на названном SHA**, а не принят
|
||||
на веру (обе претензии автора — не догадка):
|
||||
- `src/houseplan-card.ts:7126-7146` (`_pointerDown`, ветка `view`):
|
||||
`_holdTimer` действительно взводится безусловно, без проверки
|
||||
`_touchSequenceMultitouch`/`_touchClickGuard.sequenceMultitouch` —
|
||||
соответствует претензии «второй палец на маркере заново вооружает
|
||||
long-press».
|
||||
- Capture-guard `_touchGestureGuard` (`src/houseplan-card.ts:2090-2093`)
|
||||
подписан только на `pointerdown/pointermove/pointerup/pointercancel/
|
||||
lostpointercapture/click` (`:11451-11456`); `@contextmenu` навешен прямо на
|
||||
маркер (`:12555`) и вызывает `_ctxDevice()` (`:5751-5759`) в обход
|
||||
capture-фазы — соответствует претензии «touch contextmenu обходит защиту
|
||||
pinch».
|
||||
- `src/touch-gesture-click-guard.ts` (класс `TouchGestureClickGuard`,
|
||||
проверен целиком) и `test/touch-gesture-click-guard.test.mjs` показывают,
|
||||
что существующий guard #563 закрывает только `click`, не имеет понятия о
|
||||
long-press/contextmenu, и уже сейчас корректно снимает блокировку на любой
|
||||
новый `pointerDown` иного `pointerType` (тест «a new mouse sequence on a
|
||||
hybrid device is not held by an old pinch», строки 50-63) — то есть
|
||||
примитив, который ТЗ предлагает обобщить (§14), уже имеет нужное свойство
|
||||
для гибридного случая, и его расширение с высокой вероятностью наследует
|
||||
это свойство бесплатно.
|
||||
- `_keyDevice` (`:5903-5908`) подтверждает существование клавиатурной
|
||||
активации Enter/Space, на которую ссылается контракт п.9 раздела 5 — не
|
||||
придуманная фича.
|
||||
7. Сверены названные в плане автотестов файлы/инструменты:
|
||||
`demo/smoke_editor_gestures.mjs`, `demo/smoke_long_press_gesture.mjs`,
|
||||
`scripts/mutation-gate.mjs`, `docs/TESTING.md` (раздел про
|
||||
`scripts/mutation-registry.mjs`) — все существуют, инфраструктура для
|
||||
AC8/AC9 реальна, а не гипотетична.
|
||||
8. Проверено согласование трека: у `small` (PROCESS.md §5) требуется ровно одна
|
||||
поверхность, отсутствие нового UX-контракта и touch-эффекта — задача правит
|
||||
`pointerdown`/long-press/`contextmenu`/click одновременно и меняет
|
||||
`docs/TOUCH-SUPPORT.md`, то есть полный трек выбран верно, автор обосновал
|
||||
это явно, а не промолчал.
|
||||
|
||||
Код не менялся, гейты (`typecheck`/`test`/`build`) на этой стадии не
|
||||
прогонялись — они относятся к этапу code-review, спецификация не трогает `src/**`.
|
||||
|
||||
## Находки
|
||||
|
||||
### Low-1 — i18n закрыт только косвенно
|
||||
|
||||
`§7.1` требует отдельно отраслью i18n. В ТЗ единственное упоминание — пункт
|
||||
«вне scope»: «изменение конфигурации, backend/WebSocket протокола, HA
|
||||
entity/device model **либо i18n**» (issue, раздел «4. Вне scope»). Явного
|
||||
«новых ключей нет» как отдельного вывода не сформулировано.
|
||||
|
||||
По существу пункт закрыт: раздел «7. UX и доступность» прямо запрещает любые
|
||||
новые сообщения/тосты/overlay при pinch, а вся правка — про подавление событий,
|
||||
не про текст интерфейса. Второй независимый источник (перечень «Затронутые
|
||||
модули», раздел 10) не содержит ни одного i18n-файла. Открытого вопроса это не
|
||||
создаёт.
|
||||
|
||||
**Решение ревьюера:** Low, снимается без правки — вывод «новых i18n-ключей нет»
|
||||
следует из §7 и §10 однозначно, дублировать его отдельной строкой не обязательно
|
||||
для однозначности исполнения.
|
||||
|
||||
### Low-2 — сценарий «новый mouse pointerdown сразу после pinch» не назван явно ни в одном AC
|
||||
|
||||
Контракт п.8 раздела 5 (второе предложение): «Mouse pointerdown новой
|
||||
последовательности на гибридном устройстве не наследует блокировку
|
||||
закончившегося pinch». Ни один AC (AC1–AC9) не называет этот сценарий по имени;
|
||||
ближе всего AC6 («сохраняет mouse click, правый клик и Enter/Space без ложного
|
||||
подавления») и общая фраза плана автотестов п.4 («сохранить контрольные
|
||||
позитивные сценарии... mouse right-click»), но явного «мышь сразу после
|
||||
завершённого pinch» там нет.
|
||||
|
||||
Смягчающее обстоятельство, подтверждённое чтением кода (см. выше, п.6): для уже
|
||||
существующего click-guard это свойство уже реализовано и покрыто тестом
|
||||
`test/touch-gesture-click-guard.test.mjs:50-63`, а обобщение того же примитива
|
||||
на long-press/contextmenu (что и предлагает §14 ТЗ) с высокой вероятностью
|
||||
наследует то же поведение той же веткой кода (`if
|
||||
(this._activeTouchPointers.size === 0) this._postGestureClickBlocked = false;`
|
||||
не зависит от `pointerType`). Риск регрессии этого конкретного угла невысок, но
|
||||
он не нулевой, если реализация заведёт для long-press/contextmenu отдельный,
|
||||
не переиспользованный примитив.
|
||||
|
||||
**Решение ревьюера:** Low, не блокирует. Рекомендация автору (не обязательна к
|
||||
исполнению до кода): при реализации явно включить проверку «мышиный
|
||||
`pointerdown` сразу после конца pinch не наследует блокировку» в
|
||||
unit/browser-доказательство AC6 или AC7, раз уж контракт её формулирует.
|
||||
Формально снимаю с записью — General wording AC6 достаточно для однозначности
|
||||
приёмки, кодревьюер вправе потребовать явный тест-кейс при реализации.
|
||||
|
||||
Ни High, ни Medium-находок нет.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Обязательные разделы §7.1 присутствуют все: сценарий и «что человек увидит»
|
||||
(раздел 1), проблема (2), скоуп/не-скоуп (3–4), контракт поведения (5),
|
||||
данные/совместимость/миграция (6), UX (7), AC1–AC9 с указанным способом
|
||||
доказательства (8), план автотестов (9), риски и откат (12), release-артефакты
|
||||
(13). Плюс необязательные, но полезные «затронутые модули» (10) и
|
||||
«производительность/touch-impact» (11).
|
||||
- Каждый AC имеет наблюдаемый oracle (счётчики `_infoCard`/`hass-more-info`/
|
||||
confirmation/service, факт изменения zoom, число срабатываний нового
|
||||
tap/long-press) и назван способ доказательства (`browser smoke`, `unit`,
|
||||
`mutation witness`, стандартные гейты) — ни один не сводится к «проверить, что
|
||||
код скомпилировался».
|
||||
- AC8 заранее требует «отрицательных свидетелей» (mutation witness, красный при
|
||||
снятии каждой из двух защит по отдельности) — снимает типовой риск теста,
|
||||
который не умеет падать, до того, как код написан.
|
||||
- Технический анализ причины дефекта (два независимых пробела: bubble-phase
|
||||
`_pointerDown` не проверяет multitouch; `contextmenu` не входит в
|
||||
capture-guard) подтверждён построчным чтением кода на точном SHA, а не
|
||||
принят на веру — обе претензии верны.
|
||||
- Раздел 14 («принятые предположения») корректно выделяет ровно то, что
|
||||
пользователь не наблюдает (внутреннее представление barrier, способ отличить
|
||||
synthetic touch-`contextmenu` от настоящего), помечает это как «свободно
|
||||
меняется» и не выдаёт технические догадки за продуктовые факты нигде за
|
||||
пределами этого раздела — я не нашёл утверждений о поведении, которого нет ни
|
||||
в `TOUCH-SUPPORT.md`, ни в `USER-GUIDE.ru.md`, ни в коде, без такой пометки.
|
||||
- Терминология («долгое нажатие, 600 мс», «внутренняя карточка House Plan»,
|
||||
«HA more-info», «правый клик») дословно совпадает с `docs/USER-GUIDE.ru.md`,
|
||||
не изобретена заново.
|
||||
- Вне-scope (раздел 4) корректно исключает изменение таймаутов, самих жестов
|
||||
pan/pinch/double-tap, действий устройств, конфигурации, i18n и общей event
|
||||
architecture — граница задачи не расползается за пределы двух названных
|
||||
пробелов.
|
||||
- Выбор полного трека вместо `small` обоснован явно и по существу (несколько
|
||||
путей активации, изменение touch-контракта), а не декларативно.
|
||||
- «Вопросы владельцу: нет» — оправдано: ожидаемое поведение уже зафиксировано в
|
||||
действующем `docs/TOUCH-SUPPORT.md` («once a second touch joins... whole
|
||||
sequence is navigation only»), новых продуктовых решений задача не требует;
|
||||
открытых продуктовых вопросов (что видит/делает человек, объём изменений) я
|
||||
тоже не нашёл — оба найденных Low технические, не продуктовые, и решаю их
|
||||
сам, не выношу владельцу.
|
||||
- Откат (revert коммита, без данных/миграции) и риски (§12) покрыты
|
||||
перечисленными AC3–AC7 по существу, а не общей фразой.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Код ещё не написан — не проверялись типы гейтов (`typecheck`/`test`/`build`),
|
||||
golden, smoke-запуски: это стадия code-review, не spec-review.
|
||||
- Не проверял `scripts/mutation-registry.mjs` на предмет точного формата записи
|
||||
двух будущих witness-мутаций — техническая деталь реализации, не предмет
|
||||
ревью ТЗ.
|
||||
- Не запускал `node scripts/smoke-select.mjs` — на этой стадии диффа кода нет,
|
||||
инструмент неприменим.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. High — 0, Medium — 0. Обе Low-находки решены самим ревьюером с
|
||||
записью (без правки ТЗ и без возврата автору), как предусмотрено PROCESS.md
|
||||
§2.4 для Low.
|
||||
|
||||
**Готово к разработке** — но перевод статуса делает конвейер (эта роль его не
|
||||
меняет вручную).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `ad4000f95a15` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `752738456c32b4d4f015527719edba343c434395`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 752738456c32
|
||||
```
|
||||
- Тело issue: `ec0f61cba0a9ebb0afa611f400145755e567714b91b307e18928aed57bbb61be`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user