mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-06 06:38:57 +00:00
@@ -0,0 +1,151 @@
|
||||
# SPEC-REVIEW-360-r1
|
||||
|
||||
Issue: #360 · этап: spec (S4-spec-review) · трек: `small` (лёгкий) · заход: r1 ·
|
||||
блокирующих циклов израсходовано: 0 из 2 (лимит лёгкого трека, §4)
|
||||
|
||||
Ревьюер: Claude (роль «ревьюер ТЗ», отдельная сессия от аналитика/автора).
|
||||
ТЗ живёт в теле issue #360 (revision 1), файл `docs/specs/` не создаётся —
|
||||
верно для `small`.
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Проверялось ТЗ «Редактор подложки: цвет по умолчанию для новых элементов на
|
||||
основной панели» — постоянный `hp-color-opacity` в основной панели Background
|
||||
editor поверх уже существующего `_decorStyle`. Кода нет, ревью — только текст
|
||||
ТЗ, его внутренняя непротиворечивость и соответствие:
|
||||
|
||||
- `docs/SCOPE.md` (J4/J6);
|
||||
- `docs/TOUCH-SUPPORT.md` (Background editor — reference editing environment,
|
||||
правило маркировки новых editor-фич);
|
||||
- `docs/DECOR-EDITOR.md` (канон подсистемы decor);
|
||||
- фактическому коду `src/houseplan-editor-runtime.ts`, `src/hp-color-opacity.ts`,
|
||||
`src/styles/chrome.styles.ts`, `test/color-picker.test.mjs`, `demo/smoke_decor.mjs`,
|
||||
`src/i18n/{en,ru}.json`.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Код-ревью нет, поэтому это не проверка исполнения, а сверка утверждений ТЗ с
|
||||
деревом на SHA `0decce8d` (dev, чистое рабочее дерево):
|
||||
|
||||
| Утверждение ТЗ | Где сверено | Результат |
|
||||
|---|---|---|
|
||||
| `_decorStyle` — единый source, уже используется line/rect/ellipse/text/furniture | `houseplan-editor-runtime.ts:4100,4169-4183,4341-4342,4476-4479,4713` | подтверждено |
|
||||
| Контекстный picker сейчас показывается только для Line/Rect/Ellipse (`draws`) и Backdrop | `houseplan-editor-runtime.ts:5252-5288` (`_renderDecorSecondary`) | подтверждено |
|
||||
| Основная панель — `.editbar.decorbar`, инструменты в `.editbar-tools` (перенос), Undo/Redo там же, Close — в отдельном `.editbar-end` | `houseplan-editor-runtime.ts:5324-5389` | подтверждено |
|
||||
| `.editbar-tools` использует `flex-wrap: wrap`, а не скролл | `styles/chrome.styles.ts:200-208` | подтверждено (ТЗ говорит «прокручиваемой/переносящейся» — верно описывает wrap) |
|
||||
| Существующий смок явно утверждает «picker только в secondary, не в `.decorbar`» — эту строку ТЗ обязано заменить | `demo/smoke_decor.mjs:132-134` | подтверждено, находка ТЗ точная |
|
||||
| `test/color-picker.test.mjs` содержит хрупкий общий счётчик `<hp-color-opacity` (сейчас 13) — ТЗ обещает не плодить, а заменить его | `test/color-picker.test.mjs:57-62` | подтверждено, ссылка на файл верна |
|
||||
| i18n `decor.color` / `space.opacity` уже существуют, новых ключей не требуется | `src/i18n/en.json`, `src/i18n/ru.json` (и de.json) | подтверждено |
|
||||
| `fillColor`/`fillOpacity` независимы от `color`/`opacity` в `decorStylePatch` | `houseplan-editor-runtime.ts:4436,4467-4479` | подтверждено |
|
||||
|
||||
Технической невыполнимости не найдено: связывание нового picker с `_decorStyle`
|
||||
в `_renderDecorBar()` — тот же паттерн, что уже используется в контекстной
|
||||
панели строкой выше по файлу.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи — чинится ТЗ, без High это жёлтый вердикт)
|
||||
|
||||
**M1 — не назван обязательный touch-статус фичи.**
|
||||
`docs/TOUCH-SUPPORT.md` (раздел «Documentation rule», строки 158–167) требует:
|
||||
«New editor feature specifications and code reviews must state one of:
|
||||
`Touch editor: supported` / `Touch editor: best effort / intentionally
|
||||
degraded` / `Touch editor: not exposed`». ТЗ #360 — правка именно editor-фичи
|
||||
(Background editor), но нигде не использует одну из трёх канонических формул.
|
||||
Пункт 8 контракта («На touch View/kiosk нет изменений и регрессий») и
|
||||
не-скоуп («новый touch UX: редактирование остаётся desktop-first») касаются
|
||||
темы, но не дают требуемой метки. Без неё DoR-пункт «влияние на touch по
|
||||
`docs/TOUCH-SUPPORT.md`» формально не закрыт: он требует явную формулу, а не
|
||||
пересказ своими словами.
|
||||
**Правка:** добавить одну строку, например «Touch editor: best effort /
|
||||
intentionally degraded — без изменений относительно текущего Background
|
||||
editor».
|
||||
|
||||
**M2 — не описано, что сохранение свойств уже существующего объекта
|
||||
перезаписывает тот же `_decorStyle`, который теперь постоянно виден.**
|
||||
`_decorSaveShape()` (`houseplan-editor-runtime.ts:4476-4480`) при сохранении
|
||||
диалога свойств line/rect/ellipse/furniture пишет отредactированные
|
||||
color/opacity (и fill-поля для rect/ellipse) обратно в `this.host._decorStyle`
|
||||
— это существующее поведение, не вводимое этой задачей. Сейчас оно почти
|
||||
незаметно: пользователь увидит новое значение, только переключившись на
|
||||
Line/Rect/Oval. После этой задачи тот же `_decorStyle` подключён к **постоянно
|
||||
видимому** main-picker: правка цвета контура ОДНОГО уже размещённого
|
||||
прямоугольника через его properties-диалог мгновенно и заметно поменяет
|
||||
показание общего picker на основной панели — и следующий новый объект унаследует
|
||||
именно этот, только что отредактированный цвет, а не тот, что пользователь
|
||||
выбирал раньше через сам main-picker.
|
||||
|
||||
AC3 («Уже существующие элементы и текущий selection не изменяются») закрывает
|
||||
только направление default → existing, но не обратное — existing-edit →
|
||||
default. AC5 констатирует «main и contextual picker немедленно отражают
|
||||
изменение друг друга», но это утверждение о двух представлениях одного
|
||||
`_decorStyle`, а не о том, что сторонний диалог свойств тоже является третьим
|
||||
писателем в то же состояние. Ни один AC, риск или «принятое предположение» не
|
||||
называет эту связь явно, хотя она становится наблюдаемой ровно из-за этой
|
||||
задачи (первопричина не нова, но её видимость — новая, и это в скоупе ровно
|
||||
той же причины, по которой в §8 отдельно поднят принцип «одно число — один
|
||||
источник»: здесь тот же паттерн для состояния default, показанного теперь в
|
||||
двух разных путях изменения).
|
||||
**Правка:** одна фраза в контракте либо в «Принятых предположениях»,
|
||||
фиксирующая, что это ожидаемое и неизменное поведение (либо явный AC/риск на
|
||||
него), — на выбор автора; технического решения не требуется, только явная
|
||||
запись.
|
||||
|
||||
Обе находки закрываются добавлением одного-двух предложений в тело issue,
|
||||
без изменения архитектуры контракта; отдельный документ/файл не нужен.
|
||||
|
||||
### Low
|
||||
|
||||
Не найдено содержательных Low-находок сверх названных Medium; текст ТЗ
|
||||
избегает голословных утверждений — единственное место, которое могло бы
|
||||
читаться как незадокументированная догадка («штатная подпись «Цвет»/`Color`, а
|
||||
не безымянный swatch»), уже честно вынесено в блок «Принятые предположения»,
|
||||
как и требуется.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Обязательные разделы §7.1 присутствуют: сценарий + «что человек увидит» —
|
||||
единым абзацем (допустимо для `small`), проблема (из тела issue),
|
||||
скоуп/не-скоуп, контракт поведения и UX (пп. 1–8), модель данных/i18n/
|
||||
совместимость/производительность, затронутые файлы, AC1–AC8 с указанием
|
||||
способа доказательства, план тестов и мутанты, риски и откат,
|
||||
release-артефакты, блок принятых предположений.
|
||||
- Product-question gate соблюдён: аналитик закрыл вопросы сам, владельцу не
|
||||
передано ни одного технического вопроса под видом продуктового; блокирующих
|
||||
продуктовых вопросов у автора нет, что подтверждается и итоговым
|
||||
комментарием, и содержанием самого ТЗ.
|
||||
- Все технические утверждения о текущем коде, на которые опирается контракт
|
||||
(единый `_decorStyle`, где показывается сегодняшний контекстный picker,
|
||||
структура `.decorbar`/`.editbar-tools`/`.editbar-end`, независимость
|
||||
fill-полей, отсутствие новых i18n-ключей, конкретные хрупкие тесты,
|
||||
которые придётся тронуть), проверены по факту в дереве и подтверждены —
|
||||
ни одной догадки, выданной за факт, не найдено.
|
||||
- AC пронумерованы, каждый называет способ доказательства (`smoke`/`unit`/
|
||||
`golden`/review), что удовлетворяет требованию DoR.
|
||||
- Не-скоуп чётко исключает per-object recolor, `fillColor`/`fillOpacity`,
|
||||
миграцию/схему, перенос существующих picker, новый touch UX — блокирующей
|
||||
двусмысленности по границам задачи нет.
|
||||
- Откат описан и достаточен: убрать main-bar consumer, состояние и
|
||||
persisted-модель не меняются, обратная миграция не нужна.
|
||||
- Трек `small` подтверждён верно: одна поверхность, риск/сложность ≤3, нет
|
||||
миграции, нет нового UX-контракта (session-default уже существует и
|
||||
документирован), лимит ревью ТЗ — 2 цикла, что и отражено шапкой этого
|
||||
документа.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Реальную браузерную/визуальную проверку переполнения `.editbar-tools` при
|
||||
добавлении picker (переносится ли строка на обычной/узкой desktop-ширине) —
|
||||
на этапе ТЗ кода нет, только сверка планового покрытия (AC6 требует smoke на
|
||||
обеих ширинах и golden-ревью перед бетой); сочтено достаточным для этапа
|
||||
spec, не code-review.
|
||||
- `demo/golden` baseline и фактический рендер light/dark — не применимо к ТЗ.
|
||||
- Соответствие итогового кода контракту — предмет отдельного код-ревью после
|
||||
реализации.
|
||||
|
||||
## Вердикт
|
||||
|
||||
High-находок нет. Обе находки — Medium, в скоупе текущей задачи, устраняются
|
||||
правкой тела issue без изменения архитектуры контракта или AC. Per PROCESS.md
|
||||
§2.4/§4, это жёлтый вердикт: автор дополняет ТЗ двумя предложениями (M1, M2) и
|
||||
проходит второй (последний на лёгком треке) цикл ревью ТЗ.
|
||||
Reference in New Issue
Block a user