diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 7b00cc6d..dd764a86 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -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 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-616-r1.md b/docs/reviews/SPEC-REVIEW-616-r1.md new file mode 100644 index 00000000..e7adaf74 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-616-r1.md @@ -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.{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 сняты ревьюером с записью +(см. «Находки»), правки текста не требуют. ТЗ переходит в «Готово к +разработке». + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `e52afe63495b` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `0977af5c3a98f43e9e74c633f2817e204e35f673` + ``` + git log --all --format='%H %T' | grep 0977af5c3a98 + ``` +- Тело issue: `d31c85f266be16c9f37e1df9ea9155014dea6fcc17761e493e3a6945ad360df9` +- Вердикт конвейера: `green` · High 0