mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 11:49:16 +00:00
claude[bot]
committed by
Sergey Matyunin
Sergey Matyunin
parent
133fdc9c10
commit
02a523045b
@@ -0,0 +1,188 @@
|
||||
# SPEC-REVIEW-243-r1
|
||||
|
||||
- Issue: [#243](https://github.com/Matysh/houseplan-card/issues/243) — «Перетаскивание вкладок пространств не работает: захват указателя съедает
|
||||
цель. Плюс нужен указатель места вставки»
|
||||
- Этап: spec (PROCESS.md §2.4)
|
||||
- Заход: r1 · блокирующих циклов израсходовано 0 из 4
|
||||
- ТЗ: `docs/specs/243-space-tab-drop-target.md`, коммит `7ac1934ac5002f915c2e1f885c9142daaee645a8`
|
||||
(`docs(spec): define reliable space tab drop target`, `Issue: #243`, `User-Visible: no`)
|
||||
- Трек: обычный (не `small`) — подтверждено: задача вводит видимый UX-контракт
|
||||
(вертикальный разделитель before/after), не подпадает под критерии §5.
|
||||
- Ревьюер этого раунда контекста автора не имеет; ТЗ и issue разобраны как
|
||||
состязательный материал.
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Первый заход, предыдущего вердикта нет — раздел «Унаследовано из r0» не
|
||||
применяется, разбор ниже полный по PROCESS.md §2.4 и §7.1.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитан `docs/SCOPE.md` — задача чинит регрессию выпущенной функции J6
|
||||
(«keep the plan true as the home evolves», порядок пространств/вкладок),
|
||||
персона — home admin, поверхность — desktop editors (plan/devices/decor).
|
||||
Новый UX-контракт (разделитель) не расширяет мандат: это правка уже
|
||||
принятой функции #220, не новая возможность.
|
||||
2. Прочитаны `AGENTS.md`, `PROCESS.md` целиком (классы изменений, §7.1
|
||||
обязательные разделы, §5 критерии `small`, §4 лимит циклов, шаблон
|
||||
вердикта §7.2).
|
||||
3. Прочитано тело issue #243 и все три комментария (аналитика владельца,
|
||||
занятие автором, сдача ТЗ). Открытых продуктовых вопросов автор не
|
||||
поднимал — сверено, что действительно нечего было спрашивать (см. ниже).
|
||||
4. Прочитан `docs/USER-GUIDE.ru.md` (§ вкладки/перетаскивание, строки
|
||||
276–282) — терминология «вкладка», «перетаскивание», используемая ТЗ,
|
||||
совпадает с зафиксированной там, новых терминов не изобретено.
|
||||
5. Прочитан канонический документ затронутой функции —
|
||||
`docs/specs/220-space-tab-reorder.md` (диагноз, контракт `reorderSpaceIds`,
|
||||
продуктовые решения владельца, границы touch/View) — и
|
||||
`docs/TOUCH-SUPPORT.md` (§ «Safety floor», формулировка `Touch editor: not
|
||||
exposed` — ТЗ цитирует её дословно, не изобретает).
|
||||
6. Диагноз ТЗ сверен построчно с актуальным `dev` (не с описанием в issue):
|
||||
- `src/houseplan-card.ts:1302-1329` (`_tabPointerDown`) — `capturePointer`
|
||||
вызывается, `_tabDrag.overId` инициализируется значением `id` источника;
|
||||
- `src/houseplan-card.ts:1331-1340` (`_tabPointerMove`) — обработчик
|
||||
навешен на каждую вкладку отдельно (`houseplan-card.ts:16426`,
|
||||
`@pointermove=${(e) => this._tabPointerMove(e, s.id)}`) и получает
|
||||
фиксированный `overId = s.id` того элемента, на который повешен;
|
||||
- после `setPointerCapture` браузер ретаргетирует все `pointermove` на
|
||||
захвативший элемент — значит, только обработчик, навешенный на
|
||||
исходную вкладку, продолжает получать события, и передаёт в него
|
||||
собственный `overId` (= `drag.id`). Логический вывод ТЗ «`overId`
|
||||
остаётся равен `drag.id`» подтверждён чтением, не только заявлением
|
||||
автора issue;
|
||||
- `_tabPointerUp` (`:1342-1348`) → `_commitTabOrder(drag.id, drag.overId)`
|
||||
→ при `drag.id === drag.overId` `reorderSpaceIds` (`src/space-order.ts:55-65`)
|
||||
закономерно возвращает тот же массив — запись не идёт. Диагноз ТЗ §3
|
||||
воспроизводится по коду, а не только по логу автора issue.
|
||||
7. Проверена математика `reorderSpaceIds` на нескольких комбинациях
|
||||
(перемещение к более ранней/более поздней вкладке, соседние позиции,
|
||||
перемещение в начало/конец массива) — во всех прогнанных вручную случаях
|
||||
результат совпадает с заявленным в ТЗ §4.3 правилом «`to < from` → before,
|
||||
`to > from` → after»: сторона индикатора однозначно соответствует
|
||||
итоговой позиции после `splice`. Расхождений не найдено.
|
||||
8. Проверены факты, на которые ТЗ опирается как на данность: лимит
|
||||
`MAX_SPACES = 50` (`custom_components/houseplan/validation.py:478`,
|
||||
задокументирован в `docs/USER-GUIDE.ru.md:1520`) — используется в ТЗ §11
|
||||
как обоснование «отдельный performance-профиль не нужен»; существующие
|
||||
CSS-классы `.tab[data-reorderable]`, `.tab.dragging`, `.tab.droptarget`
|
||||
(`src/styles.ts:1725-1728`) — ТЗ корректно описывает их как
|
||||
«текущее, недостаточное» состояние (единственный `.droptarget` без
|
||||
стороны).
|
||||
9. Проверены команды гейтов из ТЗ §15 на существование: `node
|
||||
scripts/mutation-gate.mjs --check` и `--id=<mutant>` реализованы
|
||||
(`scripts/mutation-gate.mjs:1608,1620`); `node scripts/smoke-select.mjs
|
||||
--base <base> --head <head>` — существующий скрипт. Три новых
|
||||
mutation-id (`tab-drag-target-follows-captured-source`,
|
||||
`tab-drop-indicator-always-before`, `tab-drop-outside-commits-last-target`)
|
||||
не пересекаются с уже зарегистрированными (`tab-reorder-not-persisted`,
|
||||
`tab-drag-survives-release-outside`, `tab-drag-outlives-the-card`,
|
||||
`tab-reorder-eats-click`, `tab-reorder-ignores-pointer-type`).
|
||||
10. Проверено наличие прецедента в golden-харнесе для утверждаемого способа
|
||||
доказательства AC2/AC8: `demo/golden/harness.mjs` уже использует
|
||||
`page.mouse`, то есть захват мыши в момент капчура — не новый для
|
||||
инфраструктуры приём, а не голая гипотеза автора.
|
||||
11. Проверены обязательные разделы §7.1: сценарий и персона (§1) · что
|
||||
человек увидит до/после (§2) · диагноз/проблема (§3) · продуктовые
|
||||
решения (§4) · скоуп и не-скоуп (§6) · контракт поведения и
|
||||
hit-testing (§7–8) · UX/визуальный контракт (§9) · данные/i18n/a11y/touch
|
||||
(§10) · перф/безопасность (§11) · AC1–AC9 с доказательством (§12) · план
|
||||
автотестов (§13) · mutation guards (§14) · гейты (§15) · release-
|
||||
артефакты (§16) · откат (§17) · риски (§18) · явный блок принятых
|
||||
технических предположений (§19). Все присутствуют, ничего не
|
||||
зачёркнуто «TBD».
|
||||
12. Проверен трейлерный коммит: `Issue: #243`, `User-Visible: no` —
|
||||
корректно для чисто документационного изменения; diff коммита
|
||||
ограничен `docs/specs/243-space-tab-drop-target.md` и
|
||||
`docs/specs/README.md` (класс C), продуктовый код не тронут — Rule #1
|
||||
соблюдено.
|
||||
|
||||
## Находки
|
||||
|
||||
Находок нет — ни High, ни Medium, ни Low.
|
||||
|
||||
Отдельно проверенные потенциальные риски ложной уверенности, снятые
|
||||
разбором:
|
||||
|
||||
- **Не является ли диагноз просто пересказом автора issue?** Нет: диагноз
|
||||
сверен построчно с `dev` (см. п.6 выше) независимо от текста issue —
|
||||
вывод «`overId` не меняется после capture» логически следует из
|
||||
сочетания «per-tab listener + retargeting событий при `setPointerCapture`»,
|
||||
а не принят на веру.
|
||||
- **Не выдана ли догадка за решение?** Технические решения (расположение
|
||||
обработчика на `.tabs`, форма transient-state, способ рисования линии,
|
||||
вынос hit-test helper в `space-order.ts` или нет) явно помечены в §19
|
||||
«принято предположительно, поменять свободно» — ревьюер вправе оспорить,
|
||||
но это не блокер: ни одно из них не является пользовательским поведением.
|
||||
Нормативные же требования (реальный `page.mouse`, сохранение capture,
|
||||
точная сторона before/after, отмена drop вне вкладки, отсутствие drag во
|
||||
View/touch/fixedFloor) явно исключены из категории предположений в конце
|
||||
§19 и присутствуют как факт в основном тексте — обоснованно, они прямо
|
||||
унаследованы от продуктовых решений #220 и явного ТЗ #243.
|
||||
- **Нет ли скрытого расширения скоупа?** «Не входит» (§6) явно исключает
|
||||
touch/pen drag, клавиатурную сортировку, смену модели порядка, схему
|
||||
`config.spaces`, `expected_rev`, материализацию маркеров, `floor`-toast,
|
||||
свайп/карусель и `fixedFloor`-карточку — совпадает с P0-границами #220,
|
||||
которые #243 обязан не трогать.
|
||||
- **Проверяем ли AC однозначно и с доказательством?** Да, AC1–AC9 (§12)
|
||||
пронумерованы, у каждого указан способ доказательства (`browser smoke`,
|
||||
`unit`, `golden`, `gates`), совпадающий с допустимыми видами из DoR §2.5.
|
||||
- **Продуктовые вопросы владельцу нужны?** Нет открытых: сторона
|
||||
разделителя (before/after), обязательность реального `page.mouse` и
|
||||
сохранение capture — всё это уже зафиксировано либо в теле issue
|
||||
(`AC2`, «мой вариант по умолчанию — первый»), либо в комментарии
|
||||
аналитики владельца («действующие границы #220 сохраняются»). Технические
|
||||
вопросы (где хранить transient-state, чем рисовать линию) ТЗ не выносит
|
||||
владельцу и решает само — корректно по §7.1.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Соответствие `docs/SCOPE.md`: правка регрессии J6, персона/поверхность
|
||||
верны, новый видимый элемент (разделитель) не создаёт новой функции —
|
||||
только делает существующий контракт #220 рабочим и honest.
|
||||
- Диагноз причины бага — подтверждён чтением кода независимо от заявления
|
||||
issue (см. «Как проверялось», п.6).
|
||||
- Математика `reorderSpaceIds` и заявленное правило стороны индикатора —
|
||||
проверены вручную на нескольких комбинациях перестановки, расхождений с
|
||||
ТЗ §4.3/§7 не найдено.
|
||||
- Все 12 обязательных разделов §7.1 присутствуют и по существу.
|
||||
- AC1–AC9 однозначны, у каждого назван способ доказательства из
|
||||
допустимого набора; ни один не сформулирован как нефальсифицируемый.
|
||||
- Явный блок принятых технических предположений (§19) отделён от
|
||||
нормативных требований — соответствует правилу «догадка, выданная за
|
||||
факт, — худший вид дефекта».
|
||||
- Терминология («вкладка», «перетаскивание», `Touch editor: not exposed`)
|
||||
взята из `docs/USER-GUIDE.ru.md` и `docs/TOUCH-SUPPORT.md`, а не
|
||||
изобретена.
|
||||
- Упомянутые в ТЗ команды гейтов (`mutation-gate.mjs --check/--id=`,
|
||||
`smoke-select.mjs --base --head`) существуют в репозитории с такими
|
||||
флагами; новые mutation-id не пересекаются с реестром.
|
||||
- Класс изменения (C — только `docs/**`) и трейлеры коммита корректны;
|
||||
продуктовый код в этом коммите не тронут.
|
||||
- Лимит циклов ТЗ на обычном треке — 4 (не 2, трек не `small`) — учтён при
|
||||
формировании вердикта.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал никакие браузерные смоки, unit, golden или mutation-gate —
|
||||
на этапе ревью ТЗ кода ещё нет (ТЗ описывает будущую реализацию), гейты
|
||||
§15 относятся к стадии «В разработке» и будут материалом код-ревью.
|
||||
- Не проверял golden-харнесс на техническую способность реально удержать
|
||||
`page.mouse.down` до `screenshot` и корректно освободить кнопку между
|
||||
сценами (риск #18 п.5 ТЗ) — это техническая реализуемость, а не
|
||||
продуктовая ambiguity, и относится к зоне ответственности автора и
|
||||
будущего код-ревью.
|
||||
- Не проверял, действительно ли `ShadowRoot.elementFromPoint()` даёт тот же
|
||||
результат, что ручной hit-test по `getBoundingClientRect()` во всех
|
||||
браузерах — ТЗ прямо оставляет выбор реализации автору (§19 п.4),
|
||||
вопрос не продуктовый.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Спецификация полна, диагноз проверен независимо от слов автора issue,
|
||||
контракт стороны индикатора математически согласован с существующей
|
||||
семантикой `reorderSpaceIds`, скоуп и не-скоуп чётко отделяют регрессионный
|
||||
фикс от нового поведения, все обязательные разделы §7.1 на месте, AC
|
||||
проверяемы и снабжены допустимым способом доказательства, открытых
|
||||
продуктовых вопросов нет.
|
||||
|
||||
**Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 → в задаче**
|
||||
Reference in New Issue
Block a user