mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -1,9 +1,10 @@
|
||||
# Индекс ревью
|
||||
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1045, issue: 367. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1046, issue: 368. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
|
||||
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
||||
|---|---|---|---|---:|---:|---|---|
|
||||
| #647 | [SPEC-REVIEW-647-r1.md](SPEC-REVIEW-647-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 2 | П.4 ТЗ переносит на новый слот прежнее; ТЗ не упоминает и не защищает документированный | `src/styles/dialogs.styles.ts` `src/styles.ts` `smoke_glow_blending.mjs` `smoke_test_facade.mjs` `smoke_unified_wall_tool.mjs` `docs/UX-MODES.md` `docs/reviews/CODE-REVIEW-195-r1.md` `src/styles/chrome.styles.ts` |
|
||||
| #646 | [CODE-REVIEW-646-r1.md](CODE-REVIEW-646-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #645 | [SPEC-REVIEW-645-r1.md](SPEC-REVIEW-645-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 2 | Отсутствует обязательная строка Touch editor: … из docs/TOUCH-SUPPORT.md → «Documentati…; ТЗ не учитывает существующую зависимость live-подписей Resize от позиции кнопки «Настро…; / Low | `docs/TOUCH-SUPPORT.md` `docs/reviews/SPEC-REVIEW-449-r1.md` `docs/specs/359-furniture-placement-preview.md` `docs/specs/449-double-fit-all.md` `src/houseplan-editor-runtime.ts` `src/houseplan-card.ts` `src/resize-labels.ts` `docs/process/REVIEWER.md` |
|
||||
| #645 | [SPEC-REVIEW-645-r2.md](SPEC-REVIEW-645-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
|
||||
@@ -0,0 +1,254 @@
|
||||
# SPEC-REVIEW-647-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/647
|
||||
- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
|
||||
- **Трек:** `small` (лёгкий), по аналитике автора в комментарии
|
||||
`#issuecomment-5828900656`
|
||||
- **Материал:** тело issue #647, раздел `## ТЗ` (снимок на момент ревью,
|
||||
2026-09-25); комментариев — один («Аналитика»)
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 2 (лёгкий трек)
|
||||
- **Роль:** ревьюер ТЗ (не автор)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
ТЗ описывает две связанные правки шапки основной панели: (1) убрать
|
||||
текстовый счётчик устройств «N устр.» и (2) вынести крестик закрытия
|
||||
редактора из активной кнопки режима в отдельный слот фиксированной ширины
|
||||
на месте бывшего счётчика, чтобы вход/выход из редактора не менял ширину
|
||||
шапки. Проверялось: обязательные разделы §7.1, однозначность и
|
||||
проверяемость каждого AC, соответствие описания коду «как есть», согласие
|
||||
со SCOPE.md и с каноническим документом подсистемы (`docs/UX-MODES.md`),
|
||||
отсутствие домыслов, выданных за факт.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитан `docs/SCOPE.md` — задача обслуживает J6 (поддержание
|
||||
актуальности/качества интерфейса) как polish-правку самого механизма
|
||||
переключения режимов из J1/J6; прямой ссылки-строки на «полировку шапки»
|
||||
в Core user jobs нет, но это ожидаемо для `bug`/`polish` в лёгком треке —
|
||||
правка не добавляет функциональность, а чинит визуальный дефект уже
|
||||
принятого механизма (UX-MODES.md). Внутри скоупа.
|
||||
2. Прочитан `docs/process/REVIEWER.md` и разделы PROCESS.md §2.4, §2.5,
|
||||
§7.1, §4, §7.2.
|
||||
3. Прочитан весь текст issue #647 и комментарий «Аналитика».
|
||||
4. Прочитан `docs/USER-GUIDE.ru.md` — упоминаний счётчика устройств в
|
||||
шапке или крестика активного редактора с привязкой к тексту нет,
|
||||
условие ТЗ «если упомянут — убрать» корректно не требует правки этого
|
||||
документа.
|
||||
5. Прочитан `docs/UX-MODES.md` целиком (канонический документ подсистемы,
|
||||
явно указан в материале для этой задачи) — найдены два предложения,
|
||||
которые ТЗ #647 делает неточными, но не планирует обновлять
|
||||
(находка ниже).
|
||||
6. Прочитан код, который ТЗ описывает как причину бага:
|
||||
`src/houseplan-card.ts:10797-10820` (кнопки `.modetab`, крестик
|
||||
`.closex` внутри активной кнопки, `<span class="count">` после `.modes`
|
||||
как отдельный сиблинг) — описание совпадает с кодом дословно.
|
||||
7. Проверено число использований ключа `count.devices`:
|
||||
`grep -rn "count.devices" src/` — единственное место рендера
|
||||
(`src/houseplan-card.ts:10820`) плюс переводы в четырёх словарях;
|
||||
сводная панель #437 использует другой источник, ключ `count.devices`
|
||||
ею не читается. Утверждение ТЗ «ключ используется только счётчиком»
|
||||
подтверждено, а не принято на слово.
|
||||
8. Прочитан `src/styles/chrome.styles.ts:123-172` (`.modes`, `.modetab`,
|
||||
`.modetab .closex`) и `src/styles/dialogs.styles.ts:6-37` (`.head`,
|
||||
`.count`) — обе таблицы стилей входят в единый `cardStyles`
|
||||
(`src/styles.ts:17-19`), то есть правило `.head .count` из
|
||||
`dialogs.styles.ts` действует и на шапку основной панели, не только на
|
||||
диалоги (имя файла вводит в заблуждение, но код общий с #266).
|
||||
9. Прочитаны `docs/reviews/CODE-REVIEW-195-r1.md` и `r2.md` — предыдущая
|
||||
правка того же крестика (issue #195, «крестик не закрывает редактор с
|
||||
первого клика») зафиксировала контракт «hit-target ≥24×24 px» именно
|
||||
потому, что промах 2–3 px превращался в документированный no-op.
|
||||
Контракт записан в `docs/UX-MODES.md:41-44`.
|
||||
10. Проверено существование технических опор плана автотестов:
|
||||
`test/core-file-budget.test.mjs`, `scripts/smoke-links.mjs`,
|
||||
`scripts/mutation-registry.mjs`, `scripts/smoke-select.mjs`,
|
||||
`demo/helpers/hp-test.mjs` (там же — фасад `modeTab:
|
||||
'[data-hp="mode-tab"]...'`, ссылка на #629) — все существуют, ссылки
|
||||
точны.
|
||||
|
||||
Гейты не гонялись: этап `spec`, кода ещё нет (ветка `issue/647-...`
|
||||
упомянута в занятии, но диапазон ревью — только текст ТЗ). Это ожидаемо
|
||||
для ревью ТЗ, а не пропуск.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium-1 (в скоупе). П.4 ТЗ переносит на новый слот прежнее
|
||||
адаптивное поведение счётчика, не разбирая, что у счётчика и у крестика
|
||||
разные требования к видимости — риск спрятать крестик закрытия на очень
|
||||
частой ширине
|
||||
|
||||
**Файл:** тело issue #647, раздел «Скоуп», п.4.
|
||||
|
||||
**Формулировка ТЗ:** «Узкая ширина: существующие медиа-правила и
|
||||
перенос/скролл шапки сохраняются; слот участвует в них так же, как прежде
|
||||
счётчик (не создаёт нового горизонтального overflow)».
|
||||
|
||||
**Что не так.** Существующее правило, которое здесь имеется в виду,
|
||||
конкретно: `src/styles/dialogs.styles.ts:27-29`
|
||||
|
||||
```css
|
||||
@media (max-width: 1100px) {
|
||||
.head .count { display: none; }
|
||||
}
|
||||
```
|
||||
|
||||
Оно входит в общий `cardStyles` (`src/styles.ts:17-19`) и действует на
|
||||
шапку основной панели, а не только на диалоги. `max-width: 1100px` —
|
||||
это не редкий узкий кейс: большинство собственных смоков этого репозитория
|
||||
запускают страницу именно на ширине 1100 px (`demo/smoke_device_preview_
|
||||
parity.mjs:5`, `smoke_glow_blending.mjs:8`, `smoke_test_facade.mjs:10`,
|
||||
`smoke_unified_wall_tool.mjs:4` и другие) — то есть это обычная, а не
|
||||
крайняя ширина браузера/окна.
|
||||
|
||||
Счётчик было не жалко скрывать на такой ширине: это декоративная цифра.
|
||||
Крестик — не декоративная: это единственный способ выйти из редактора
|
||||
через шапку (`docs/UX-MODES.md:29-30`: «re-clicking the active tab does
|
||||
nothing»). Если реализация буквально выполнит п.4 — например, оставит
|
||||
слоту класс `.count` или скопирует то же медиа-правило — крестик исчезнет
|
||||
из шапки при ширине окна ≤1100 px, то есть в самой обычной обстановке, а
|
||||
не только на «узкой» ширине, которую описывает риск-раздел ТЗ.
|
||||
|
||||
Ни один AC этого не ловит: AC5/К5 проверяет только `scrollWidth <=
|
||||
clientWidth` на 390 и 768 px (overflow), а не видимость крестика; AC4/К4
|
||||
проверяет, что крестик «виден и срабатывает только в редакторе», но не
|
||||
указывает диапазон ширин, на которых это должно быть верно, и не
|
||||
исключает диапазон 721–1100 px. Формально реализация, скрывающая крестик
|
||||
на 900 px ширины, может пройти все перечисленные AC.
|
||||
|
||||
Смягчающее обстоятельство: у каждого редактора есть свой независимый
|
||||
крестик в его нижней панели инструментов (`.btn.barclose`,
|
||||
`src/houseplan-editor-runtime.ts:5141,11176,11223`) — то есть выйти из
|
||||
редактора можно и без крестика в шапке. Поэтому это не полный отказ
|
||||
функции, а тихий регресс одного из двух документированных способов
|
||||
закрытия редактора, воспроизводимый на самой обычной ширине окна.
|
||||
|
||||
**Как закрыть в ТЗ.** Явно указать, что новый слот **не** наследует
|
||||
скрытие `.head .count` (например: слот получает собственное имя класса
|
||||
или явное `display: <контейнер> !important` вне медиа-правила), и
|
||||
добавить к AC5/К5 проверку «крестик виден и кликабелен» хотя бы на
|
||||
ширинах из существующего диапазона теста (390/768 px) плюс на ширине
|
||||
900–1100 px, раз именно там расположено унаследованное правило.
|
||||
|
||||
### Medium-2 (в скоупе). ТЗ не упоминает и не защищает документированный
|
||||
контракт «hit-target крестика ≥24×24 px» (docs/UX-MODES.md:41-44,
|
||||
issue #195); канонический документ подсистемы не запланирован к правке
|
||||
|
||||
**Файл:** `docs/UX-MODES.md:41-44`; тело issue #647, разделы «Контракт
|
||||
поведения», «AC · чем доказан», «Release-артефакты».
|
||||
|
||||
**Что не так.** Действующий контракт зафиксирован дословно:
|
||||
|
||||
> The header X keeps its compact 13 px glyph but owns a hit target of at
|
||||
> least 24 × 24 px without changing the tab's layout footprint.
|
||||
|
||||
Это не декоративная деталь, а результат реального разбора бага (issue
|
||||
#195, `docs/reviews/CODE-REVIEW-195-r1.md`): промах 2–3 px по глифу 13×13
|
||||
делал клик по крестику no-op. Фикс расширил кликабельную зону крестика до
|
||||
24×24 (`src/styles/chrome.styles.ts:154-165`, приём с отрицательными
|
||||
полями `margin: -5.5px -5.5px -5.5px -3.5px`, чтобы визуальный футпринт
|
||||
кнопки-режима не рос).
|
||||
|
||||
ТЗ #647 переносит крестик из кнопки режима в новый, независимый слот, но:
|
||||
- ни в «Контракте поведения» (К1–К5), ни в таблице AC нет строки,
|
||||
требующей сохранить минимальный hit-target ≥24×24 px в новом слоте —
|
||||
AC4/К4 проверяет только факт клика и доступность фокуса/aria, но не
|
||||
размер кликабельной области;
|
||||
- «принято предположительно» фиксирует только то, что ширина слота — «одна
|
||||
CSS-переменная, равная размеру кнопки закрытия», не называя это значение
|
||||
и не связывая его с существующим контрактом ≥24×24 px;
|
||||
- «Release-артефакты» планирует правку `USER-GUIDE.ru/en` (условно — там
|
||||
и так ничего нет, см. «как проверялось» п.4) и golden, но не называет
|
||||
`docs/UX-MODES.md`, хотя это явно указанный в материале канонический
|
||||
документ подсистемы, и он станет фактически неточным сразу в двух
|
||||
местах: строка 13 («space tabs, editor navigation, **device count**,
|
||||
zoom and actions remain» — счётчика больше не будет) и строки 41-44
|
||||
(описывают крестик как часть футпринта кнопки-режима, чего после этой
|
||||
правки уже не будет).
|
||||
|
||||
Отдельно: комментарий «Аналитика» обосновывает лёгкий трек в том числе
|
||||
тезисом «touch-контракт не меняется (кнопки, их размер и hit area
|
||||
прежние...)». Это утверждение не подкреплено ни ТЗ, ни кодом — hit area
|
||||
крестика как раз перестаёт быть тем, чем была (переезжает из кнопки
|
||||
режима в новый слот с неназванным размером), и это именно то, что
|
||||
описывает §7.1 как догадку, записанную как факт.
|
||||
|
||||
**Как закрыть в ТЗ.** Добавить в «Контракт поведения» пункт: минимальная
|
||||
кликабельная область крестика в новом слоте ≥24×24 px (или явно
|
||||
задокументировать другое решение и обновить `docs/UX-MODES.md` этим же
|
||||
изменением, раз ТЗ меняет описанную там архитектуру), добавить это в
|
||||
таблицу AC с проверкой размера (`getBoundingClientRect` кликабельного
|
||||
элемента, а не только факт клика), и включить `docs/UX-MODES.md` в
|
||||
«Release-артефакты» рядом с USER-GUIDE.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Обязательные разделы §7.1 присутствуют все: сценарий, «что человек
|
||||
увидит до/после», проблема, скоуп/не-скоуп, контракт поведения, UX/i18n/
|
||||
модель данных, таблица AC с доказательством, план автотестов, риски,
|
||||
откат, release-артефакты.
|
||||
- Описание бага «по коду» и границы правки (счётчик — отдельный элемент
|
||||
после `.modes`, крестик — внутри активной кнопки режима) точно совпадают
|
||||
с `src/houseplan-card.ts:10797-10820`.
|
||||
- Удаление ключа `count.devices` безопасно: подтверждено, что у него ровно
|
||||
одно место чтения в коде (сводная панель #437 использует другой
|
||||
источник данных, её AC6 в таблице корректно требует не трогать эту
|
||||
логику).
|
||||
- AC1–AC3, AC6 однозначны, привязаны к конкретному способу доказательства
|
||||
(смок с именем файла, конкретный DOM-путь/метрика) и называют мутацию,
|
||||
на которой тест должен покраснеть — колонка «чем краснеет» не пустая ни
|
||||
в одной строке таблицы, кроме AC5, где отсутствие мутанта прямо
|
||||
объяснено как регресс-проверка (приемлемо для не-защитного AC об
|
||||
overflow).
|
||||
- План автотестов ссылается на реально существующие механизмы:
|
||||
`test/core-file-budget.test.mjs`, `scripts/smoke-links.mjs`,
|
||||
`scripts/mutation-registry.mjs`, `scripts/smoke-select.mjs`, фасад
|
||||
`data-hp="mode-tab"` в `demo/helpers/hp-test.mjs` (#629) — ссылки не
|
||||
выдуманы, соответствуют текущему коду.
|
||||
- Не-скоуп сформулирован явно и без пересечения с #437.
|
||||
- Риски раздела учитывают golden-регресс (шапка почти во всех кадрах) и
|
||||
необходимость обновить смоки, которые ищут крестик внутри кнопки
|
||||
режима — это отдельно от находок выше и закрыто корректно.
|
||||
- `User-Visible: yes` и требование обоих changelog в одном коммите
|
||||
указаны верно.
|
||||
- Открытых продуктовых вопросов владельцу нет и не требовалось: поведение
|
||||
полностью описано в теле issue, включая явное указание места крестика.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Гейты (`tsc`, `npm test`, `npm run build`, `check-docs.mjs`, смоки,
|
||||
golden, инварианты) — не прогонял: этап `spec`, продуктового кода к этой
|
||||
правке ещё нет, диапазон ревью — текст ТЗ в теле issue, а не диапазон
|
||||
коммитов. Это будет предметом код-ревью после реализации.
|
||||
- Не проверял визуально в браузере текущую ширину/расположение крестика и
|
||||
счётчика (ревью ТЗ не предполагает ручного тестирования UI) — вывод
|
||||
о правиле `.head .count` и ширине `1100px` сделан чтением CSS и состава
|
||||
`cardStyles`, а не измерением в реальном рендере; при код-ревью это
|
||||
стоит перепроверить смоком на ширинах 900–1100 px, если автор примет
|
||||
находку Medium-1.
|
||||
- Не оценивал производительность/аниматику перехода между режимами — ТЗ
|
||||
не меняет этот механизм, вне скоупа задачи.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Найдено 0 High и 2 Medium в скоупе задачи. Оба Medium закрываются
|
||||
правкой текста ТЗ (не кода): явно оговорить, что новый слот не наследует
|
||||
скрытие бывшего счётчика на обычных, не только «узких», ширинах, и
|
||||
явно защитить документированный контракт минимального hit-target
|
||||
крестика (≥24×24 px, `docs/UX-MODES.md:41-44`), включая обновление самого
|
||||
`docs/UX-MODES.md` в Release-артефактах. Без High это жёлтый вердикт —
|
||||
ТЗ возвращается автору на доработку текста, отдельные issue не заводятся.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `b4d6effbca80` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `973c05543e3f539e1a4cdf476e8cd13e42e86413`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 973c05543e3f
|
||||
```
|
||||
- Тело issue: `3fcd14cbc60c13c45ead7c441a8eb79b1b203acb2be8fd56fa40f5f253c523af`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user