From b63373fb9c6381e0ab31fa39773ac9bdfbf39044 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 21 Sep 2026 05:39:53 +0000 Subject: [PATCH] docs: review document for #603 Issue: #603 User-Visible: no --- docs/reviews/SPEC-REVIEW-603-r1.md | 93 ++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-603-r1.md diff --git a/docs/reviews/SPEC-REVIEW-603-r1.md b/docs/reviews/SPEC-REVIEW-603-r1.md new file mode 100644 index 00000000..4c68a450 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-603-r1.md @@ -0,0 +1,93 @@ +# SPEC-REVIEW-603-r1 + +**Issue:** [#603](https://github.com/Matysh/houseplan-card/issues/603) — «Полировка редизайна диалогов, часть 2: confirm, переключатель, комната» +**Этап:** ТЗ на ревью (PROCESS.md §2.4), полный трек (аналитик явно назвал критерий §5, который задача не проходит: «одна поверхность») +**Заход:** r1 · блокирующих циклов израсходовано 0 из 4 +**Ревьюер:** независимая сессия, без контекста автора ТЗ + +## Скоуп + +ТЗ живёт в теле issue #603, раздел `## ТЗ` (владелец, 2026-09-10, #517). Три независимых визуальных дефекта после #602/beta.4: + +1. discard-confirm диалог (несохранённые изменения) — обрезка левой кнопки на узкой ширине, RU-подписи слишком длинные; +2. общий form-kit toggle — визуальное смещение шарика в дорожке Off/On; +3. диалог настроек комнаты — лишний заголовок `h3` «Основное» первой карточки. + +Заявлено: 6 AC (AC1…AC6), контракт UX, данные/i18n/touch/perf, модули и артефакты, риски/откат/предположения. + +## Как проверялось + +Материал — тело issue #603 (получено `mcp__github__get_issue` / `gh issue view`), комментарии аналитика и автора. Ревью **чтением**, без исполнения кода (этап ТЗ, кода ещё нет — до `S5-ready` продуктовый код не менялся, статус подтверждён комментарием автора «Взял: автор ТЗ»). + +Прочитано и сопоставлено с текстом ТЗ: + +- `docs/SCOPE.md` — персона/job (J4/J6), правило «View — продукт», лимиты touch; +- `AGENTS.md`, `PROCESS.md` §1–§10 (классы файлов, лёгкий/полный трек, §7.1 обязательные разделы, §2.4 критерии ревью ТЗ, шаблон вердикта §7.2); +- `docs/USER-GUIDE.ru.md` и `docs/USER-GUIDE.md` — терминология диалогов, кнопок и карточек (discard-confirm, Отменить/Повторить toolbar, карточка «Основное»/«Basics»); +- `docs/TOUCH-SUPPORT.md`, `docs/UX-MODES.md` — 44×44 hit area, десктоп-first редакторы, отсутствие конфликтов с режимной моделью; +- исходный код заявленных модулей: `src/hp-confirm.ts`, `src/danger-confirm.ts`, `src/styles/dialogs.styles.ts`, `src/styles/form-kit.styles.ts`, `src/editors/form-kit.ts` (`formCard`), `src/editors/room-settings-dialog.ts`, `src/editors/{marker-dialog,space-form,general-settings-dialog}.ts`, `src/i18n/settings/{ru,en,de,fr}.json`; +- существующая тестовая инфраструктура: `demo/smoke_dialog_footer_width.mjs` (константа `NARROW_WIDTH=320`, порог 560px для `form-shell`), `demo/smoke_{room,space,device,general}_settings_form.mjs`, `demo/smoke_danger_confirm*.mjs`, `demo/golden/run.mjs` и `matrix.mjs` (поддержка `deviceScaleFactor`, языковые/тематические сценарии), `demo/capture_summary_panel_505.mjs` (прецедент имитации 200% zoom через эффективный viewport). + +Цель — не согласиться с текстом, а проверить: выполнимость и однозначность каждого AC, соответствие технических утверждений реальному коду, отсутствие догадки под видом решения, отсутствие противоречий с каноном. + +## Находки + +### Low-1 — «Отменить» в discard-confirm пересекается с уже существующим термином «Отменить» = Undo + +**Файл:** тело issue #603, раздел `## ТЗ` → «Контракт UX», пункт 1 и «Риски, откат, принятые предположения». + +**Суть:** ТЗ (по прямому наблюдению владельца) вводит подпись кнопки **«Отменить»** для действия «отбросить черновик и закрыть форму». Но `docs/USER-GUIDE.ru.md:465` и `:1030` фиксируют, что **«Отменить»** — уже действующее, видимое название постоянной кнопки **Undo** в панелях редактора плана/подложки и в редакторе устройств (парная кнопка «Повторить» = Redo, история на 50 шагов). Смысл там принципиально другой и *не* деструктивный: «отменить последний шаг и продолжить работу», а не «закрыть форму, потеряв все несохранённые правки». Пользователь, знающий toolbar-Undo, может интерпретировать кнопку confirm-диалога как безопасное «шаг назад, остаюсь здесь» — обратное тому, что она делает. + +Раздел «Риски» ТЗ называет двусмысленность генерически («Отменить вне контекста двусмысленно») и предлагает митигацию (заголовок диалога, сообщение о несохранённых изменениях, безопасный первый порядок с autofocus на «Продолжить»), но не называет именно этот — уже задокументированный — источник конфликта. + +**Почему не блокирует:** формулировка кнопки — прямая цитата из «Наблюдений владельца» (продуктовое решение, принятое им самим, а не догадка автора ТЗ); митигация (заголовок + сообщение остаются, safe default первым) реально снижает риск случайной потери данных, которую AC1 и так требует проверить смоком; AC не меняется, никакой автотест не станет ложно-зелёным. + +**Рекомендация (не блокирует, снимается с записью):** одной строкой явно назвать в «Рисках» существующий toolbar-термин `Отменить`=Undo (`USER-GUIDE.ru.md:465,1030`) как конкретный источник конфликта — это ускорит код-ревью, где иначе придётся находить этот прецедент заново. + +## Проверено и корректно + +- **Обязательные разделы §7.1** — все присутствуют (сценарий+что-увидит, проблема, скоуп/не-скоуп, контракт UX, данные/миграция, i18n, AC1–AC6 с доказательством, модули+план автотестов, риски, откат, release-артефакты); первые два — продуктовые и стоят первыми, как требует §7.1. +- **Персона/поверхность/момент** названы явно: администратор (Home admin, `docs/SCOPE.md`), десктопные редакторы (form-kit диалоги), момент — выход с несохранёнными правками / открытие настроек комнаты. +- **Трек и его обоснование** — аналитик прямо назвал нарушенный критерий §5 («одна поверхность»: confirm+i18n, общий toggle четырёх форм, отдельная карточка комнаты), а не написал «обычный трек» без причины. +- **Каждый AC однозначен и имеет названный способ доказательства** (AC1–AC6, таблица «Наблюдаемый результат · Свидетель»); ни один не сформулирован как нефальсифицируемое «работает лучше». +- **Технические утверждения о коде проверены построчно и подтверждены**: + - `src/hp-confirm.ts`/`danger-confirm.ts`: `cancelLabel` (safe, autofocus, первый) = `dialog.discard_keep`, `confirmLabel` (второй, danger-стиль) = `dialog.discard_confirm`; Escape/крестик/скрим действительно маппятся на `_decide(false)` = «продолжить» — ТЗ описывает это верно, не как догадку; + - `src/styles/dialogs.styles.ts:1297-1380`: `.danger-confirm-footer` уже принудительно `flex-wrap: nowrap` на узкой ширине — совпадает с описанием «уже один ряд» в разделе «Воспроизведение»; + - `src/styles/form-kit.styles.ts:267-291`: `::before`(inset 11px 4px)/`::after`(top 50%, left 7px/21px) — числа в ТЗ совпадают с кодом байт-в-байт; + - `src/editors/form-kit.ts:77-84`: `formCard()` уже поддерживает пустой `title` («карточка без шапки») — механизм для AC5 существует в проекте до этой задачи, значит удаление `h3` не требует придумывать новый API; + - все 4 потребителя `dialog.discard_keep/discard_confirm` (`marker-dialog`, `space-form`, `room-settings-dialog`, `general-settings-dialog`) действительно используют один и тот же ключ и одну и ту же семантику — заявление «общий CSS влияет на четыре формы» точное, не занижает и не завышает влияние. +- **Ambiguity vs assumption** — оба явно помеченных допущения (точный CSS-приём выравнивания; точные слова EN/DE/FR; сохранение неиспользуемого i18n-ключа `room.group_basics`) корректно вынесены в блок «принято предположительно, поменять свободно» и не выданы за факт. Продуктовых вопросов, скрытых под видом технических, не найдено — размытых мест, которые требовали бы эскалации владельцу, нет: все формулировки взяты прямо из «Наблюдений владельца» и подтверждаются кодом. +- **Тестируемость AC не переоценена**: заявленные способы доказательства (browser smoke на 320/360/560px — совпадает с уже существующей константой `NARROW_WIDTH=320` и порогом 560px для `form-shell` в `smoke_dialog_footer_width.mjs`; golden с `deviceScaleFactor` 1/2 — механизм есть в `demo/golden/run.mjs`/`serve.mjs`; имитация 200% zoom — есть прецедент в `demo/capture_summary_panel_505.mjs`) опираются на реально существующую инфраструктуру, а не на несуществующие возможности гейтов. +- **Соответствие `docs/USER-GUIDE.ru.md`/`.md`**: обе версии описывают карточку комнаты через заголовок «Основное»/«Basics» (`USER-GUIDE.ru.md:601`, `USER-GUIDE.md:523`) — ТЗ корректно требует их обновления («RU/EN user guide при наличии описания заголовка»), а не оставляет документацию устаревшей. +- **`docs/TOUCH-SUPPORT.md`**: требование 44×44 hit area в AC4 — не изобретённое число, совпадает с каноном (`TOUCH-SUPPORT.md:14,72,97`). +- **Модель данных/миграция/compatibility**: задача действительно не касается конфигурации, `docs/CONFIG-COMPATIBILITY.md` не затронут — заявление «нет миграции» верно по факту отсутствия персистентных полей в затронутых модулях. +- **Откат** соразмерен риску (чистый revert коммита, без миграции). +- **Скоуп/не-скоуп** заданы симметрично и без утечки: другие confirm-сценарии, другие заголовки (например, «Основное» в диалоге устройства, `USER-GUIDE.ru.md:1056`), размер дорожки/hit area явно исключены — не позволяет скоуп-крип на соседние диалоги. + +## Чего не проверял + +- Не проверялся код реализации — его нет: продуктовый код по правилу не менялся до `S5-ready`, что подтверждено комментарием автора. +- Не запускались гейты (`typecheck`/`test`/`build`/golden/smoke) — на этапе ТЗ они неприменимы, кода ещё нет. +- Не проверялась фактическая пиксельная величина визуального смещения шарика тумблера (требует рендера в браузере) — на этапе ТЗ это не нужно: числа CSS в коде совпадают с текстом ТЗ, а конкретный приём исправления прямо помечен как допущение для реализации. +- Не оценивались точные слова будущих EN/DE/FR подписей кнопок confirm — ТЗ сознательно не фиксирует их (см. блок допущений), это в компетенции разработчика/ревью кода. +- Связанные issue #591/#600/#602 прочитаны только через ссылки и итог в тексте ТЗ, не разбирались целиком построчно — не требовалось: #603 явно объявлен отдельным дефектом, не переоткрытием #602. + +## Вердикт + +High: 0 · Medium: 0 · Low: 1 (снят с записью, см. Low-1). Обязательные разделы §7.1 на месте, каждый AC однозначен и имеет способ доказательства, технические утверждения о коде подтверждены построчно, продуктовые формулировки — прямые цитаты решений владельца, допущения корректно помечены. Оснований для возврата на правки нет. + +**Зелёный.** + +--- + + + +## Материал раунда + +- Ветка: `issue/603-dialog-polish-2`, коммит `8cd04d4cbed1` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `aaa5bbe0b0d1109c997e33daad02e52b1c37cea7` + ``` + git log --all --format='%H %T' | grep aaa5bbe0b0d1 + ``` +- Тело issue: `e3fa4cf47f624039a45b39ad23ab485c72afd9cf2cd47a00b0faa0a7404fdae8` +- Вердикт конвейера: `green` · High 0