mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,177 @@
|
||||
# CODE-REVIEW-666-r1
|
||||
|
||||
**Issue:** [#666](https://github.com/Matysh/houseplan-card/issues/666) — «Шапка: крестик × активного редактора — внутри подсветки вкладки»
|
||||
**Трек:** `trivial` (короткий) · **Заход:** r1 · **Блокирующих циклов:** 0 из 2
|
||||
**Материал:** `3bdd3c5163d05c1717bb719dda8f081d4c4ae5a7` (1 коммит поверх `dev` `42f437ca`), рабочая копия — на нём
|
||||
**Вердикт:** зелёный · High: 0 · Medium: 0
|
||||
|
||||
## Скоуп
|
||||
|
||||
Один коммит, класс A+B: `src/styles/chrome.styles.ts` (заливка активной вкладки
|
||||
растянута на `::after` поверх промежутка группы и слота ×), `demo/smoke_toolbar_stable_width.mjs`
|
||||
(пиксельные пробы), `scripts/mutation-registry.mjs` (мутант
|
||||
`toolbar-active-highlight-stops-at-tab`), `docs/CHANGELOG.md`/`.ru.md`,
|
||||
`docs/images/screenshots.json` (только отпечаток источника). DOM
|
||||
(`src/houseplan-card.ts`, `editorClose`) не тронут — контракт #647/#660
|
||||
(`slot.previousElementSibling === активная вкладка`) сохранён.
|
||||
|
||||
Три AC из тела issue:
|
||||
- **AC1** — заливка покрывает промежуток и слот ×, за слотом заливки нет.
|
||||
- **AC2** — геометрия (`.modetab`, `.editor-close-slot`, `.closex`, ширины) не изменилась.
|
||||
- **AC3** — цвет × на заливке = цвет текста вкладки.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
### Гейты, подтверждённые Validate на этом SHA (не перегонял)
|
||||
|
||||
Validate run [36239668263](https://github.com/Matysh/houseplan-card/actions/runs/36239668263) —
|
||||
`completed success`, `heavy=false` (проверил сам, `gh run view --json jobs` +
|
||||
лог job «Классификация изменённых файлов»: `heavy=false`, `mutants_requested=true`,
|
||||
ручной `workflow_dispatch`). Зелёные job на этом SHA:
|
||||
- «Фронтенд: типы, юниты, мутанты, синхрон бандла» — `tsc --noEmit`, `npm test`, `bundle-policy --verify`, `no-new-any`.
|
||||
- «Предполёт: документация, провенанс, процесс» — `check-docs.mjs` и трейлеры.
|
||||
- «Мутанты по диффу» (6/6 шардов) — включая новый мутант. В логе явно:
|
||||
`ok чистый прогон: node demo/smoke_toolbar_stable_width.mjs` (все 6 шардов) и
|
||||
`ok toolbar-active-highlight-stops-at-tab: заявленный тест покраснел на мутанте`
|
||||
(шард 3/6). Это исполнение, не заявление автора — я прочитал сырой лог сам.
|
||||
|
||||
**Важно:** job «Смоки в браузере» и «Golden-кадры» на этом прогоне —
|
||||
`skipped` (не `success`), потому что `heavy=false` (ручной запуск с
|
||||
`full=false`). Значит браузерный смок-набор и полный golden на этом SHA
|
||||
Validate **не прогонял** — это моя обязанность по инструкции ревью, и я её
|
||||
выполнил ниже.
|
||||
|
||||
### Гейты, прогнанные мной лично в этом ревью
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| Build (для смока/golden нужен свежий бандл) | `npm run build` | ok, 22.4s |
|
||||
| Синхрон копий стенда | `node scripts/bundle-sync.mjs` | ok |
|
||||
| Смок из AC (браузер, целевой) | `node demo/smoke_toolbar_stable_width.mjs` | `OK` — все ширины (1400…390) × все редакторы (plan/devices/decor): `highlightCoversGapAndSlot`, `highlightStopsAtSlot`, `crossReadsOnHighlight` — везде `true`; существующие геометрические проверки (`widthAndTabsStable`, `closeSlotLivesInsideModes`, `crossHitTarget`, `edgeClickCloses` и т.д.) — тоже `true` |
|
||||
| Связь диффа со смоками | `node scripts/smoke-select.mjs --base 42f437ca --head 3bdd3c51` | `НЕОПРЕДЕЛЁННОСТЬ` (0 смоков связано доказуемо — чисто CSS-дифф без символов, которые отслеживает выборка). Не разрешение ничего не гонять: смок, названный в AC (`smoke_toolbar_stable_width`), прогнан отдельно и напрямую (выше) |
|
||||
| Golden, полный набор (диф трогает рендер) | `npm run golden:verify` | 163 `passed`, **12 `different`** (список и разбор — ниже) |
|
||||
|
||||
**Полный список 12 `different`:** `panel-wide-plan-editor-dark-en`,
|
||||
`room-label-parity-plan-dark`, `room-label-parity-plan-light`,
|
||||
`geometry-plan-editor-dark`, `safe-resize-handles-clamp-light`,
|
||||
`safe-resize-handles-clamp-dark`, `tray-medium-group-en`,
|
||||
`tray-medium-selection-ru`, `furniture-variants-dark`,
|
||||
`furniture-variants-light`, `room-temperature-dialog-desktop-en`,
|
||||
`decor-color-popover-desktop-en`.
|
||||
|
||||
Я не поверил на слово диагностике автора (один кадр `geometry-plan-editor-dark`,
|
||||
1153 px) и разобрал **все 12** через `artifacts/golden/golden-report.json` плюс
|
||||
собственный скрипт на канале различий (маркер diff-пикселя — `rgb(255,0,180)`,
|
||||
объявлен в `demo/golden/run.mjs:374-376`; серые «неотличающиеся» пиксели идут с
|
||||
альфой 90 и не в счёт). Для каждого кадра построил bounding box маркированных
|
||||
пикселей:
|
||||
|
||||
| Сцена | differingPixels / ratio | bbox маркера |
|
||||
|---|---|---|
|
||||
| panel-wide-plan-editor-dark-en | 926 / 0.09% | x∈[456,491] y∈[75,105] |
|
||||
| geometry-plan-editor-dark | 926 / 0.09% | x∈[771,806] y∈[20,50] |
|
||||
| room-label-parity-plan-dark | 926 / 0.10% | x∈[124,159] y∈[72,102] |
|
||||
| room-label-parity-plan-light | 1380 / 0.15% | x∈[23,159] y∈[72,102] |
|
||||
| safe-resize-handles-clamp-light | 1380 / 0.13% | x∈[53,189] y∈[72,102] |
|
||||
| safe-resize-handles-clamp-dark | 926 / 0.09% | x∈[154,189] y∈[72,102] |
|
||||
| tray-medium-group-en | 725 / 0.12% | x∈[124,157] y∈[72,98] |
|
||||
| tray-medium-selection-ru | 725 / 0.12% | x∈[501,534] y∈[67,93] |
|
||||
| furniture-variants-dark | 725 / 0.10% | x∈[407,440] y∈[74,100] |
|
||||
| furniture-variants-light | 725 / 0.10% | x∈[501,534] y∈[72,98] |
|
||||
| room-temperature-dialog-desktop-en | 723 / 0.09% | x∈[125,157] y∈[74,100] |
|
||||
| decor-color-popover-desktop-en | 723 / 0.12% | x∈[408,440] y∈[72,98] |
|
||||
|
||||
Во всех 12 сценах различие — компактный прямоугольник ~35×30 px в полосе
|
||||
шапки (`y` 20…105), т.е. ровно зона активной вкладки + промежуток + ×, без
|
||||
единого пикселя за пределами шапки (не задета геометрия плана, комнат, диалогов,
|
||||
трея). Затем я визуально сверил `artifacts/golden/actual/panel-wide-plan-editor-dark-en.png`
|
||||
с `demo/golden/baselines/panel-wide-plan-editor-dark-en.png` (кроп шапки,
|
||||
масштаб ×4): на baseline «×» — серый, снаружи синей таблетки, отдельно от неё
|
||||
(в точности баг из тела issue); на actual — «×» белый, внутри одной сплошной
|
||||
скруглённой заливки без шва, а рамка фокуса теперь обводит вкладку целиком
|
||||
вместе с ×. Это прямое исполняемое доказательство AC1/C1/C3 — не пересказ
|
||||
смок-утверждений, а разница пикселей, которую я сам увидел.
|
||||
|
||||
Эталоны (`demo/golden/baselines/**`) **не обновлены** в этом коммите — это
|
||||
ожидаемо и объявлено в самом ТЗ («Release-артефакты: golden-кадры редакторов…
|
||||
изменятся… приёмка эталонов — по Linux CI на кандидате беты»), т.е. не
|
||||
пропуск, а сознательно отложенное действие класса D (PROCESS §12/§13:
|
||||
сгенерированное коммитится только релизным промоушеном или приёмкой с
|
||||
доказательством ревью на полном Linux-артефакте). Урок 2026-09-18
|
||||
(`docs/LESSONS.md`, CODE-REVIEW-598-r1 H1) требовал не ревьюить поверх
|
||||
непринятых эталонов, когда приёмка была частью самого AC той задачи — здесь
|
||||
приёмка explicitly не часть AC #666, поэтому 12 «different» не читаю как
|
||||
находку, а как подтверждённое, точное, ожидаемое отличие.
|
||||
|
||||
### AC → доказательство (перепроверено чтением)
|
||||
|
||||
| AC | Чем доказан | Чем краснеет |
|
||||
|---|---|---|
|
||||
| AC1 (заливка покрывает промежуток и слот) | `smoke_toolbar_stable_width` (прогнан мной) + golden 12/175 сцен, разница подтверждена как заголовочная зона | мутант `toolbar-active-highlight-stops-at-tab` (`right: 0` вместо `calc(-1*(sp-1+slot))`) — в CI **исполнено**: чистый прогон зелёный, на мутанте красный, лог прочитан |
|
||||
| AC2 (геометрия неизменна) | тот же смок: `widthAndTabsStable`, `closeSlotLivesInsideModes`, `modeToZoomGapMatchesSpec`, `crossHitTarget`, `edgeClickCloses` — все `true` на 7 ширинах | существующие мутанты `toolbar-close-*` не тронуты (новый мутант — отдельный `id`, найденная строка `--hp-editor-close-size: 24px;` в файле уникальна, поэтому её патч продолжает находить цель после переноса объявления в `.modes` — проверено чтением `mutation-registry.mjs:7632`) |
|
||||
| AC3 (цвет × = цвет текста вкладки) | `crossReadsOnHighlight: true` во всех комбинациях | **не мутировано отдельно** (автор сам это пишет в хендоффе). Я не стал патчить `src/**` (ревьюеру запрещено править продуктовый код), поэтому закрываю чтением: правило `.modetab.active + .editor-close-slot .closex { color: var(--text-primary-color, #fff); }` — единственный источник этого цвета для × на подсветке; без него действует базовое `.editor-close-slot .closex { color: var(--hp-muted); }`, а `--hp-muted` = `var(--secondary-text-color, #8aa0b3)` — заведомо другое значение, чем `--text-primary-color` (`#fff`) в любой теме HA, которую я нашёл в репозитории. Значит `crossColour === tabColour` красится при удалении правила. Отмечаю как «проверено чтением, не исполнением» — самостоятельного мутанта на этот конкретный AC нет, но брешь не приводит к находке: логика однозначна и не зависит от рантайма |
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Один источник размера слота (`--hp-editor-close-size`) — теперь объявлен
|
||||
один раз в `.modes`, читается и слотом, и заливкой; никакого дублирования
|
||||
числа (§8, «одно число — один источник»).
|
||||
- `isolation: isolate` на `.modes` ограничена этим маленьким контейнером
|
||||
(только вкладки + слот ×, без вложенных диалогов/оверлеев с собственным
|
||||
`z-index`) — проверил `grep z-index` по всем `src/styles/*.ts`: ничего
|
||||
внутри `.modes` не имеет своего позиционирования, конфликтов со стекингом
|
||||
нет.
|
||||
- Мобильная ветка (≤ 480 px) не тронута: `.modetab { display: none }` скрывает
|
||||
и сам таб, и его `::after` разом — подтверждено чтением существующего
|
||||
медиа-запроса (не менялся) и смоком (`w390_*` — все зелёные, включая
|
||||
`noCrossInsideTabs`).
|
||||
- View: в DOM ни один `.modetab` не получает класс `active`, когда
|
||||
`this._mode === 'view'` (map идёт только по `['plan','devices','decor']`,
|
||||
`view` там нет) — значит `.modetab.active::after` в принципе не существует
|
||||
в View; «заливки у слота нет» доказано структурой шаблона, а не только
|
||||
побочным эффектом стилей.
|
||||
- Терминология changelog («крестик активного редактора») совпадает с
|
||||
`docs/USER-GUIDE.ru.md:251` — не изобретена заново.
|
||||
- Трейлеры коммита: `Issue: #666`, `User-Visible: yes`, оба changelog
|
||||
(RU+EN) в том же коммите.
|
||||
- Бандл не закоммичен (класс D, ожидаемо согласно #657 — обычная задача не
|
||||
тащит `dist/**`).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- `pytest tests_backend` — диф не касается `custom_components/**/*.py`, не
|
||||
запускал.
|
||||
- `node scripts/model-invariants.mjs` — геометрия модели и ссылки на неё не
|
||||
менялись (чистый CSS), не запускал.
|
||||
- Performance-профили — не названы в AC, диф не связан с рендер-циклом
|
||||
устройств/геометрии.
|
||||
- Полный `demo/smoke_*.mjs` (276 файлов) — `smoke-select.mjs` не нашёл
|
||||
доказанной связи (НЕОПРЕДЕЛЁННОСТЬ), а AC называет ровно один смок; прогнал
|
||||
его напрямую. Остальные не гонял.
|
||||
- Мутацию AC3 «вручную» (временный патч `src/styles/chrome.styles.ts` и
|
||||
повторный прогон) — не стал вносить изменения в продуктовый код даже
|
||||
временно; закрыл AC3 чтением (см. таблицу выше).
|
||||
- Повторную приёмку golden-эталонов — по объявлению самого ТЗ это
|
||||
предрелизная (Linux CI/кандидат беты) обязанность, не гейт этого ревью.
|
||||
|
||||
## Итог
|
||||
|
||||
Три AC доказаны — два исполнением (мутант в CI: чистый+красный, смок лично
|
||||
перепрогнан), один чтением с однозначным обоснованием. Диф локальный,
|
||||
корректный, без побочных эффектов за пределами объявленной зоны — подтверждено
|
||||
не только смоком, но и пиксельным разбором всех 12 отличающихся golden-сцен и
|
||||
визуальным сравнением до/после. Находок нет.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/666-active-tab-close`, коммит `3bdd3c5163d0` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `a24c553fa7d582765d217596bbf20a519a61cf91`
|
||||
```
|
||||
git log --all --format='%H %T' | grep a24c553fa7d5
|
||||
```
|
||||
- Тело issue: `da355dc7a92e263f1b17a812ebde8f2605b9d7332b5cbda5538a299451b24573`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user