18 KiB
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, полнота ответа на все четыре вопроса
владельцу, отсутствие домыслов, выданных за факт.
Как проверялось
docs/SCOPE.md— персона «Household members» на телефоне, для которой «View mode is the product for two of the three personas» — прямое попадание в это ограничение: задача убирает редакторские аффордансы и лишние ряды хрома именно на этой поверхности. Не пересекается ни с одним пунктом «Out of scope». В скоупе.docs/process/REVIEWER.mdцеликом и разделы PROCESS.md §2.4, §2.5, §7.1, §4, §7.2.- Прочитан весь текст issue #616 и оба комментария владельца — все четыре вопроса закрыты умолчаниями («да / нет, скрыть на любой ширине / да, редакторы desktop-first / да, kiosk не меняется»), в ТЗ они отражены без искажения.
- Сверено описание «Проблема (по коду)» с
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 скоупа. - Сверено
.head { flex-wrap: wrap }и медиа-правило@media (max-width: 620px)вsrc/styles/dialogs.styles.ts:6-30— описание «flex-wrap: wrapбез правил для телефона, кроме уменьшения отступов на ≤ 620 px» совпадает дословно. - Прочитаны
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» файлы для правки названы точно. - Подтверждено существование 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). - Подтверждено существование всех смоков, названных в «Плане тестов»:
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 проверяется слот ×») — риск описан точно, а не декларативно. - Проверен потолок строк:
test/core-file-budget.test.mjs:40—'src/houseplan-card.ts': 12889;wc -lпоказывает 12886 — запас 3 строки. ТЗ верно определяет это как жёсткий риск и явно выносит логику меню в новыйsrc/header-menu.ts, а не расширяет монолит — без этого решения задача не проходила бы гейтcore-file-budgetпочти гарантированно. - Проверены локали
en/ru/de/fr(src/i18n/{en,de,fr,ru}.jsonреально существуют) — новый ключtitle.header_menuзаявлен для всех поддерживаемых языков, ни один не пропущен и лишних не добавлено. - Сверен таргет 44×44 с существующим стандартом
docs/TOUCH-SUPPORT.md:14,72— переиспользует принятую метрику, не изобретает новую. - Проверены все восемь 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— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
0977af5c3a98f43e9e74c633f2817e204e35f673git log --all --format='%H %T' | grep 0977af5c3a98 - Тело issue:
d31c85f266be16c9f37e1df9ea9155014dea6fcc17761e493e3a6945ad360df9 - Вердикт конвейера:
green· High 0