mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -1,6 +1,6 @@
|
||||
# Индекс ревью
|
||||
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1051, issue: 370. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1052, issue: 371. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
|
||||
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
||||
|---|---|---|---|---:|---:|---|---|
|
||||
@@ -65,6 +65,7 @@
|
||||
| #617 | [SPEC-REVIEW-617-r1.md](SPEC-REVIEW-617-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | «новый необязательный параметр» уже существует | `src/backdrop-pick.ts` `houseplan-editor-runtime.ts` |
|
||||
| #617 | [CODE-REVIEW-617-r1.md](CODE-REVIEW-617-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #617 | [CODE-REVIEW-617-r2.md](CODE-REVIEW-617-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #616 | [SPEC-REVIEW-616-r1.md](SPEC-REVIEW-616-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | AC8 не перечисляет поимённо весь набор golden-фикстур; AC2 не называет .projection-toggle в списке скрытых классов | `demo/golden/matrix.mjs` `houseplan-card.ts` |
|
||||
| #615 | [SPEC-REVIEW-615-r1.md](SPEC-REVIEW-615-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | AC3 называет несуществующую защиту от расползания на плашки цвета | `smoke_room_settings_form.mjs` `smoke_space_settings_form.mjs` `smoke_device_settings_form.mjs` `smoke_dialog_polish_605.mjs` `smoke_general_settings_form.mjs` |
|
||||
| #615 | [SPEC-REVIEW-615-r2.md](SPEC-REVIEW-615-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #615 | [CODE-REVIEW-615-r1.md](CODE-REVIEW-615-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
|
||||
@@ -0,0 +1,205 @@
|
||||
# SPEC-REVIEW-616-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/616
|
||||
- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
|
||||
- **Трек:** полный (аналитика владельца, комментарий `#issuecomment-5805306423`:
|
||||
«лёгкий трек: нет (новый UX-контракт, touch)»)
|
||||
- **Материал:** тело issue #616, раздел `## ТЗ` (снимок на момент ревью,
|
||||
2026-09-25, редакция r1); комментариев — два: вопросы владельцу и принятие
|
||||
умолчаний по всем четырём пунктам (`#issuecomment-5830704284`)
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4
|
||||
- **Роль:** ревьюер ТЗ (не автор)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
ТЗ вводит компактную однострочную шапку View/редакторов на ширине окна
|
||||
≤ 480 px: вкладки пространств + зум + кнопка-шестерёнка (у админа — ещё слот
|
||||
крестика редактора, #647), а все остальные действия (редакторы, общие
|
||||
настройки, PDF, помощь, сводная панель, объёмный вид, добавление пространства)
|
||||
переносятся во всплывающее меню шестерёнки. На > 480 px и в киоске поведение
|
||||
не меняется. Проверялось: обязательные разделы §7.1, однозначность и
|
||||
проверяемость каждого AC, соответствие описания «по коду» реальному коду,
|
||||
согласие со `SCOPE.md`, `TOUCH-SUPPORT.md`, `UX-MODES.md`,
|
||||
`USER-GUIDE.ru.md`, `STYLING-HOOKS.md`, полнота ответа на все четыре вопроса
|
||||
владельцу, отсутствие домыслов, выданных за факт.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. `docs/SCOPE.md` — персона «Household members» на телефоне, для которой
|
||||
«View mode is the product for two of the three personas» — прямое
|
||||
попадание в это ограничение: задача убирает редакторские аффордансы и
|
||||
лишние ряды хрома именно на этой поверхности. Не пересекается ни с одним
|
||||
пунктом «Out of scope». В скоупе.
|
||||
2. `docs/process/REVIEWER.md` целиком и разделы PROCESS.md §2.4, §2.5, §7.1,
|
||||
§4, §7.2.
|
||||
3. Прочитан весь текст issue #616 и оба комментария владельца — все четыре
|
||||
вопроса закрыты умолчаниями («да / нет, скрыть на любой ширине / да,
|
||||
редакторы desktop-first / да, kiosk не меняется»), в ТЗ они отражены без
|
||||
искажения.
|
||||
4. Сверено описание «Проблема (по коду)» с `src/houseplan-card.ts:10758-10852`
|
||||
построчно: заголовок карточки, `nav.tabs` (с `.tabedit`/`.tabadd` только при
|
||||
`_canEdit` — `houseplan-card.ts:982,10778-10800`), `.modes` (три вкладки
|
||||
режимов, только при `_canEdit`), `.editor-close-slot` (#647), `.zoomctl`,
|
||||
`projection-toggle` (только `_labsIso && mode==='view' && !kiosk`), три
|
||||
`header-action` (только при `_canEdit`), составная кнопка сводной панели
|
||||
(`_summary?.renderControls`, только `!kiosk`) — их состав и условия
|
||||
рендера в ТЗ переданы точно, ни один элемент шапки не упущен из таблицы
|
||||
меню в п.4 скоупа.
|
||||
5. Сверено `.head { flex-wrap: wrap }` и медиа-правило `@media (max-width:
|
||||
620px)` в `src/styles/dialogs.styles.ts:6-30` — описание «`flex-wrap: wrap`
|
||||
без правил для телефона, кроме уменьшения отступов на ≤ 620 px» совпадает
|
||||
дословно.
|
||||
6. Прочитаны `docs/TOUCH-SUPPORT.md` (контракт «View/kiosk — touch-first,
|
||||
редакторы — desktop-first», 44×44 таргет), `docs/UX-MODES.md` (текущее
|
||||
описание составной кнопки сводной панели и режимов — в шапке пока не
|
||||
упомянут телефонный кейс, ТЗ корректно планирует добавить его),
|
||||
`docs/USER-GUIDE.ru.md` §5 (абзаца «На телефоне» пока нет — ТЗ верно
|
||||
требует его добавить), `docs/STYLING-HOOKS.md` §7.3/7.4 (существующие
|
||||
хуки `data-hp="settings"/"pdf"/"support"/"mode-tab"/"editor-close"` и
|
||||
принцип «zoom остаётся в DOM внутри CSS-hidden киоск-шапки» — прямой
|
||||
прецедент для подхода ТЗ «прежние кнопки скрыты CSS, не удалены»), и
|
||||
`docs/data-hp-contract.json` (формат `hooks.<name>{elements,since,audience}`
|
||||
— новые `header-menu`/`header-menu-item` в ТЗ ложатся в этот формат).
|
||||
Противоречий не найдено; все перечисленные в разделе «Контракт и UX» файлы
|
||||
для правки названы точно.
|
||||
7. Подтверждено существование golden-фикстур `german-view-mobile-light` и
|
||||
`version-mismatch-touch-light-ru` (`demo/golden/matrix.mjs:226,1060`,
|
||||
`demo/golden/baselines/baselines-index.json:34,179`) и ещё ~13 фикстур на
|
||||
390 px в матрице (диалоги, tray, decor-попапы) — компонент «и остальные
|
||||
мобильные с видимой шапкой» в AC8 не поимённый список, но задача снята
|
||||
§11.4: AC8 — предрелизный гейт (`golden:verify`), несовпадение любой не
|
||||
пересобранной фикстуры проявится диффом на CI независимо от того, назвал
|
||||
ли автор её в ТЗ явно (см. «Находки», Low-1).
|
||||
8. Подтверждено существование всех смоков, названных в «Плане тестов»:
|
||||
`smoke_summary_panel(.mjs/_polish.mjs)`, `smoke_support_feedback.mjs`,
|
||||
`smoke_gear_tabs.mjs`, `smoke_dialog_modal_recovery.mjs`,
|
||||
`smoke_houseplan_panel.mjs`, `smoke_isometric_live_touch.mjs`,
|
||||
`smoke_toolbar_stable_width.mjs`, `scripts/smoke-select.mjs`. Прочитан
|
||||
`demo/smoke_toolbar_stable_width.mjs:9-40` — он действительно гоняет
|
||||
`WIDTHS = [1400, 1000, 768, 390]` и на 390 px измеряет видимость трёх
|
||||
`[data-hp="mode-tab"]` (`visibleTabs`, ожидает `width > 0`); после правки
|
||||
`.modes` на ≤ 480 px будет `display:none`, и этот же смок красным подтвердит
|
||||
правильность утверждения риск-раздела ТЗ («проверка режимов остаётся на
|
||||
768/1000/1400, на 390 проверяется слот ×») — риск описан точно, а не
|
||||
декларативно.
|
||||
9. Проверен потолок строк: `test/core-file-budget.test.mjs:40` —
|
||||
`'src/houseplan-card.ts': 12889`; `wc -l` показывает 12886 — запас 3
|
||||
строки. ТЗ верно определяет это как жёсткий риск и явно выносит логику
|
||||
меню в новый `src/header-menu.ts`, а не расширяет монолит — без этого
|
||||
решения задача не проходила бы гейт `core-file-budget` почти гарантированно.
|
||||
10. Проверены локали `en/ru/de/fr` (`src/i18n/{en,de,fr,ru}.json` реально
|
||||
существуют) — новый ключ `title.header_menu` заявлен для всех
|
||||
поддерживаемых языков, ни один не пропущен и лишних не добавлено.
|
||||
11. Сверен таргет 44×44 с существующим стандартом `docs/TOUCH-SUPPORT.md:14,72`
|
||||
— переиспользует принятую метрику, не изобретает новую.
|
||||
12. Проверены все восемь AC на однозначность и способ доказательства:
|
||||
каждый привязан к конкретному смоку/юниту и называет мутацию, на которой
|
||||
тест краснеет («чем краснеет» не пустая ни в одной строке) — включая
|
||||
защитные AC4 (отказ клика мимо меню) и AC6 (лимит размера тапа).
|
||||
|
||||
## Находки
|
||||
|
||||
### Low-1 — AC8 не перечисляет поимённо весь набор golden-фикстур
|
||||
**Файл:** тело issue #616, раздел «Критерии приёмки», строка AC8.
|
||||
|
||||
AC8 называет по имени только `german-view-mobile-light` и
|
||||
`version-mismatch-touch-light-ru`, остальное — общей фразой «и остальные
|
||||
мобильные с видимой шапкой». В матрице (`demo/golden/matrix.mjs`) не менее
|
||||
~15 фикстур на 390 px, часть из них — с открытым диалогом поверх шапки, где
|
||||
неочевидно, входит ли шапка в кадр диалога. Формально это блок с
|
||||
неоднозначной границей множества.
|
||||
|
||||
Не поднимаю до Medium: AC8 сам явно помечен предрелизным гейтом (`golden:verify`,
|
||||
§11.4), а не гейтом код-ревью, и любой не пересобранный кадр даёт пиксельный
|
||||
диф на CI независимо от того, был ли он поимённо анонсирован в ТЗ —
|
||||
недостающая пересъёмка сама себя обнаруживает, лишняя пересъёмка (кадр без
|
||||
видимых изменений) не ломает `golden:verify`. Снимаю без правки текста: при
|
||||
код-ревью после реализации достаточно свериться со списком реально
|
||||
изменившихся кадров в диффе, а не с этим перечислением.
|
||||
|
||||
### Low-2 — AC2 не называет `.projection-toggle` в списке скрытых классов
|
||||
**Файл:** тело issue #616, раздел «Критерии приёмки», строка AC2.
|
||||
|
||||
AC2 перечисляет классы, которые не должны быть видны у админа на ≤ 480 px
|
||||
(`.modes`, `header-action`, `.summary-control`, `.tabedit`, `.tabadd`), но не
|
||||
упоминает `.projection-toggle` (кнопка «Объёмный/Плоский вид»,
|
||||
`houseplan-card.ts:10820-10828`), хотя по п.4 скоупа она тоже должна уйти в
|
||||
меню («только в просмотре, любая роль… как сейчас»). Формально смок,
|
||||
написанный буквально по перечню AC2, мог бы не проверить этот один класс
|
||||
отдельно.
|
||||
|
||||
Не поднимаю до Medium: тот же AC2 требует «одна строка ≤ 56 px» для ВСЕХ
|
||||
видимых элементов `.head` — если `.projection-toggle` останется видимым при
|
||||
включённом `hp_alpha`, это либо добавит четвёртую группу в строку (не
|
||||
`.modes`/`header-action`/`.summary-control`/`.tabedit`/`.tabadd`, но всё равно
|
||||
лишний элемент), либо не уместится в 56 px — оба случая ловятся тем же
|
||||
измерением высоты/переноса, которое уже входит в AC2. Дыры в приёмке нет,
|
||||
только в удобстве чтения перечня. Снимаю без правки текста.
|
||||
|
||||
Оба Low не блокируют; ни один не требует остановки конвейера.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Присутствуют все обязательные разделы §7.1: сценарий и персона (одним
|
||||
абзацем, без терминов реализации — «одна строка ≤ 56 px» вместо ссылок на
|
||||
классы), «что человек увидит до/после» (интегрировано в тот же абзац,
|
||||
явно посчитаны текущие ≈106/≈156 px хрома), проблема по коду, скоуп и
|
||||
не-скоуп (полностью раздельны, без пересечения с #437/#629/#485/#53/#89),
|
||||
контракт поведения и UX (атрибуты `data-hp`, `aria-*`, размеры), модель
|
||||
данных/миграция/i18n, таблица AC1–AC8 с доказательством и мутацией, план
|
||||
автотестов, риски, откат, release-артефакты.
|
||||
- Все четыре продуктовых вопроса владельцу закрыты именно умолчаниями,
|
||||
указанными в аналитике, без домыслов от лица владельца.
|
||||
- Таблица меню в п.4 скоупа покрывает **все** реальные элементы текущей
|
||||
шапки без остатка (см. «Как проверялось», п.4) — ни один существующий
|
||||
контрол не потерян и не задублирован.
|
||||
- «Принято предположительно» корректно разграничивает продуктовое (уже
|
||||
решено владельцем) от инженерного (порог по ширине окна, а не карточки;
|
||||
disclosure- а не menu-паттерн; модуль `src/header-menu.ts`; подложка для
|
||||
тапа мимо) — ревьюер не оспаривает ни один пункт, они действительно
|
||||
нейтральны для пользователя.
|
||||
- Откат («один коммит, данных не касается») и release-артефакты (оба
|
||||
changelog + 4 канонических документа + golden предрелизно) — полны и не
|
||||
требуют технического решения владельца.
|
||||
- Открытых продуктовых вопросов к владельцу не осталось — все сняты
|
||||
предыдущим циклом аналитики; технических вопросов, ошибочно вынесенных
|
||||
владельцу, в тексте нет.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Гейты (`tsc`, `npm test`, `npm run build`, `check-docs.mjs`, смоки,
|
||||
golden, инварианты) — не прогонял: этап `spec`, продуктового кода к этой
|
||||
правке ещё нет; предмет ревью — текст ТЗ в теле issue, а не диапазон
|
||||
коммитов. Это станет предметом код-ревью после реализации.
|
||||
- Не проверял визуально в браузере фактическую компоновку на 320/390 px —
|
||||
ревью ТЗ не предполагает исполнения/ручного тестирования UI; все выводы о
|
||||
соответствии коду сделаны чтением `src/houseplan-card.ts` и
|
||||
`src/styles/dialogs.styles.ts`, а не измерением в реальном рендере.
|
||||
- Не оценивал производительность открытия/закрытия меню — ТЗ не меняет
|
||||
геометрию сцены и не вводит новых тяжёлых пересчётов (чистая функция
|
||||
состава + CSS-попап), риск явно ниже порога, требующего отдельного
|
||||
раздела; по прецеденту `SPEC-REVIEW-647-r1` для CSS-only правок такого
|
||||
масштаба это не считается пропуском ТЗ.
|
||||
- Не оценивал реализуемость «≤ 56 px» и «≥ 44 px» одновременно на пиксель —
|
||||
это инженерная, а не продуктовая величина; арифметически совместимо
|
||||
(56 − 44 = 12 px запаса на отступы), проверка факта — гейт код-ревью.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Найдено 0 High и 0 Medium в скоупе задачи; 2 Low сняты ревьюером с записью
|
||||
(см. «Находки»), правки текста не требуют. ТЗ переходит в «Готово к
|
||||
разработке».
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `e52afe63495b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `0977af5c3a98f43e9e74c633f2817e204e35f673`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 0977af5c3a98
|
||||
```
|
||||
- Тело issue: `d31c85f266be16c9f37e1df9ea9155014dea6fcc17761e493e3a6945ad360df9`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user