mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,290 @@
|
||||
# SPEC-REVIEW-505-r1
|
||||
|
||||
Issue: https://github.com/Matysh/houseplan-card/issues/505
|
||||
Стадия: ТЗ на ревью (`S4-spec-review`), полный трек (лёгкий отклонён аналитиком —
|
||||
сложность/риск >3, три поверхности, поддерживаемый touch-контракт).
|
||||
Заход: r1 (первый прогон ревью ТЗ для этой задачи).
|
||||
Материал: `docs/specs/505-summary-panel-design-parity.md` на SHA
|
||||
`8defee4ce1cb0f2794495c03173348c7a6c34e13` (ветка
|
||||
`issue/505-summary-panel-polish`, коммит «docs: specify summary panel designer
|
||||
parity»). Рабочая копия уже стоит на этом SHA.
|
||||
|
||||
## 1. Скоуп ревью
|
||||
|
||||
Задача расширена владельцем 2026-09-09: к четырём исходным UX-исправлениям
|
||||
(плавное скрытие панели, широкий диалог настроек, удаление `Sizes on this
|
||||
screen`, зависимость mobile-опции от master-переключателя) добавлено полное
|
||||
визуальное/поведенческое соответствие трёх поверхностей (шапка, панель, диалог
|
||||
настроек) прототипу Dashboard 5 из архива `макет.zip`
|
||||
(`sha256:bc754c16c9…357e15`).
|
||||
|
||||
Ревью охватывает:
|
||||
- само ТЗ `docs/specs/505-summary-panel-design-parity.md`;
|
||||
- вспомогательные документы `docs/design/505-summary-panel/**` (референс,
|
||||
handoff, README);
|
||||
- обновление индекса `docs/specs/README.md`;
|
||||
- соответствие ТЗ телу issue #505 и обоим комментариям аналитики (r1 «первая
|
||||
версия задачи», r2 «возобновление после расширения scope»);
|
||||
- соответствие `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §7.1/§7.2/§5,
|
||||
`docs/USER-GUIDE.ru.md`, `docs/TOUCH-SUPPORT.md`, `docs/CONFIG-COMPATIBILITY.md`.
|
||||
|
||||
Это первый заход — раздела «Унаследовано из r0» не требуется, разбор полный.
|
||||
|
||||
## 2. Как проверялось
|
||||
|
||||
- Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (включая §2.4,
|
||||
§2.10, §4, §5, §7.1, §7.2, §9) перед оценкой.
|
||||
- Прочитано тело issue #505 и все три комментария (аналитика r1, пауза
|
||||
владельца, возобновление аналитики r2 после расширения scope).
|
||||
- Прочитан ТЗ-документ целиком (`git show
|
||||
origin/issue/505-summary-panel-polish:docs/specs/505-summary-panel-design-parity.md`).
|
||||
- Проверены фактические утверждения ТЗ о текущем состоянии `dev` чтением кода:
|
||||
- `renderPanel()` в `src/summary-panel-runtime-loaded.ts:182-192` действительно
|
||||
возвращает `nothing` сразу при `!visible` — exit-анимация невозможна,
|
||||
подтверждает описанную причину дефекта №1;
|
||||
- `src/summary-panel-style.ts` содержит только `@keyframes hp-summary-in-right`
|
||||
и `hp-summary-in-bottom` (строки 144–152) — входных keyframes для скрытия нет;
|
||||
- `min-width` в `src/summary-panel-style.ts:156` навешен на `.summary-editor`
|
||||
(внутренний контент), а не на оболочку `hp-dialog` — подтверждает причину
|
||||
переполнения диалога;
|
||||
- `src/summary-panel-editor.ts` не связывает `disabled` mobile-чекбокса
|
||||
(строка ~102) с `dialog.localShow` — подтверждает независимость от
|
||||
master-переключателя;
|
||||
- `summary-sizes-title`/`summary.sizes_title` (строка 58) по-прежнему
|
||||
рендерится — раздел `Sizes on this screen` не удалён.
|
||||
Все пять фактических предпосылок ТЗ верны на названном SHA.
|
||||
- Проверен механизм wide-диалога: `--hp-dialog-wide-width` уже используется
|
||||
(`src/styles/dialogs.styles.ts:1592`, `device-inbox-dialog`), `hp-dialog.ts:163`
|
||||
читает эту переменную для `.surface` при `wide` — предпочтение ТЗ «переиспользовать
|
||||
системный wide-dialog contract» технически реализуемо без новой инфраструктуры.
|
||||
- Проверено соответствие терминологии `docs/USER-GUIDE.ru.md` (раздел «Сводная
|
||||
панель», строки 261–314) — используемые в ТЗ названия настроек и поведение
|
||||
совпадают с зафиксированным пользовательским контрактом (общий/локальный
|
||||
scope настроек, ресайз-скрытие, mobile-политика). Обновление самого гайда
|
||||
корректно отнесено к §9 «Release artifacts» реализации, а не к этапу ТЗ.
|
||||
- Проверено `docs/TOUCH-SUPPORT.md:48-56` — сводная панель и её простая форма
|
||||
настроек явно объявлены полноценной View-поверхностью с 44×44 px целями;
|
||||
требования ТЗ по touch-таргетам этому не противоречат.
|
||||
- Проверено `docs/CONFIG-COMPATIBILITY.md:100-132` («Summary panel namespace
|
||||
(#437)») — утверждение ТЗ «Config namespace stays version1; compatibility
|
||||
registry/storage shapes do not change» и утверждение, что локальные
|
||||
size-preferences — браузерные runtime-данные вне серверного конфига,
|
||||
соответствуют канону; удаление UI-раздела `Sizes on this screen` не требует
|
||||
правки этого документа.
|
||||
- Проверена целостность референс-архива: `docs/design/505-summary-panel/reference/`
|
||||
содержит воспроизводимый `index.html`, CSS/JS/SVG и три handoff-документа;
|
||||
README задачи и README референса указывают SHA-256 архива и совпадают между
|
||||
собой и с телом issue.
|
||||
- Сверено обновление `docs/specs/README.md` — строка для #505 добавлена.
|
||||
- Прогнан `node scripts/process-gate.mjs --range origin/dev..origin/issue/505-summary-panel-polish`
|
||||
→ «гейт пройден, предупреждений 0» (офлайн-часть; статус issue не проверялся
|
||||
флагом `--issues`, т.к. факт статуса уже виден в метках issue: `S4-spec-review`,
|
||||
без `blocked`).
|
||||
- Проверены трейлеры коммита `8defee4c`: `Issue: #505`, `User-Visible: no` —
|
||||
корректно для документации без изменения продуктового поведения.
|
||||
- Проверен CI: `gh run list --branch issue/505-summary-panel-polish` →
|
||||
прогон `34316129681`, `completed/success`, `headSha == 8defee4ce1cb…` — точный
|
||||
SHA материала. (Уточнение к вводным этого ревью: там указано, что зелёного
|
||||
Validate на этом SHA не найдено — это было неверно, прогон найден и зелёный;
|
||||
дешёвые гейты дополнительно вручную не гонялись, так как класс изменений —
|
||||
только C (документация), продуктовый код не тронут, и зафиксированный CI
|
||||
прогон уже покрывает `docs`/`process-gate`/`provenance`.)
|
||||
- Сверена диффа `git diff origin/dev..origin/issue/505-summary-panel-polish
|
||||
--stat` — только `docs/**`, класс C; продуктовый код (`src/**`,
|
||||
`custom_components/**`) не изменён, что и требуется на этапе ТЗ.
|
||||
|
||||
## 3. Находки
|
||||
|
||||
### Medium — отсутствуют обязательные первые разделы «Сценарий» и «Что человек увидит до и после» (PROCESS.md §7.1)
|
||||
|
||||
`docs/specs/505-summary-panel-design-parity.md`, раздел 1 «Problem, value and
|
||||
scope» объединяет проблему, ценность и скоуп, но нигде явно не называет: какая
|
||||
персона (`docs/SCOPE.md`: home admin / household members / kiosk) на какой
|
||||
поверхности и в какой момент встречает это изменение, и не даёт отдельной фразы
|
||||
«что человек увидит до и после» без терминов реализации. PROCESS.md §7.1 требует
|
||||
эти два раздела **первыми** и явно: «ТЗ, которое не может ответить на эти два
|
||||
вопроса, описывает работу, а не изменение продукта». Это не абстрактное
|
||||
требование формата — непосредственно предшествующий и тематически связанный ТЗ
|
||||
того же автора, `docs/specs/493-summary-panel-hardening.md` (issue #493, та же
|
||||
подсистема, тот же день), содержит именно эти два раздела первыми
|
||||
(`## 1. Сценарий`, `## 2. Что человек увидит до и после`, строки 11 и 24), то
|
||||
есть автору конвенция известна и обычно соблюдается — здесь она пропущена
|
||||
целиком, а не сокращена.
|
||||
|
||||
**Сценарий проявления:** ревьюер (или будущий читатель ТЗ, включая ревьюера
|
||||
кода на следующем этапе) не может быстро проверить, что изменение решает
|
||||
заявленный пользовательский сценарий, а не просто список визуальных
|
||||
требований — именно такой класс дефекта («AC выполнены, но сценарий не решён»)
|
||||
явно разрешён как основание для жёлтого вердикта в PROCESS.md §2.4/§7.2.
|
||||
Материал, впрочем, косвенно восстановим (персоны и поверхности разбросаны по
|
||||
§2–3 и по комментарию аналитики), поэтому находка — дефект формы и
|
||||
трассируемости, а не скрытая двусмысленность поведения; поэтому Medium, а не
|
||||
High.
|
||||
|
||||
**Как чинится:** добавить в начало документа (перед текущим «1. Problem, value
|
||||
and scope», со сдвигом нумерации) два раздела по образцу #493 — «Сценарий»
|
||||
(admin/household member/kiosk-пользователь на View/kiosk/диалоге настроек, в
|
||||
момент открытия карточки, включения показа или редактирования блоков) и «Что
|
||||
человек увидит до и после» (одной фразой, без терминов реализации: сейчас
|
||||
составной контрол и панель визуально не совпадают с согласованным прототипом
|
||||
и не позволяют держать диалог настроек без горизонтальной прокрутки; после —
|
||||
шапка/панель/диалог выглядят и ведут себя как согласованный дизайн, функционал
|
||||
не теряется).
|
||||
|
||||
### Medium — отсутствует обязательный раздел «Принятые технические предположения» (PROCESS.md §7.1/§7)
|
||||
|
||||
ТЗ #505 не содержит финального блока «принято предположительно, поменять
|
||||
свободно», хотя делает по ходу текста ряд односторонних технических решений,
|
||||
которые пользователь не наблюдает и которые PROCESS.md §7 прямо требует
|
||||
выносить в такой явный блок, чтобы ревьюер мог их оспорить: точное имя CSS
|
||||
custom property для реального HA-диалога (`--ha-dialog-width-md`, §6,
|
||||
помечено «verify actual pinned HA frontend contract», то есть уже осознаётся
|
||||
как непроверенное), состав и расположение новых файлов реализации (§7,
|
||||
«Expected files: …»), новые i18n-ключи и их точные имена (§6), решение
|
||||
переиспользовать `--hp-dialog-wide-width` вместо отдельного механизма. Ни
|
||||
один из этих пунктов не проходил как продуктовый вопрос владельцу (и не
|
||||
должен был), но без явного блока они читаются как окончательные решения ТЗ, а
|
||||
не как предположения, которые исполнитель волен скорректировать при сохранении
|
||||
AC. Тот же #493 (строки 462 и далее, «## 20. Принятые технические
|
||||
предположения», 7 пунктов) — прямой прецедент того же автора для той же
|
||||
подсистемы, показывающий, что формат достижим без дополнительной информации:
|
||||
большая часть содержимого уже есть в тексте ТЗ, недостаёт только явного
|
||||
заголовка и итоговой фразы «эти пункты не меняют видимый контракт и могут
|
||||
быть скорректированы ревьюером при сохранении AC».
|
||||
|
||||
**Как чинится:** добавить раздел «Принятые технические предположения» в конец
|
||||
документа (после текущего раздела 9), перечислив как минимум: точное имя
|
||||
CSS-переменной ширины real-HA диалога подлежит проверке при реализации; состав
|
||||
файлов из §7 ориентировочный; конкретные ключи i18n, кроме перечисленных как
|
||||
обязательные, могут быть переименованы при сохранении семантики; способ
|
||||
хранения generation-guard для анимации (упомянут в §4, но не в отдельном
|
||||
пункте — имя/модуль структуры state machine не названы явно как свободные).
|
||||
|
||||
### Low — неполный список визуальных пар в разделе доказательств (§8)
|
||||
|
||||
Абзац «Visual evidence is mandatory» в конце §8 перечисляет
|
||||
«control+open right panel and settings, dark desktop, bottom portrait, kiosk;
|
||||
plus narrow native/real HA settings screenshots». Чек-лист самого issue
|
||||
(«Дополнительные критерии приёмки», пункт 4) требует пары «desktop light/dark,
|
||||
bottom, kiosk, mobile» — пять явно названных контекстов. В тексте ТЗ явно
|
||||
назван «dark desktop», но не назван отдельно «light desktop» (light,
|
||||
предположительно, подразумевается базовым/неотмеченным вариантом «control+open
|
||||
right panel and settings»), и «mobile» из чек-листа issue визуально не
|
||||
отделён от «narrow native/real HA settings screenshots» — неясно, требуется ли
|
||||
отдельная пара для мобильного отображения самой панели (не только диалога
|
||||
настроек) в узкой View. Это не блокирует понимание общего требования (AC10
|
||||
дополнительно требует смок-проверку 320/390/desktop, light/dark, RU/EN/DE/FR),
|
||||
и не меняет ни одного AC по существу, поэтому не блокирует зелёный вердикт —
|
||||
достаточно явно перечислить все пять контекстов из чек-листа issue при правке
|
||||
двух Medium-находок выше, либо оставить как есть с запиской, что реализация
|
||||
интерпретирует this list по образцу issue буквально. Снимается автором на своё
|
||||
усмотрение с пометкой в ТЗ.
|
||||
|
||||
## 4. Что проверено и корректно
|
||||
|
||||
- **Формат этапа:** issue не помечен `small`/`trivial`, полный трек выбран и
|
||||
обоснован дважды (первичная аналитика — сложность/риск >3 и touch-контракт;
|
||||
повторная аналитика после расширения scope — три поверхности плюс touch),
|
||||
файл ТЗ существует по правильному пути и имени
|
||||
(`docs/specs/505-summary-panel-design-parity.md`), ссылки issue↔ТЗ на месте.
|
||||
- **Продуктовая рамка:** задача остаётся внутри одобренного исключения #437 из
|
||||
`docs/SCOPE.md` (read-only summary overlay); ТЗ явно перечисляет, что НЕ
|
||||
переносится из демо (fake data, localStorage, HA-хром, сворачивание блоков,
|
||||
отдельные экранные размеры) — совпадает и с §2 SCOPE.md, и с телом issue.
|
||||
- **Приоритет источников истины** задан явно и в правильном порядке: owner
|
||||
requirements > текущие контракты House Plan (данные/безопасность/a11y) >
|
||||
визуальный референс > детали демо-реализации — снимает риск слепого
|
||||
копирования прототипа поверх контрактов.
|
||||
- **Технический анализ причин** всех четырёх исходных дефектов подтверждён
|
||||
чтением актуального кода (см. §2 выше) — не голословен.
|
||||
- **Явное разведение прототипной и продуктовой логики** там, где они
|
||||
расходятся: `IMPLEMENTATION-NOTES.md` референса описывает выбор позиции по
|
||||
`matchMedia("orientation: portrait")`, тогда как ТЗ §3 прямо фиксирует, что
|
||||
действующий resolver «stage width >= height => right» авторитетен и **не**
|
||||
является viewport-ориентацией — корректное решение конфликта в пользу
|
||||
существующего контракта, а не слепое копирование демо.
|
||||
- **AC1–AC11** пронумерованы, у каждого явно указан способ доказательства
|
||||
(smoke/визуальная сверка/runtime-тест/gate), не перекрываются бессмысленно и
|
||||
покрывают все пункты обоих чек-листов issue (четыре исходных + расширенные).
|
||||
Для дорогих защитных проверок (AC3 exit-DOM-retention, AC5 shell-wide sizing)
|
||||
явно требуется отрицательный свидетель и mutation-gate — соответствует
|
||||
PROCESS.md §2.7 «чем краснеет».
|
||||
- **Анимационная state machine** (§4) описана как явные состояния
|
||||
hidden→entering→visible→exiting→hidden с generation-guard против «призраков»,
|
||||
явно перечисленными граничными случаями (reduced motion, resize/anchor change,
|
||||
скрытый документ, смена identity, editor entry) — не оставляет открытых
|
||||
вопросов по логике жизненного цикла.
|
||||
- **Config-совместимость и данные** не меняются (namespace version 1,
|
||||
local-only size prefs вне серверного конфига) — подтверждено
|
||||
`docs/CONFIG-COMPATIBILITY.md`, миграция не требуется, откат тривиален.
|
||||
- **i18n**: новые ключи перечислены явно (en/ru/de/fr), с условием на переиспользование
|
||||
существующих и удаление `summary.sizes_title` только при отсутствии других
|
||||
потребителей — проверяемо статическим поиском на этапе код-ревью.
|
||||
- **Touch/a11y**: 44×44 px, disabled-семантика, keyboard/focus traversal —
|
||||
согласовано с `docs/TOUCH-SUPPORT.md`, который прямо относит эту поверхность
|
||||
к гарантированному View-контракту, а не к best-effort editor.
|
||||
- **Продуктовых вопросов владельцу не осталось** — оба комментария аналитики
|
||||
подтверждают это, и текст ТЗ действительно не содержит скрытых догадок о
|
||||
видимом поведении, выданных за факт (единственное место, где остаётся
|
||||
неопределённость — имя HA CSS-переменной — прямо помечено как требующее
|
||||
проверки, а не заявлено как факт).
|
||||
- **Процесс/гейты:** коммит несёт корректные трейлеры, `process-gate.mjs`
|
||||
проходит без предупреждений, CI зелёный на точном материале ревью, диапазон
|
||||
диффа ограничен классом C (документация), продуктовый код не тронут — как и
|
||||
должно быть на этапе ТЗ.
|
||||
- **Индекс** `docs/specs/README.md` обновлён.
|
||||
|
||||
## 5. Чего не проверял
|
||||
|
||||
- Пиксельное сравнение реализации с прототипом — на этапе ТЗ реализации не
|
||||
существует, сверка станет обязанностью код-ревью (AC1/AC2/AC6/AC10 явно
|
||||
требуют парные скриншоты как доказательство).
|
||||
- Тяжёлые браузерные гейты (`golden`, `smoke_*`, `performance_smoke`,
|
||||
`pytest tests_backend`) не запускались — не относится к этапу ТЗ, продуктовый
|
||||
код и тесты не изменены этим коммитом.
|
||||
- Флаг `--issues` у `process-gate.mjs` не использовался (нужен токен с широким
|
||||
доступом к Issues API сверх текущего); статус подтверждён напрямую через
|
||||
`gh issue view` — ровно одна метка `S4-spec-review`, без `blocked`.
|
||||
- Не проверялось содержимое самого архива `макет.zip` за пределами
|
||||
зафиксированного SHA-256 и распакованных `docs/design/505-summary-panel/reference/**`
|
||||
файлов — оригинальный архив в репозиторий не входит, воспроизводимость
|
||||
обеспечена самими распакованными файлами.
|
||||
|
||||
## 6. Вердикт
|
||||
|
||||
High: 0. Medium: 2 (обе — дефект формата ТЗ по PROCESS.md §7.1, в скоупе
|
||||
текущей задачи, чинятся правкой того же документа без нового технического
|
||||
исследования). Low: 1 (снимается автором на своё усмотрение).
|
||||
|
||||
Обязательные разделы присутствуют по существу содержания, но не по форме,
|
||||
предписанной PROCESS.md §7.1 и подтверждённой прецедентом #493 того же автора;
|
||||
это возврат на доработку ТЗ, а не отклонение — формат восстановим без нового
|
||||
анализа.
|
||||
|
||||
Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 2 → в задаче
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/505-summary-panel-polish`
|
||||
- SHA материала: `8defee4ce1cb0f2794495c03173348c7a6c34e13`
|
||||
- Дерево ТЗ: `docs/specs/505-summary-panel-design-parity.md` на указанном SHA
|
||||
- Поиск при устаревании ветки:
|
||||
`git log --all --format='%H %T' | grep <дерево HEAD^{tree}>`
|
||||
`git log --all --find-object=<блоб docs/specs/505-summary-panel-design-parity.md> -- docs/specs/505-summary-panel-design-parity.md`
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/505-summary-panel-polish`, коммит `8defee4ce1cb` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `eddbf58ae62fc94849148a7d3881d5543cf16870`
|
||||
```
|
||||
git log --all --format='%H %T' | grep eddbf58ae62f
|
||||
```
|
||||
- ТЗ `docs/specs/505-summary-panel-design-parity.md`, блоб `61a52163e5a30f3b86f24de3ace9fbd6ebb4e0a5`
|
||||
```
|
||||
git log --all --find-object=61a52163e5a30f3b86f24de3ace9fbd6ebb4e0a5 -- docs/specs/505-summary-panel-design-parity.md
|
||||
```
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user