mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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 однозначен и имеет способ доказательства, технические утверждения о коде подтверждены построчно, продуктовые формулировки — прямые цитаты решений владельца, допущения корректно помечены. Оснований для возврата на правки нет.
|
||||
|
||||
**Зелёный.**
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/603-dialog-polish-2`, коммит `8cd04d4cbed1` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `aaa5bbe0b0d1109c997e33daad02e52b1c37cea7`
|
||||
```
|
||||
git log --all --format='%H %T' | grep aaa5bbe0b0d1
|
||||
```
|
||||
- Тело issue: `e3fa4cf47f624039a45b39ad23ab485c72afd9cf2cd47a00b0faa0a7404fdae8`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user