mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,196 @@
|
||||
# SPEC-REVIEW-359-r1
|
||||
|
||||
Issue: [#359 — Предпросмотр мебели на плане перед размещением](https://github.com/Matysh/houseplan-card/issues/359)
|
||||
ТЗ: [docs/specs/359-furniture-placement-preview.md](https://github.com/Matysh/houseplan-card/blob/issue/359-furniture-placement-preview/docs/specs/359-furniture-placement-preview.md)
|
||||
Материал: коммит `392ef22c` "docs: specify furniture placement preview" (единственный
|
||||
файл, `docs/specs/359-furniture-placement-preview.md`, 197 строк; трейлеры
|
||||
`Issue: #359` / `User-Visible: no` корректны — код не менялся).
|
||||
Заход: r1 (первый раунд, раздела «Унаследовано»/«Закрытие раунда» нет).
|
||||
|
||||
## Скоуп
|
||||
|
||||
Полный трек (owner-аналитика: `small` не выполнен — «нарушен критерий "нет
|
||||
нового UX-контракта"»), поэтому спек живёт файлом в `docs/specs/`, а не в теле
|
||||
issue. Диапазон разбора — весь документ ТЗ плюс тело issue #359 и единственный
|
||||
комментарий владельца (оценка/взятие в работу).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Ревью документа без исполнения кода:
|
||||
|
||||
1. `docs/SCOPE.md` — задача закрывает J4/J6 (снижение ошибок первичного
|
||||
размещения мебели, «Keep the plan true» / «zero to plan GUI onboarding»);
|
||||
не задевает лок-инвариант, View/kiosk, ничего из «Out of scope».
|
||||
2. `AGENTS.md`, `PROCESS.md` §2.4/§7.1/§12 — обязательные разделы, формат AC,
|
||||
класс изменения (C — только `docs/**`, коммит и так закрыт DoD спека).
|
||||
3. Тело issue #359 и комментарий-аналитика — сверка скоупа, персоны,
|
||||
поверхности (Background editor, touch best-effort), P2/сложность 4/10.
|
||||
4. `docs/TOUCH-SUPPORT.md`, `docs/CANVAS.md` §9.4, `docs/USER-GUIDE.ru.md`
|
||||
(раздел «Мебель», строки 1303–1311) — сверка терминологии и заявленного
|
||||
поведения магнита/`Shift`/touch с каноном.
|
||||
5. Чтение текущего кода без исполнения: `src/furniture.ts` (`snapFurnitureToWall`,
|
||||
`furnitureCorners`, `furnitureResize`), `src/houseplan-editor-runtime.ts`
|
||||
(`_furnPlace`, `_furnMoveUpdate`, `_decorShapeDown`, ветка `t === 'furniture'`
|
||||
в `_decorPointerDown` ~L4090), `src/pointer-modality.ts` — проверка
|
||||
технической реализуемости single-resolver и hover-condition claims.
|
||||
6. `demo/golden/matrix.mjs`, `demo/golden/README.md` — сверка заявления «golden
|
||||
не требуется» с существующей практикой (`hoverRoom`/`hoverDevice`,
|
||||
`junction-draft-end-node-dark`).
|
||||
7. `demo/smoke_furniture.mjs`, `test/furniture.test.mjs` — существующее покрытие,
|
||||
на которое опирается план автотестов.
|
||||
|
||||
Гейты `typecheck`/`test`/`build` не гонялись: диапазон — один документ в
|
||||
`docs/specs/`, продуктовый код не тронут. `check-docs.mjs`/golden/smoke не
|
||||
применимы к этапу спек-ревью — они станут предметом код-ревью, когда появится
|
||||
реализация.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе) — контрактный пункт 9 (invalid/unknown symbol) не имеет доказательства
|
||||
|
||||
**Файл:** `docs/specs/359-furniture-placement-preview.md`, «Контракт поведения»
|
||||
п.9 (строка 83) и «Критерии приёмки» (114–141).
|
||||
|
||||
**Воспроизведение по тексту:** п.9 контракта формулирует два независимых
|
||||
требования: «изменение Width/Depth… пересчитывает preview» и «невалидное/
|
||||
неизвестное изображение не создаёт preview и не ломает редактор». AC3
|
||||
доказывает только первую половину («живые размеры»). Ни один из AC1–AC8, ни
|
||||
«План автотестов» (145–155) не называет проверку для случая, когда у
|
||||
`_furnPalette.symbol` нет соответствующего `FurnitureGraphic`/`FurnitureSymbol`
|
||||
(например, `furnitureGraphic(id)` возвращает `null` — путь уже существует в
|
||||
`src/furniture.ts:362`). Риски (161–174) упоминают это как «Fail dark», но риск
|
||||
— не доказательство, а декларация намерения; без названного AC/теста это
|
||||
утверждение контракта останется непроверенным после реализации.
|
||||
|
||||
**Почему это Medium, а не High:** не блокирует реализуемость — поведение
|
||||
однозначно описано («не создаёт preview и не ломает редактор»), только не
|
||||
привязано к способу проверки, как того требует §7.1 DoR («каждый AC… с
|
||||
указанием доказательства»).
|
||||
|
||||
**Как чинится в скоупе:** добавить AC9 (или расширить AC3/AC7) с явным
|
||||
доказательством — unit-тест резолвера на неизвестный `symbol` либо smoke-шаг,
|
||||
проверяющий отсутствие `.furniture-placement-preview` и отсутствие исключения.
|
||||
|
||||
### Medium (в скоупе) — «golden не требуется» слабо обосновано на фоне уже существующей практики
|
||||
|
||||
**Файл:** `docs/specs/359-furniture-placement-preview.md`, «Release-артефакты»
|
||||
(182–190), обоснование в конце «Плана автотестов» (157–159).
|
||||
|
||||
**Воспроизведение:** обоснование — «smoke проверяет реальный SVG path,
|
||||
computed style и позиционную геометрию, а принятие нового изображения добавило
|
||||
бы дорогой платформенный шум к transient editor-only состоянию». Но
|
||||
`demo/golden/matrix.mjs` уже содержит именно такие детерминированные transient
|
||||
и hover-состояния без «платформенного шума»: `hoverRoom`/`hoverDevice`
|
||||
(`decor-over-opaque-hover-light:452`, `hover-over-glow-dark:627`,
|
||||
`hover-nested-room-dark:629`) и черновик рисования линии
|
||||
(`junction-draft-end-node-dark:668`) — оба задаются программно через состояние
|
||||
карточки, а не реальным движением указателя, то есть детерминированы точно так
|
||||
же, как предлагает избежать этот спек. Ценность фичи — именно пиксель-точное
|
||||
совпадение preview с будущим объектом (контракт п.2, UX: «ghost находится в
|
||||
decor composition layer поверх сохранённого decor»); AC7 доказывает это только
|
||||
через DOM/computed-style-ассерты (path `d`, `opacity`, `aria-hidden`,
|
||||
`pointer-events`), которые не ловят дефекты z-order/композитинга (например,
|
||||
ghost отрисован позади другого decor-объекта, либо неверный порядок слоёв
|
||||
относительно сохранённой мебели) — то, что видно только на растровом
|
||||
сравнении.
|
||||
|
||||
**Почему это Medium, а не High:** не блокирует реализацию и не касается
|
||||
продуктового решения — способ проверки визуального результата чисто
|
||||
технический и не требует вопроса владельцу.
|
||||
|
||||
**Как чинится в скоупе:** либо добавить один детерминированный golden-сценарий
|
||||
для preview-состояния тем же программным способом, что и
|
||||
`junction-draft-end-node-dark`, либо заменить обоснование в «Release-
|
||||
артефактах» на техническую причину, специфичную для этого случая (а не общее
|
||||
«transient = шум», которое опровергается двумя уже существующими сценариями в
|
||||
том же файле).
|
||||
|
||||
Low-находок нет.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Обязательные разделы §7.1 присутствуют полностью**: сценарий, «что человек
|
||||
увидит до/после», проблема, скоуп/не-скоуп, контракт поведения, UX, модель
|
||||
данных и миграция, i18n, AC1–AC8 с доказательством, план автотестов, риски,
|
||||
откат, release-артефакты — плюс обе продуктовые вставки из AGENTS.md (персона/
|
||||
поверхность/момент в «Сценарии»; факт без терминов реализации в «Что человек
|
||||
увидит»).
|
||||
- **Персона и поверхность корректны и привязаны к J4/J6** из `docs/SCOPE.md`;
|
||||
задача не задевает лок-инвариант, View/kiosk, ничего из «Out of scope».
|
||||
- **Touch-контракт (п.10, «Не-скоуп») дословно совпадает с
|
||||
`docs/TOUCH-SUPPORT.md`**: «best effort», допустимое отсутствие hover-preview
|
||||
на coarse pointer, запрет «сохранения непреднамеренной геометрии из-за pinch/
|
||||
cancel/второго касания» — прямое соответствие разделу «Safety floor that
|
||||
still applies to touch editors».
|
||||
- **Магнит к стене и роль `Shift` (контракт п.3) технически точны и совпадают
|
||||
с каноном и текущим кодом**: `docs/CANVAS.md` §9.4 («bypassing the furniture
|
||||
wall magnet while the ordinary decor/room/grid magnet remains active») и
|
||||
`docs/USER-GUIDE.ru.md:1305-1311` слово в слово подтверждают заявленное
|
||||
поведение; в `src/houseplan-editor-runtime.ts:4653` и `:4689`
|
||||
(`_furnPlace`/`_furnMoveUpdate`) уже сегодня `ev.shiftKey ? null :
|
||||
snapFurnitureToWall(...)` — ровно та развилка, которую описывает спек.
|
||||
- **Единый resolver (контракт п.5) технически реализуем без притягивания
|
||||
сущностей.** `_furnPlace` (L4642-4675) уже сегодня — короткая
|
||||
последовательность чистых вызовов (`_decorSnap` → `snapFurnitureToWall` →
|
||||
clamp → сборка `DecorShape`); выделение общей чистой функции, которую
|
||||
вызовут и превью, и `pointerdown`, не требует новой архитектуры.
|
||||
`snapFurnitureToWall` в `src/furniture.ts:418` уже чистая и уже покрыта
|
||||
сигнатурой, которую предполагает AC2.
|
||||
- **Условие видимости preview (контракт п.1, «последний fine/hover-capable
|
||||
mouse pointer») опирается на существующий механизм**, а не на новый: в
|
||||
проекте уже есть `src/pointer-modality.ts` и задокументированное в
|
||||
`docs/TOUCH-SUPPORT.md` правило «Hover is instance-local and follows the
|
||||
latest real pointer input… enabled only after a mouse event when the browser
|
||||
also reports fine, hover-capable hardware» — спек его переиспользует, а не
|
||||
изобретает.
|
||||
Симметрично п.10 («touch/pen… очищают возможный mouse-preview») — прямая
|
||||
калька с «Touch and pen input immediately clear transient room and device
|
||||
hover» из того же документа.
|
||||
- **Терминология UX-раздела корректна**: «decor composition layer» совпадает с
|
||||
комментарием в `src/houseplan-card.ts:11207` («Decor is one composition
|
||||
layer above every floor treatment»); класс decor-слоя (`.decorlayer`) уже
|
||||
существует (`src/houseplan-card.ts:8140`).
|
||||
- **Значение opacity `0.55` не произвольно** — то же число уже используется как
|
||||
устоявшийся визуальный язык проекта для «неокончательного/фонового» состояния
|
||||
(`src/styles/chrome.styles.ts:151` `.tab.dragging`, `src/styles/plan.styles.ts:572,1112`,
|
||||
анимации в `src/styles/base.styles.ts`), то есть не гадание, а согласованный
|
||||
выбор.
|
||||
- **i18n корректен**: новых текстовых строк нет, новых ключей не требуется —
|
||||
документ прав, что раздел можно закрыть без i18n-плана.
|
||||
- **Модель данных/миграция корректны**: `DecorShape` не меняется, preview —
|
||||
чисто runtime state; откат тривиален и не требует обратной миграции.
|
||||
- **Открытых продуктовых вопросов не найдено.** Отдельно проверены места,
|
||||
которые могли скрывать угаданное продуктовое решение под видом факта:
|
||||
выбор непрозрачности (обоснован существующим стилем, не изобретён),
|
||||
отсутствие дополнительной рамки/анимации (дословно повторяет «Предлагаемое
|
||||
поведение» из тела issue, не добавляет нового решения), поведение на
|
||||
гибридных touch+mouse устройствах (уже решено существующим
|
||||
`pointer-modality.ts`, не новый вопрос). Раздел «Принято предположительно»
|
||||
содержит только технические допущения (имя state, модуль резолвера, способ
|
||||
коалесцирования, состав source-contract тестов) — ровно то, что §7.1 отдаёт
|
||||
автору, а не владельцу.
|
||||
- **Не-скоуп чётко ограничивает риск расползания**: multi-stamp, размерные
|
||||
плашки, collision detection, миграция схемы, move/resize/rotate уже
|
||||
сохранённой мебели — исключены явно и обоснованно, никаких «попутных»
|
||||
претензий.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Реализуемость AC1–AC8 в реальном браузере (Playwright) — код ещё не
|
||||
написан, это предмет код-ревью на S7.
|
||||
- `npm run typecheck` / `npm test` / `npm run build` / `npm run bundle:sync` /
|
||||
`npm run bundle:budget` / `node demo/smoke_furniture.mjs` — не применимы,
|
||||
диапазон изменения не касается `src/**`, `demo/**`, `test/**`.
|
||||
- `node scripts/check-docs.mjs` — не применим, `src/**` не менялся, отпечаток
|
||||
скриншотов не мог устареть от этого коммита.
|
||||
- Точная формулировка будущих i18n/changelog записей (`docs/FURNITURE.md`,
|
||||
`docs/USER-GUIDE.ru.md`, `docs/CHANGELOG*.md`) — они появятся вместе с
|
||||
реализацией и будут предметом код-ревью, а не спек-ревью.
|
||||
- Производительность render-цикла на реальном железе — риск назван и выглядит
|
||||
правдоподобно (один SVG path, без пересчёта модели), но это утверждение,
|
||||
которое подтвердит только код-ревью/смок, не документ.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Жёлтый: 0 High, 2 Medium, оба в скоупе задачи #359 и чинятся правкой ТЗ в этом
|
||||
же issue (без отдельного issue, решение владельца 2026-08-19, #202).
|
||||
Reference in New Issue
Block a user