mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 13:18:58 +00:00
@@ -1,9 +1,10 @@
|
||||
# Индекс ревью
|
||||
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1084, issue: 384. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1085, issue: 385. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
|
||||
|
||||
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|
||||
|---|---|---|---|---:|---:|---|---|
|
||||
| #665 | [SPEC-REVIEW-665-r1.md](SPEC-REVIEW-665-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | заявленная правка docs/ISOMETRIC.md | `docs/ISOMETRIC.md` `docs/adr/160-isometric-stage3-overlays.md` `check-docs.mjs` `ISOMETRIC.md` |
|
||||
| #664 | [CODE-REVIEW-664-r1.md](CODE-REVIEW-664-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
| #663 | [SPEC-REVIEW-663-r1.md](SPEC-REVIEW-663-r1.md) | spec · r1 | 🟡 жёлтый | 1 | 0 | §6.2 (wall snap) и §8 (canonicalization/Optimize) описывают два; AC7 называет четыре состояния сломанной; AC13 не называет конкретный бюджет | `docs/CANVAS.md` `docs/WALL-THICKNESS.md` |
|
||||
| #663 | [SPEC-REVIEW-663-r2.md](SPEC-REVIEW-663-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
|
||||
|
||||
@@ -0,0 +1,213 @@
|
||||
# SPEC-REVIEW-665-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/665
|
||||
- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
|
||||
- **Трек:** лёгкий (`small`), но не `trivial` — авторская аналитика (комментарий
|
||||
1) сама называет причину: правка задевает цель касания поднятой подписи
|
||||
комнаты в 2.5D View/киоске (блокирующая зона `TOUCH-SUPPORT.md`), поэтому её
|
||||
сохранение доказывается отдельным AC, а не декларацией — отсюда полноценное
|
||||
ревью ТЗ, а не короткий трек.
|
||||
- **Материал:** тело issue #665 (единственная редакция, раздел `## ТЗ`) +
|
||||
комментарий владельца (аналитика/оценка, метка `S4-spec-review`, вопросов
|
||||
владельцу нет).
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 2
|
||||
- **Роль:** ревьюер ТЗ (не автор)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
CSS-баг в 2.5D View: `.stage.projection-iso.mode-view .roomlabel` использует
|
||||
`min-height: 44px` + `justify-content: center` как способ дать поднятой подписи
|
||||
комнаты минимальную цель касания. Побочный эффект — расстояние от имени
|
||||
комнаты до строки показателей (`.rlmetrics`, абсолютно позиционирована от
|
||||
низа контейнера подписи) зависит от того, насколько 44 px больше высоты
|
||||
самого имени, то есть меняется при зуме. Во Flat такого эффекта нет — там
|
||||
контейнер не растягивается, и зазор — постоянные `0,15em`. Контракт: перенести
|
||||
44×44 px минимум с самого блока подписи на невидимый `::before` (приём уже
|
||||
применяется для `.dev`/`.oplock`), вернув геометрию блока подписи к
|
||||
поведению Flat. Не в скоупе: размер шрифта подписи на зуме, состав
|
||||
показателей, раскладка столкновений поднятых подписей.
|
||||
|
||||
**SCOPE-проверка (`docs/SCOPE.md`):** правка обслуживает J1 («show the whole
|
||||
home… room fills… values») и опирается на узкое исключение #89 (2.5D —
|
||||
детерминированная проекция того же состояния/геометрии, что и Flat, без
|
||||
второй модели): здесь она восстанавливает именно это равенство, устраняя
|
||||
единственное расхождение геометрии подписи между Flat и 2.5D. Новой
|
||||
scope-дыры не создаёт: правка не меняет состав показателей, не вводит новую
|
||||
интерактивность и не расширяет исключение #89.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны `docs/SCOPE.md`, `docs/process/REVIEWER.md`, по ссылкам —
|
||||
PROCESS.md §2.4, §2.5, §7.1, §7.2, §4 (§2.10 не применялся — это r1).
|
||||
2. Прочитано тело issue #665 целиком (`gh issue view 665 --json body`) и
|
||||
единственный комментарий владельца (`--json comments`) — вопросов
|
||||
владельцу нет, подтверждаю по тексту.
|
||||
3. Сверены обязательные разделы §7.1: сценарий (персона «домочадцы и киоск»,
|
||||
поверхность 2.5D View, момент — зум) — на месте и без терминов реализации;
|
||||
«что человек увидит до/после» — одной фразой каждое; проблема и причина
|
||||
воспроизведены исполнением (демо-стенд, таблица замеров на 4 уровнях зума);
|
||||
скоуп/не-скоуп — явно; контракт C1–C3 — однозначен; модель
|
||||
данных/миграция/i18n — явное «нет»; AC1–AC3 — каждый с указанным способом
|
||||
доказательства (smoke/smoke/мутант); откат — «вернуть правило»;
|
||||
release-артефакты — changelog RU+EN, golden (условно, с оговоркой о приёмке
|
||||
по CI), скриншоты документации. Явного заголовка «Риски» и «UX» нет, но
|
||||
содержание покрыто (см. находку Low-1 ниже — только по одному пункту, доке).
|
||||
4. **Перепроверены фактические утверждения ТЗ по коду на SHA `942b9ede`
|
||||
(== origin/dev), а не приняты на слово:**
|
||||
- `src/styles/plan.styles.ts:692-702` — правило `.stage.projection-iso
|
||||
.mode-view .roomlabel { min-width: 44px; min-height: 44px;
|
||||
justify-content: center; … }` существует буквально как процитировано в
|
||||
issue.
|
||||
- `src/styles/plan.styles.ts:555-570,647-649` — `.roomlabel` это flex-column
|
||||
c `gap: 0.15em`; `.rlname`/`.rlmetrics` (`src/houseplan-card.ts:12077,
|
||||
12087`) — прямые дети; `.rlmetrics` вынута из потока
|
||||
(`position: absolute; top: calc(100% + 0.15em)`), поэтому единственный
|
||||
flex-элемент — `.rlname`, и в 2.5D `justify-content: center` при
|
||||
`min-height: 44px` центрирует имя в блоке высотой 44 px, а
|
||||
`top: calc(100% + 0.15em)` считается от высоты **контейнера**
|
||||
(`.roomlabel`), а не от низа имени. Математика issue
|
||||
(`(44 − высота_имени)/2 + 0.15em`) воспроизводится по коду точно, не на
|
||||
веру.
|
||||
- `demo/smoke_isometric_contract.mjs:36-47` (`owns44`) — уже берёт
|
||||
`Math.max(rect.width/height, ::before width/height)`, то есть уже
|
||||
нейтрален к тому, откуда взялся 44-px минимум (сам блок или его
|
||||
`::before`). Утверждение AC2 «`owns44` уже учитывает `::before`» —
|
||||
подтверждено чтением, не принято на слово.
|
||||
- `src/iso-scene-render.ts:229-264` (`isoRaisedOverlayHalfSize`, кейс
|
||||
`'room-label'`) — считает половину футпринта из `nameHeight`/текстовых
|
||||
метрик шрифта, константы `44` в функции нет. Утверждение C3 «раскладка
|
||||
столкновений уже считает от шрифта, а не от 44 px» — подтверждено.
|
||||
- `src/styles/plan.styles.ts:541-551` (`.oplock::before`) — приём
|
||||
invisible-`::before` с `width/height: max(44px, 100%)`,
|
||||
`transform: translate(-50%, -50%)`, `pointer-events: auto` уже применяется
|
||||
в проекте буквально в предложенном автором виде — «принятое предположение»
|
||||
не гипотетическое, а копия рабочего паттерна.
|
||||
- `src/houseplan-card.ts:12058-12060` — узел с
|
||||
`data-hp-iso-overlay-kind="room-label"` (тот, что проверяет `owns44`) —
|
||||
это и есть сам `.roomlabel`, а не обёртка вокруг него; перенос
|
||||
минимального размера на его собственный `::before` не рассинхронизирует
|
||||
смок с реальным деревом DOM.
|
||||
5. Проверен единственный сомнительный пункт «Затронутые файлы» — заявленная
|
||||
правка `docs/ISOMETRIC.md` («строка про цель касания подписи»): найдена
|
||||
находка Low-1 ниже.
|
||||
6. `git branch -a` / `git log --all --oneline | grep 665` — ветки `issue/665-*`
|
||||
и коммитов с трейлером `Issue: #665` не существует; `git diff
|
||||
origin/dev...HEAD` пуст (HEAD == origin/dev == материал). Продуктового кода
|
||||
для #665 нет — стадия `spec`, гейты (`tsc`, `test`, `build`, смоки, golden,
|
||||
инварианты) неприменимы к этому этапу, это штатное состояние, а не находка.
|
||||
|
||||
## Находки
|
||||
|
||||
### Low-1 (снят без возврата) — заявленная правка `docs/ISOMETRIC.md`
|
||||
(«строка про цель касания подписи») не соответствует текущему документу
|
||||
|
||||
**Что не так.** Раздел «Затронутые файлы» называет `docs/ISOMETRIC.md` со
|
||||
пометкой «строка про цель касания подписи», подразумевая правку существующей
|
||||
строки. Проверено чтением всего файла: `docs/ISOMETRIC.md` не содержит ни
|
||||
числа `44`, ни слова `touch`/`target`/`tap` применительно к подписи комнаты, ни
|
||||
упоминания приёма `::before`. Единственное место в проекте, где написано «keeps
|
||||
its existing axis-aligned minimum 44 CSS-pixel target», — это
|
||||
`docs/adr/160-isometric-stage3-overlays.md:60`, ADR-документ (заморожена
|
||||
история решения), и эта фраза не завязана на конкретный CSS-механизм
|
||||
(`min-height` vs `::before`) — после правки она останется истинной без
|
||||
изменений.
|
||||
|
||||
**Почему не High/Medium.** Это не открытый продуктовый вопрос и не
|
||||
противоречие контракту: ни один AC не зависит от этой правки, `check-docs.mjs`
|
||||
не привязывает конкретную CSS-реализацию к тексту `ISOMETRIC.md`. Худший
|
||||
исход — исполнитель не найдёт заявленную строку, ничего не тронет в этом файле
|
||||
и это не сломает ни один AC.
|
||||
|
||||
**Что нужно (не блокирует).** При реализации либо снять этот пункт из
|
||||
«Затронутые файлы» (в `docs/ISOMETRIC.md` сейчас нечего редактировать), либо
|
||||
явно решить, что добавляется новая строка (например, рядом с «Device markers,
|
||||
room labels/cards and opening-lock badges keep their canonical floor anchors…»,
|
||||
`docs/ISOMETRIC.md:220`) с фактом «label touch target size lives on an
|
||||
invisible `::before`, matching `.dev`/`.oplock`» — тогда это новый факт, а не
|
||||
правка существующего. Оставляю на усмотрение исполнителя; не создаю отдельный
|
||||
issue (Low, в скоупе, не блокирует).
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Обязательные разделы §7.1 присутствуют по существу и в разумном порядке для
|
||||
однострочного CSS-бага: сценарий и «что человек увидит» — продуктовые, без
|
||||
терминов реализации; проблема подкреплена исполненной репродукцией (реальный
|
||||
демо-стенд, а не гипотеза); скоуп/не-скоуп разделены явно.
|
||||
- **Причина бага перепроверена по коду, а не принята на веру** — воспроизведена
|
||||
вся цепочка: `min-height`+`justify-content:center` на flex-контейнере с одним
|
||||
in-flow элементом + `.rlmetrics` абсолютно спозиционирована от контейнера, а
|
||||
не от имени (см. «Как проверялось» п.4). Табличные замеры в issue (4 уровня
|
||||
зума, отношение `gap/высота_имени` падает с 2,0 до 0,16 в 2.5D и держится на
|
||||
~0,09–0,1 во Flat) согласуются с этой моделью без противоречий.
|
||||
- Контракт C1–C3 однозначен и без пересечений: C1 — числовая цель (`0,15em`,
|
||||
учёт `--rl-name`/`--rl-meta`), C2 — сохранение 44×44 px через `::before`,
|
||||
C3 — явный запрет трогать `isoRaisedOverlayHalfSize`/Flat/редакторы/
|
||||
`houseplan-space-card`. Каждый пункт проверяем независимо от двух других.
|
||||
- **AC1** (smoke, gap/name-height ratio на двух зумах, допуск 0,02) — численно
|
||||
привязан к уже исполненной таблице замеров, метод измерения (`gap =
|
||||
rlmetrics.top − rlname.bottom`) уже указан и воспроизводим.
|
||||
- **AC2** (существующая `raisedTargetsOwn44Pixels`/`owns44`) — заявление, что
|
||||
проверка уже нейтральна к источнику 44 px, подтверждено чтением
|
||||
`demo/smoke_isometric_contract.mjs:36-47`, а не принято как факт со слов
|
||||
автора.
|
||||
- **AC3** (мутант — вернуть `min-height: 44px`, красит AC1) — механизм
|
||||
find/replace для мутации CSS-строки уже используется в
|
||||
`scripts/mutation-registry.mjs` для аналогичных `min-height: 44px`-правил;
|
||||
подход реализуем.
|
||||
- «Принятые предположения» — не гипотетический дизайн с нуля, а копия рабочего
|
||||
паттерна `.oplock::before` (`src/styles/plan.styles.ts:541-551`), уже
|
||||
проверенного в проде для другого раскрытого элемента; технический риск
|
||||
реализации низкий.
|
||||
- Откат, i18n, миграция, перф — покрыты явным «нет»/«вернуть правило», без
|
||||
зазора для домысливания.
|
||||
- Открытых продуктовых вопросов нет — подтверждаю: контракт (C1: расстояние
|
||||
как во Flat; C2: сохранить цель 44 px) отвечает на оба вопроса, которые
|
||||
вообще мог бы задать владелец («что человек должен видеть» и «что не должно
|
||||
сломаться»), и делает это без домысливания — ожидание владельца прямо
|
||||
процитировано в теле issue («в 2.5D расстояние… не меняется — как в плоском
|
||||
виде»).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Гейты (`tsc --noEmit`, `npm test`, `npm run build`, `check-docs.mjs`, смоки,
|
||||
`golden:verify`, инварианты) — не прогонял: этап `spec`, ветки/коммитов для
|
||||
#665 нет, `git diff origin/dev...HEAD` пуст. Предмет код-ревью после
|
||||
реализации.
|
||||
- Не проверял `golden`-матрицу построчно на предмет того, есть ли там кадр
|
||||
именно 2.5D + `label_temp`/`label_light` включёнными на комнате с коротким
|
||||
именем (условие ТЗ «изменятся, если есть в матрице»); нашёл, что фикстуры с
|
||||
этими флагами существуют в `demo/golden/harness.mjs:621,657-660`, но не
|
||||
сверял, попадает ли конкретно 2.5D-кадр под порог видимого изменения — это
|
||||
разумно оставить `golden:verify` на код-ревью.
|
||||
- Не оспаривал числовой допуск AC1 (0,02) по существу — это техническая деталь
|
||||
реализации теста, не продуктовый контракт; при коде-ревью его стоит
|
||||
сверить на устойчивость (не флейкует ли на реальных шрифтах/DPR), но это не
|
||||
предмет ревью ТЗ.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Обязательные разделы полны, причина бага и оба смок-факта (AC1 таблица замеров,
|
||||
AC2 уже-нейтральный `owns44`, C3 уже-от-шрифта `isoRaisedOverlayHalfSize`)
|
||||
перепроверены по коду, а не приняты на слово автора — расхождений не найдено.
|
||||
Единственная находка (Low-1, неточная ссылка на несуществующую строку в
|
||||
`docs/ISOMETRIC.md`) не блокирует ни один AC и снимается без возврата автору.
|
||||
Открытых продуктовых вопросов нет, откат и release-артефакты названы. Готово к
|
||||
разработке.
|
||||
|
||||
Вердикт: зелёный · заход r1 · блокирующих циклов 0/2 · High: 0 · Medium: 0 → в задаче
|
||||
|
||||
---
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `942b9ede6789` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `4fd2ba833946aa89eac3733667f2d15b952b24c5`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 4fd2ba833946
|
||||
```
|
||||
- Тело issue: `8da493f662bb94963a92e6ca40c224f16235947efbd137e809e313833e01cc97`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user