mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 13:18:58 +00:00
Merge remote-tracking branch 'origin/issue/220-space-tab-reorder' into dev
This commit is contained in:
Sergey Matyunin
File diff suppressed because one or more lines are too long
@@ -0,0 +1,224 @@
|
||||
// Issue #220: the order of the space tabs is changed by dragging one of them.
|
||||
//
|
||||
// The demo fixture has two spaces, which is all the panel needs to prove the
|
||||
// contract. What this smoke protects is not the animation but the three
|
||||
// promises around it: the
|
||||
// new order survives a save, an ordinary click still switches the space, and
|
||||
// nothing of the sort exists in View, where the same tabs are a touch-first
|
||||
// navigation control.
|
||||
import { launch, checkAll, finish } from './serve.mjs';
|
||||
const { page, browser } = await launch({ width: 1100, height: 900 }, 1);
|
||||
const res = await page.evaluate(async () => {
|
||||
const out = {};
|
||||
const c = window.__card;
|
||||
const sr = () => c.shadowRoot || c.renderRoot;
|
||||
const tabs = () => [...sr().querySelectorAll('[data-hp="space-tab"]')];
|
||||
const ids = () => tabs().map((tab) => tab.dataset.id);
|
||||
const settle = async () => {
|
||||
const started = performance.now();
|
||||
do { await new Promise((r) => requestAnimationFrame(r)); }
|
||||
while (c._modeTransitionBusy && performance.now() - started < 1500);
|
||||
await c.updateComplete;
|
||||
};
|
||||
|
||||
// A save must reach the server exactly once per drop, and carry the order.
|
||||
const writes = [];
|
||||
const realWrite = c._writeConfig.bind(c);
|
||||
c._writeConfig = () => {
|
||||
writes.push((c._serverCfg.spaces || []).map((space) => space.id));
|
||||
return Promise.resolve();
|
||||
};
|
||||
|
||||
const drag = async (fromId, toId, { travel = 40 } = {}) => {
|
||||
const from = tabs().find((tab) => tab.dataset.id === fromId);
|
||||
const to = tabs().find((tab) => tab.dataset.id === toId);
|
||||
const a = from.getBoundingClientRect();
|
||||
const b = to.getBoundingClientRect();
|
||||
const event = (type, x, y, target) => target.dispatchEvent(new PointerEvent(type, {
|
||||
pointerId: 7, pointerType: 'mouse', clientX: x, clientY: y, bubbles: true, composed: true,
|
||||
}));
|
||||
event('pointerdown', a.x + a.width / 2, a.y + a.height / 2, from);
|
||||
// one intermediate move on the source keeps the gesture honest: the drag
|
||||
// must begin from travel, not from merely touching a second tab
|
||||
event('pointermove', a.x + a.width / 2 + travel, a.y + a.height / 2, from);
|
||||
event('pointermove', b.x + b.width / 2, b.y + b.height / 2, to);
|
||||
event('pointerup', b.x + b.width / 2, b.y + b.height / 2, to);
|
||||
await settle();
|
||||
// The write is debounced (~500 ms). Waiting for it is the point: a smoke
|
||||
// that checks the panel and leaves proves the DOM, not the save.
|
||||
const deadline = performance.now() + 1500;
|
||||
const seen = writes.length;
|
||||
while (writes.length === seen && performance.now() < deadline) {
|
||||
await new Promise((r) => setTimeout(r, 25));
|
||||
}
|
||||
};
|
||||
|
||||
await settle();
|
||||
c._mode = 'plan';
|
||||
c.requestUpdate();
|
||||
await settle();
|
||||
|
||||
// A marker with neither an explicit space nor an area that names one is the
|
||||
// whole reason this feature has to be careful: today it renders in whichever
|
||||
// space sits first, so a reorder would hand it to another one. The fixture
|
||||
// has no such marker, so the smoke plants it — otherwise the guarantee would
|
||||
// be tested only as a pure function, never as applied behaviour.
|
||||
const firstBefore = c._model[0].id;
|
||||
c._serverCfg.markers = [
|
||||
...(c._serverCfg.markers || []),
|
||||
{ id: 'smoke-dangling', binding: 'virtual', name: 'dangling' },
|
||||
];
|
||||
const dangling = () => (c._serverCfg.markers || [])
|
||||
.find((marker) => marker.id === 'smoke-dangling');
|
||||
out.plantedMarkerStartsWithoutSpace = !dangling().space;
|
||||
|
||||
const before = ids();
|
||||
out.enoughTabsToReorder = before.length >= 2;
|
||||
out.reorderableInEditor = tabs()[0].hasAttribute('data-reorderable');
|
||||
|
||||
// --- AC1: the drop changes the order and asks for a save -------------------
|
||||
const moved = before[before.length - 1];
|
||||
const target = before[0];
|
||||
const active = c._space;
|
||||
await drag(moved, target);
|
||||
const after = ids();
|
||||
out.tabMovedToTheFront = after[0] === moved;
|
||||
out.otherTabsKeptOrder = JSON.stringify(after.filter((id) => id !== moved))
|
||||
=== JSON.stringify(before.filter((id) => id !== moved));
|
||||
out.orderReachedTheServer = writes.length >= 1;
|
||||
out.savedOrderMatchesPanel = JSON.stringify(writes[writes.length - 1])
|
||||
=== JSON.stringify(after);
|
||||
out.activeSpaceUnchanged = c._space === active;
|
||||
// AC3: the order-dependent marker keeps the space it had, written down.
|
||||
out.danglingMarkerPinnedToItsOldSpace = dangling().space === firstBefore;
|
||||
out.danglingMarkerDidNotFollowTheOrder = dangling().space !== ids()[0]
|
||||
|| firstBefore === ids()[0];
|
||||
|
||||
// --- AC7: the positional-floor warning is said once ------------------------
|
||||
out.warnedAboutPositionalFloor = typeof c._toast === 'string' && c._toast.length > 0;
|
||||
c._toast = '';
|
||||
const second = ids();
|
||||
await drag(second[second.length - 1], second[0]);
|
||||
out.secondDropAlsoReordered = ids()[0] === second[second.length - 1];
|
||||
out.warningNotRepeated = !c._toast;
|
||||
|
||||
// --- AC2: a click without travel still switches the space ------------------
|
||||
const other = ids().find((id) => id !== c._space);
|
||||
const writesBeforeClick = writes.length;
|
||||
await new Promise((r) => setTimeout(r, 700)); // let any pending debounce land
|
||||
const tab = tabs().find((t) => t.dataset.id === other);
|
||||
const rect = tab.getBoundingClientRect();
|
||||
const at = (type) => tab.dispatchEvent(new PointerEvent(type, {
|
||||
pointerId: 8, pointerType: 'mouse', composed: true,
|
||||
clientX: rect.x + rect.width / 2, clientY: rect.y + rect.height / 2, bubbles: true,
|
||||
}));
|
||||
at('pointerdown'); at('pointermove'); at('pointerup');
|
||||
tab.click();
|
||||
await settle();
|
||||
out.clickStillSwitchesSpace = c._space === other;
|
||||
await new Promise((r) => setTimeout(r, 700));
|
||||
out.clickDidNotReorder = writes.length === writesBeforeClick;
|
||||
|
||||
// --- AC5: touch and View never start a drag -------------------------------
|
||||
const touchOrder = ids();
|
||||
const src = tabs()[tabs().length - 1];
|
||||
const dst = tabs()[0];
|
||||
const ra = src.getBoundingClientRect();
|
||||
const rb = dst.getBoundingClientRect();
|
||||
const touch = (type, x, y, target) => target.dispatchEvent(new PointerEvent(type, {
|
||||
pointerId: 9, pointerType: 'touch', clientX: x, clientY: y, bubbles: true, composed: true,
|
||||
}));
|
||||
touch('pointerdown', ra.x + ra.width / 2, ra.y + ra.height / 2, src);
|
||||
touch('pointermove', rb.x + rb.width / 2, rb.y + rb.height / 2, dst);
|
||||
touch('pointerup', rb.x + rb.width / 2, rb.y + rb.height / 2, dst);
|
||||
await settle();
|
||||
out.touchDidNotReorder = JSON.stringify(ids()) === JSON.stringify(touchOrder);
|
||||
|
||||
c._mode = 'view';
|
||||
c.requestUpdate();
|
||||
await settle();
|
||||
out.notReorderableInView = !tabs()[0].hasAttribute('data-reorderable');
|
||||
const viewOrder = ids();
|
||||
await drag(viewOrder[viewOrder.length - 1], viewOrder[0]);
|
||||
out.viewDidNotReorder = JSON.stringify(ids()) === JSON.stringify(viewOrder);
|
||||
|
||||
// --- review r1 M1: the mouse is released away from the panel ---------------
|
||||
//
|
||||
// A horizontal drag that ends a few pixels below the tabs is ordinary hand
|
||||
// imprecision. Without pointer capture no tab ever sees the release, the
|
||||
// gesture stays stuck with moved:true, and the next click is swallowed by
|
||||
// _tabClick — the panel simply stops switching spaces.
|
||||
c._mode = 'plan';
|
||||
c.requestUpdate();
|
||||
await settle();
|
||||
const strayTabs = tabs();
|
||||
const strayFrom = strayTabs[strayTabs.length - 1];
|
||||
const strayRect = strayFrom.getBoundingClientRect();
|
||||
const stray = (type, x, y) => strayFrom.dispatchEvent(new PointerEvent(type, {
|
||||
pointerId: 11, pointerType: 'mouse', clientX: x, clientY: y, bubbles: true, composed: true,
|
||||
}));
|
||||
stray('pointerdown', strayRect.x + strayRect.width / 2, strayRect.y + strayRect.height / 2);
|
||||
stray('pointermove', strayRect.x + strayRect.width / 2 + 40, strayRect.y + strayRect.height / 2);
|
||||
// released far below the panel, where no tab lives
|
||||
// released on the stage, not on a tab: without pointer capture no tab
|
||||
// handler ever runs and the gesture stays stuck
|
||||
(sr().querySelector('.stage') || document.body).dispatchEvent(new PointerEvent('pointerup', {
|
||||
pointerId: 11, pointerType: 'mouse', composed: true,
|
||||
clientX: strayRect.x + 400, clientY: strayRect.y + 400, bubbles: true,
|
||||
}));
|
||||
await settle();
|
||||
out.strayReleaseEndedTheDrag = c._tabDrag === null;
|
||||
const strayTarget = ids().find((id) => id !== c._space);
|
||||
const strayNext = tabs().find((tab) => tab.dataset.id === strayTarget);
|
||||
strayNext.dispatchEvent(new PointerEvent('pointerdown', {
|
||||
pointerId: 12, pointerType: 'mouse', bubbles: true, composed: true,
|
||||
}));
|
||||
strayNext.dispatchEvent(new PointerEvent('pointerup', {
|
||||
pointerId: 12, pointerType: 'mouse', bubbles: true, composed: true,
|
||||
}));
|
||||
strayNext.click();
|
||||
await settle();
|
||||
out.panelStillSwitchesAfterStrayRelease = c._space === strayTarget;
|
||||
|
||||
// --- review r2/r3 F1: the card is destroyed mid-drag -----------------------
|
||||
//
|
||||
// Lovelace rebuilds its tree, or the user leaves the view with the button
|
||||
// still down. The window listeners the gesture installed must not outlive
|
||||
// the card: they hold the instance alive and would let an invisible card
|
||||
// write its order on the next pointerup anywhere on the page.
|
||||
c._mode = 'plan';
|
||||
c.requestUpdate();
|
||||
await settle();
|
||||
const orderBeforeDetach = ids();
|
||||
const detachTabs = tabs();
|
||||
const held = detachTabs[detachTabs.length - 1];
|
||||
const heldRect = held.getBoundingClientRect();
|
||||
const heldEvent = (type, x, y) => held.dispatchEvent(new PointerEvent(type, {
|
||||
pointerId: 13, pointerType: 'mouse', clientX: x, clientY: y,
|
||||
bubbles: true, composed: true,
|
||||
}));
|
||||
heldEvent('pointerdown', heldRect.x + heldRect.width / 2, heldRect.y + heldRect.height / 2);
|
||||
heldEvent('pointermove', heldRect.x + heldRect.width / 2 + 40, heldRect.y + heldRect.height / 2);
|
||||
out.dragWasActiveBeforeDetach = c._tabDrag !== null && c._tabDrag.moved === true;
|
||||
|
||||
const parent = c.parentNode;
|
||||
const next = c.nextSibling;
|
||||
const writesBeforeDetach = writes.length;
|
||||
c.remove();
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
out.detachEndedTheDrag = c._tabDrag === null;
|
||||
// the release the detached card must no longer hear
|
||||
window.dispatchEvent(new PointerEvent('pointerup', {
|
||||
pointerId: 13, pointerType: 'mouse', bubbles: true, composed: true,
|
||||
}));
|
||||
await new Promise((r) => setTimeout(r, 700));
|
||||
out.detachedCardDidNotWrite = writes.length === writesBeforeDetach;
|
||||
parent.insertBefore(c, next);
|
||||
await settle();
|
||||
out.orderSurvivedDetach = JSON.stringify(ids()) === JSON.stringify(orderBeforeDetach);
|
||||
|
||||
c._writeConfig = realWrite;
|
||||
return out;
|
||||
});
|
||||
checkAll(res);
|
||||
await finish(browser, res);
|
||||
File diff suppressed because one or more lines are too long
Vendored
+51
-42
File diff suppressed because one or more lines are too long
@@ -2,6 +2,11 @@
|
||||
|
||||
## Unreleased
|
||||
|
||||
- Space tabs can be reordered: grab a tab with the mouse in any editor mode and
|
||||
drop it where it belongs. The order is saved, and swipe and carousel
|
||||
navigation follow it. Markers stay exactly where they were
|
||||
([#220](https://github.com/Matysh/houseplan-card/issues/220)).
|
||||
|
||||
- “Optimize plans” now removes microscopic floating-point noise from stored
|
||||
grid coordinates even when nothing visibly moves. Its preview separately
|
||||
reports updated spaces and cleaned coordinate values, and a second run is an
|
||||
|
||||
@@ -8,6 +8,11 @@
|
||||
|
||||
## Не выпущено
|
||||
|
||||
- Порядок вкладок пространств теперь можно менять: в режиме редактора возьмите
|
||||
вкладку мышью и перетащите на нужное место. Порядок сохраняется, свайп и
|
||||
карусель следуют ему. Устройства при этом остаются на своих местах
|
||||
([#220](https://github.com/Matysh/houseplan-card/issues/220)).
|
||||
|
||||
- «Оптимизировать планы» теперь устраняет микроскопический floating-point шум
|
||||
сохранённых координат сетки, даже если визуально ничего не сдвигается. В
|
||||
предпросмотре отдельно показаны обновлённые пространства и очищенные
|
||||
|
||||
@@ -235,6 +235,23 @@ The plan image keeps its proportions initially. Background can later move,
|
||||
scale or rotate it. Detaching a plan never deletes its server file; deletion
|
||||
requires an explicit user action.
|
||||
|
||||
### Tab order
|
||||
|
||||
Space tabs follow the order in which the spaces were created, and that order can
|
||||
be changed: in any editor mode, grab a tab with the mouse and drag it to a new
|
||||
position. The new order is saved immediately and applies everywhere — the tabs,
|
||||
the kiosk swipe between floors and the carousel arrows.
|
||||
|
||||
Dragging works **with a mouse and in the editors only**. In ordinary View and on
|
||||
touch screens a tab still does one thing: it switches the space. There it is the
|
||||
primary way to navigate, and a gesture must not compete with a plain tap. Order
|
||||
is changed on a computer, like the rest of the plan work.
|
||||
|
||||
If a card anywhere pins its floor **by number** (`floor: 0`), remember that the
|
||||
number means a position: after a reorder such a card shows a different floor.
|
||||
The card warns about this once. Pin the floor by space id instead of a number to
|
||||
avoid it entirely.
|
||||
|
||||
### Display settings
|
||||
|
||||
A space can show/hide room borders, names, LQI, Background and openings. It can
|
||||
|
||||
@@ -269,6 +269,24 @@ desktop: для точного рисования, Resize, модификато
|
||||
них, последующая смена источника больше не сбрасывает ни один выбор. В мастере
|
||||
этажей каждый следующий этаж получает собственные чистые defaults.
|
||||
|
||||
### Порядок вкладок
|
||||
|
||||
Вкладки пространств стоят в том порядке, в котором пространства заведены, и
|
||||
этот порядок можно изменить: в любом режиме редактора возьмите вкладку мышью и
|
||||
перетащите на новое место. Порядок сохраняется сразу и действует везде —
|
||||
вкладки, свайп между этажами в киоске, стрелки карусели.
|
||||
|
||||
Перетаскивание работает **только мышью и только в редакторах**. В обычном
|
||||
просмотре и на сенсорных экранах вкладка по-прежнему только переключает
|
||||
пространство: там это основной способ навигации, и жест не должен мешать
|
||||
обычному нажатию. Порядок меняется на компьютере — как и остальная работа с
|
||||
планом.
|
||||
|
||||
Если где-то карточка закреплена за этажом **по номеру** (`floor: 0`), помните,
|
||||
что номер означает позицию: после перестановки такая карточка покажет другой
|
||||
этаж. Карточка предупредит об этом один раз. Чтобы этого не случалось, задавайте
|
||||
этаж идентификатором пространства, а не номером.
|
||||
|
||||
### Настройки пространства
|
||||
|
||||
| Раздел | Настройка | Результат |
|
||||
|
||||
@@ -0,0 +1,277 @@
|
||||
# Код-ревью issue #220 — порядок вкладок пространств перетаскиванием
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/220
|
||||
- **ТЗ:** `docs/specs/220-space-tab-reorder.md`, зелёное ревью r3
|
||||
(`docs/reviews/SPEC-REVIEW-220-r3.md`), заход исчерпан на 2/4.
|
||||
- **Заход код-ревью:** r1/4 (первый цикл код-ревью; счётчик отдельный от
|
||||
ревью ТЗ, §10.4 PROCESS.md).
|
||||
- **Диапазон:** ветка `issue/220-space-tab-reorder`, коммит `8369c0e`
|
||||
(единственный коммит реализации поверх `e57e1d9`).
|
||||
- **Ревьюер:** свежая сессия, без контекста реализации.
|
||||
|
||||
## 1. Скоуп проверки
|
||||
|
||||
Диапазон — весь коммит `8369c0e` (первый код-ревью для этой задачи, полный
|
||||
разбор, §2.10 не применяется): `src/space-order.ts` (новый), правки
|
||||
`src/houseplan-card.ts` (обработчики drag, `_commitTabOrder`), `src/styles.ts`,
|
||||
i18n en/ru, `scripts/mutation-gate.mjs`, `test/space-order.test.mjs`,
|
||||
`demo/smoke_space_tab_reorder.mjs`, оба changelog, оба USER-GUIDE, три копии
|
||||
бандла.
|
||||
|
||||
## 2. Как проверялось
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| Typecheck | `npx tsc --noEmit` | чисто |
|
||||
| Unit | `npm test` | 982/982, 0 fail |
|
||||
| Build + сверка копий | `npm run build && cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js && cmp dist/houseplan-card.js demo/srv/assets/houseplan-card.js` | пересобран, все три копии побайтово совпадают, `git status` после сборки чист |
|
||||
| Смок AC1/2/4/5/7 | `node demo/smoke_space_tab_reorder.mjs` | OK, все 18 проверок зелёные |
|
||||
| Смок соседнего контракта (#210) | `node demo/smoke_fixed_floor.mjs` | OK |
|
||||
| Смок соседнего контракта (размещение маркеров) | `node demo/smoke_subarea.mjs` | OK |
|
||||
| Mutation-gate, дешёвая проверка якорей | `node scripts/mutation-gate.mjs --check` | все патчи, включая 4 новых, находят свой якорь в исходниках |
|
||||
| **Прямой вызов production-функций** (не гейт, целевая проверка находки H1) | см. §4.1 | воспроизводит расхождение |
|
||||
|
||||
**Не прогонялось и почему:** `golden:verify` — задача не трогает визуал в
|
||||
состоянии покоя (панель вкладок вне матрицы, §15 ТЗ), diff подтверждает: правки
|
||||
только в `.tab`/`.dragging`/`.droptarget` стилях активного взаимодействия;
|
||||
`pytest tests_backend` — ни один файл `custom_components/**/*.py` не тронут;
|
||||
полный (дорогой) прогон `mutation-gate` с пересборкой на каждого мутанта — это
|
||||
предрелizный гейт, не гейт ревью (§8 PROCESS.md); остальные 124 браузерных
|
||||
смока — diff не задевает их поверхности (canvas-инструменты, устройства,
|
||||
партиции и т.д.), только панель вкладок и её ближайших соседей, которые
|
||||
прогнаны выше.
|
||||
|
||||
## 3. Находки
|
||||
|
||||
### H1 (High) — материализация неверно определяет «зависимые от порядка» маркеры: обычный маркер устройства с площадью из реестра HA получает чужой `space`
|
||||
|
||||
**Что не так.** `markersNeedingPlacement` (`src/space-order.ts:101-119`) решает,
|
||||
какому маркеру нужно материализовать `firstSpaceId`, глядя только на
|
||||
персистентные поля самого маркера — `marker.area` и `marker.space`:
|
||||
|
||||
```ts
|
||||
const area = typeof marker.area === 'string' ? marker.area : '';
|
||||
if (area && areaToSpace[area]) continue;
|
||||
out.push({ id, space: firstSpaceId });
|
||||
```
|
||||
|
||||
Но реальное размещение маркера, привязанного к HA-устройству или сущности
|
||||
(`binding: 'device:...'` / `'entity:...'`), решает не только `marker.area` —
|
||||
`resolveExplicitMarkerPlacement` (`src/devices.ts:1045-1065`) в первую очередь
|
||||
берёт **область из реестра HA** (`dev?.area_id` / `reg?.area_id`), и только
|
||||
если её нет — откатывается к `marker.area`, `marker.space`, затем
|
||||
`firstSpaceId`:
|
||||
|
||||
```ts
|
||||
const area = marker.area || registryArea || '';
|
||||
return { area, space: (area && areaToSpace[area]) || marker.space || firstSpaceId };
|
||||
```
|
||||
|
||||
**Ровно так и создаются обычные маркеры.** `_markerDraft`
|
||||
(`houseplan-card.ts:18868-18913`) и её аналог для существующей формы
|
||||
(`houseplan-card.ts:13305-13344`) пишут `marker.area`/`marker.space` **только
|
||||
если** `d.binding === 'virtual' || d.room` — то есть только для виртуальных
|
||||
маркеров или маркеров, которым вручную выбрана комната. Любой маркер,
|
||||
созданный обычным способом «привязать существующее HA-устройство» (переименовать,
|
||||
сменить иконку, добавить контролы, спрятать с плана) без ручного выбора
|
||||
комнаты — **самый частый путь в реальной инсталляции** (SCOPE.md: 20–200
|
||||
устройств, в основном автообнаруженных) — сохраняется **без** `area` и без
|
||||
`space` вовсе. Такой маркер сегодня прекрасно резолвится через область своего
|
||||
HA-устройства и от порядка `spaces` не зависит.
|
||||
|
||||
`markersNeedingPlacement` не видит `registryArea` — у неё в сигнатуре нет ни
|
||||
самого устройства, ни его области, только поля маркера. Поэтому она
|
||||
классифицирует **любой такой маркер** как «зависящий от `firstSpaceId`» и
|
||||
`_commitTabOrder` (`houseplan-card.ts:1300-1330`) прямо в транзакции
|
||||
перестановки дописывает ему `marker.space = <старое первое пространство>` —
|
||||
хотя AC3/§8.3 ТЗ прямо требуют, чтобы такие маркеры «остались побитово
|
||||
прежними».
|
||||
|
||||
**Воспроизведение** (прямой вызов той же production-сборки, без браузера):
|
||||
|
||||
```js
|
||||
import { buildDevices } from './test-build/devices.js';
|
||||
import { markersNeedingPlacement } from './test-build/space-order.js';
|
||||
|
||||
const hass = { devices: { boiler: {
|
||||
id: 'boiler', name: 'Boiler', model: 'X', area_id: 'attic',
|
||||
identifiers: [['demo', 'boiler']], entry_type: null, via_device_id: null,
|
||||
} }, entities: {}, states: {}, areas: {} };
|
||||
const marker = { id: 'boiler-marker', binding: 'device:boiler' }; // ни area, ни space
|
||||
const ctx = { hass, areaToSpace: { attic: 'f2' }, markers: [marker], settings: {},
|
||||
excluded: new Set(), showAll: false, firstSpaceId: 'f1', loc: () => '' };
|
||||
|
||||
buildDevices(ctx).find((d) => d.id === 'boiler-marker').space;
|
||||
// → 'f2' — маркер сегодня корректно и стабильно резолвится через область HA,
|
||||
// пространство 'f2' не имеет отношения к firstSpaceId
|
||||
|
||||
markersNeedingPlacement([marker], { attic: 'f2' }, 'f1');
|
||||
// → [{ id: 'boiler-marker', space: 'f1' }]
|
||||
// _commitTabOrder на основании этого результата запишет marker.space = 'f1' —
|
||||
// пространство, в котором маркер никогда не находился.
|
||||
```
|
||||
|
||||
Запущено против собранного `test-build` этой самой ветки (`npx tsc -p
|
||||
tsconfig.test.json`) — расхождение воспроизводится, не гипотетическое.
|
||||
|
||||
**Почему это не ловится существующими тестами.** Юнит `test/space-order.test.mjs`
|
||||
(«only order-dependent markers are pinned») и смок (`smoke-dangling`, marker с
|
||||
`binding: 'virtual'`) проверяют только маркеры, у которых «отсутствие
|
||||
зависимости от порядка» и так видно по их собственным полям (`area: 'kitchen'`,
|
||||
`space: 'f2'`). Ни один тест не заводит обычный `device:`/`entity:`-маркер без
|
||||
area/space, чьё реальное размещение решает область его HA-устройства —
|
||||
единственный случай, где расхождение проявляется. Мутационный гейт тоже не
|
||||
покрывает эту ветку: из пяти записей §14 ТЗ реализованы четыре (см. M2 ниже),
|
||||
причём именно недостающая `materialization-touches-bound-markers`
|
||||
концептуально ближе всего к этой находке.
|
||||
|
||||
**Практическое следствие.** Прямо сейчас маркер не переезжает визуально: в
|
||||
`resolveExplicitMarkerPlacement` `area && areaToSpace[area]` стоит раньше
|
||||
`marker.space` в приоритете, поэтому испорченный `marker.space` временно
|
||||
маскируется правильной резолюцией через область. Но конфиг тихо получает
|
||||
недостоверное персистентное поле на **каждом** обычном маркере устройства без
|
||||
ручной комнаты при **каждой** перестановке вкладок: как только HA-область этого
|
||||
устройства когда-либо станет недоступна (переназначение area в HA, замена
|
||||
устройства, повторная привязка сущности) — `area && areaToSpace[area]`
|
||||
перестанет резолвиться, и маркер молча прыгнет в то самое `firstSpaceId`,
|
||||
которое было актуально на момент случайной перестановки вкладок месяцы назад,
|
||||
без какой-либо связи с текущим действием пользователя. Это ровно тот
|
||||
сценарий («маркеры уезжают»), который §8.3 ТЗ называет главным риском задачи —
|
||||
только отложенный во времени и оттого более коварный.
|
||||
|
||||
**Это находка в скоупе задачи** (§8.3/AC3 — центральный контракт этой самой
|
||||
задачи), блокирует зелёный вердикт.
|
||||
|
||||
### M1 (Medium, в скоупе) — перетаскивание вкладки не захватывает указатель: отпускание за пределами панели оставляет «зависшее» состояние drag
|
||||
|
||||
`_tabPointerDown` (`houseplan-card.ts:1259-1272`) не вызывает
|
||||
`setPointerCapture`/`capturePointer`, в отличие от **всех** остальных
|
||||
pointer-based drag-жестов в этом файле — resize краёв/углов
|
||||
(`_rszEdgeDown`/`_rszCornerDown`, `capturePointer(ev)`), компас
|
||||
(`setPointerCapture` напрямую, `:14271`), инструменты декора (`:9598`, `:9608`,
|
||||
`:14944` и другие) — все идут через явный захват указателя или через
|
||||
специально написанный для этого хелпер:
|
||||
|
||||
```ts
|
||||
// houseplan-card.ts:667-673
|
||||
const capturePointer = (ev: PointerEvent): void => {
|
||||
try { (ev.target as Element | null)?.setPointerCapture?.(ev.pointerId); }
|
||||
catch { /* an inactive pointerId must never kill the drag */ }
|
||||
};
|
||||
```
|
||||
|
||||
Без захвата `pointermove`/`pointerup` у `_tabPointerMove`/`_tabPointerUp`
|
||||
привязаны к каждому `.tab`-элементу и получают события только пока указатель
|
||||
физически находится над этим элементом. Если пользователь, естественно
|
||||
двигая руку по горизонтали, слегка уводит курсор за пределы строки вкладок
|
||||
(вниз, на «+», в промежуток между вкладками) и там отпускает кнопку — ни
|
||||
`pointerup`, ни `pointercancel` не срабатывают ни на одном `.tab`. Результат:
|
||||
|
||||
1. `_tabDrag` остаётся с `moved: true` навсегда — вкладка-источник продолжает
|
||||
показывать класс `dragging` (`opacity: 0.55`), последняя наведённая —
|
||||
`droptarget` (синяя рамка), пока не начнётся и не завершится следующий
|
||||
полноценный drag на каком-то `.tab`.
|
||||
2. `_tabClick` (`houseplan-card.ts:1290-1293`) читает `this._tabDrag?.moved`,
|
||||
чтобы отличить клик от драга. Пока `_tabDrag` в таком «зависшем» виде,
|
||||
следующий тап **другим** типом указателя (например, тач на гибридном
|
||||
тач-экране ноутбука — актуально: `docs/TOUCH-SUPPORT.md` прямо
|
||||
рассматривает такие устройства) не пройдёт через `_tabPointerDown`
|
||||
(`canStartTabDrag` вернёт `false` для `pointerType !== 'mouse'`, ранний
|
||||
`return`), состояние не сбросится, и `_tabClick` молча проглотит переключение
|
||||
пространства.
|
||||
|
||||
Спек §8.1 требует «при отпускании вне панели порядок не меняется» — это
|
||||
формально верно (commit не вызывается), но не покрывает визуальный/интерактивный
|
||||
артефакт, который отпускание вне панели создаёт. Смок не ловит это: `drag()` в
|
||||
`demo/smoke_space_tab_reorder.mjs` всегда завершает `pointerup` на целевой
|
||||
вкладке (`to`), сценарий «отпустить вне панели» не воспроизведён ни разу.
|
||||
|
||||
**Почин.** Один вызов `capturePointer(event)` (используя уже существующий
|
||||
хелпер `houseplan-card.ts:667`) в начале `_tabPointerDown`, как это уже сделано
|
||||
для каждого другого перетаскивания в файле.
|
||||
|
||||
### M2 (Medium, в скоупе) — мутационный гейт реализован не полностью относительно принятого §14 ТЗ
|
||||
|
||||
Спека (`docs/specs/220-space-tab-reorder.md` §14) фиксирует пять записей
|
||||
мутационного гейта, в том числе:
|
||||
|
||||
```
|
||||
| materialization-touches-bound-markers | материализовать space и у маркеров с area | юнит AC3 |
|
||||
```
|
||||
|
||||
В `scripts/mutation-gate.mjs` добавлены только четыре: `tab-reorder-not-persisted`,
|
||||
`reorder-skips-materialization`, `tab-reorder-eats-click`,
|
||||
`tab-reorder-ignores-pointer-type`. Хендофф-комментарий в issue тоже называет
|
||||
«четыре записи, все прогнаны» — без пометки, что пятая из принятого ТЗ
|
||||
пропущена, и без объяснения почему. Раз AC5.md §14 — часть зелёного ТЗ (не
|
||||
«принятое предположение»), тихое сокращение состава гейта — отклонение от DoR,
|
||||
а не право исполнителя.
|
||||
|
||||
Показательно, что именно эта недостающая проверка концептуально ближе всего к
|
||||
H1: она должна была утверждать «материализация не трогает маркеры с area» —
|
||||
и её реализация (более широкая, чем нынешний юнит-тест, который проверяет
|
||||
только буквальное поле `marker.area`) с большей вероятностью поймала бы
|
||||
находку выше на этапе реализации.
|
||||
|
||||
**Не отдельный issue** — это Medium в скоупе текущей задачи (§14 — её
|
||||
собственный принятый план тестов), правится в этой же ветке добавлением
|
||||
пятой записи мутационного гейта с юнит-тестом, покрывающим маркер, чьё
|
||||
размещение решает область HA-устройства, а не собственное поле `area`.
|
||||
|
||||
## 4. AC — чем доказано (сверх заявленного автором)
|
||||
|
||||
| AC | Проверка ревьюера | Вывод |
|
||||
|---|---|---|
|
||||
| AC1 | смок прогнан повторно, зелёный; код `_commitTabOrder` + `reorderSpaceIds`/`applySpaceOrder` прочитан | доказано |
|
||||
| AC2 | смок повторно; unit `passedDragThreshold`/`canStartTabDrag` прочитаны | доказано |
|
||||
| AC3 | смок повторно зелёный **для сценария из смока**, но разбор по коду нашёл незакрытый случай — **H1** | **не доказано полностью**, находка блокирует |
|
||||
| AC4 | `swipeTarget` берёт `this._model.map(m => m.id)` (`:5825`), `_model` пересчитывается по `_cfgEpoch`, который `_saveConfig()` инкрементирует синхронно до дебаунса записи — прочитано, логика верна | проверено чтением, корректно |
|
||||
| AC5 | смок повторно; `canStartTabDrag` разобран построчно, семь юнит-веток на отрицание | доказано |
|
||||
| AC6 | `_commitTabOrder` → `_saveConfig()` → `_saveConfigDebounced` → `_writeConfig()` — тот же путь конфликта, что у остальных правок (`toast.conflict`, `_reloadConfigOnly`), своего кода обработки не добавлено — прочитано | проверено чтением, не исполнением (согласен с автором) |
|
||||
| AC7 | смок повторно зелёный | доказано |
|
||||
| AC8 | md5 всех трёх копий бандла совпадает после независимой пересборки; changelog/USER-GUIDE поменяны в обоих языках | доказано — **с оговоркой**: формулировка «Markers stay exactly where they were» станет точной только после фикса H1 |
|
||||
|
||||
## 5. Что проверено и корректно
|
||||
|
||||
- Порог 4 px, разделение клика и драга, вставка перед целевой вкладкой (а не
|
||||
обмен местами) — соответствует §17 ТЗ.
|
||||
- `.tabadd` («+») не участвует: у неё нет `pointerdown`/`pointermove`/`pointerup`
|
||||
обработчиков, поэтому она не может стать ни источником, ни целью — код
|
||||
подтверждает намерение спеки без отдельного смок-кейса.
|
||||
- Одно пространство — `canStartTabDrag` возвращает `false` при `spaceCount <= 1`
|
||||
(юнит-тест есть).
|
||||
- `_hasFixedFloor` корректно блокирует drag (панель с фиксированным этажом
|
||||
показывает одну вкладку — двойная защита, не только через `spaceCount`).
|
||||
- View/киоск/тач исключены на уровне чистой функции и подтверждены смоком.
|
||||
- Атомарность записи (порядок + материализация одним `cfg`-мутированием перед
|
||||
единственным `_saveConfig()`) соблюдена — ровно то, что требовал норматив
|
||||
§17.3 ТЗ и на чём настоял ревью ТЗ r2/r3 (M3).
|
||||
- Тост «once per session» — `_tabOrderWarned` не персистится, инстанс-поле,
|
||||
сбрасывается только при пересоздании карточки — соответствует формулировке
|
||||
АС7 «за сессию».
|
||||
- Три копии бандла побайтово идентичны после независимой пересборки ревьюером.
|
||||
|
||||
## 6. Чего не проверял
|
||||
|
||||
- Полный (дорогой, предрелизный) прогон `mutation-gate.mjs` с реальной
|
||||
пересборкой на каждый мутант — вне объёма код-ревью (§8 PROCESS.md), прогнан
|
||||
только `--check`.
|
||||
- `golden:verify` — визуал панели вкладок не в матрице, изменений в покое нет;
|
||||
не прогонялся сознательно (см. §2).
|
||||
- `pytest tests_backend` — Python не тронут.
|
||||
- Остальные ~124 браузерных смока за пределами панели вкладок и её
|
||||
непосредственных соседей (#210, привязка маркеров) — diff их поверхностей не
|
||||
касается.
|
||||
- Ручного тестирования в браузере (drag мышью вживую) не было — по регламенту
|
||||
цикла (нет фазы ручного тестирования); H1 и M1 доказаны разбором по коду и
|
||||
прямым вызовом продакшен-функций соответственно, а не интерактивной сессией.
|
||||
|
||||
## 7. Итог
|
||||
|
||||
Один High (H1) — блокирует. Два Medium (M1, M2) — в скоупе задачи, правятся в
|
||||
этой же ветке. AC3 не доказан полностью: смок проверяет только тот подкласс
|
||||
«зависимых от порядка» маркеров, который сам код классифицирует явными
|
||||
полями, и не покрывает самый частый в реальных конфигурациях случай —
|
||||
маркер устройства, чьё размещение решает область HA, а не поле маркера.
|
||||
|
||||
Возврат автору: `S6-in-progress`.
|
||||
@@ -0,0 +1,213 @@
|
||||
# SPEC-REVIEW-220-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/220
|
||||
- **ТЗ:** `docs/specs/220-space-tab-reorder.md` (коммит `0fd2331`)
|
||||
- **Ревьюер:** Claude (роль «ревьюер ТЗ», PROCESS.md §2.4)
|
||||
- **Цикл:** r1/2 (обычный трек — лимит 4, но фиксирую фактический номер цикла)
|
||||
- **Вердикт:** жёлтый · High: 0 · Medium: 2 (обе в скоупе задачи)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Полный разбор — первый цикл. Проверено:
|
||||
|
||||
1. `docs/SCOPE.md` — соответствие job J6, персона «администратор плана».
|
||||
2. `AGENTS.md`, `PROCESS.md` §2.4, §2.5, §7.1 — обязательные разделы ТЗ,
|
||||
критерии DoR, формат вердикта.
|
||||
3. Тело issue #220 и оба комментария (аналитика → `S3-spec`; хендофф ТЗ →
|
||||
`S4-spec-review`).
|
||||
4. `docs/USER-GUIDE.ru.md` — терминология «пространство», «вкладка».
|
||||
5. `docs/TOUCH-SUPPORT.md` — канонический документ для editor-only фичи с
|
||||
явным touch-решением владельца.
|
||||
6. `docs/CONFIG-COMPATIBILITY.md` — паттерн описания новых/условных полей.
|
||||
7. Код на `dev` (`src/houseplan-card.ts`, `src/logic.ts`, `src/devices.ts`) —
|
||||
сверка всех фактических утверждений ТЗ (номера строк, сигнатуры,
|
||||
поведение).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
- Построчное чтение ТЗ (`docs/specs/220-space-tab-reorder.md`, 236 строк)
|
||||
против требуемых разделов §7.1 — все присутствуют: сценарий, что человек
|
||||
увидит до/после, подтверждённая причина, скоуп/не-скоуп, контракт поведения,
|
||||
данные/i18n/a11y/security, performance, риски, AC1…AC8 с доказательством,
|
||||
план автотестов, мутационный гейт, release-артефакты, откат, блок
|
||||
принятых предположений.
|
||||
- Каждая техническая ссылка на код (`houseplan-card.ts:15717/6692/3350/4273/
|
||||
1165`, `devices.ts:1058/1063/1249`, `logic.ts:1870-1879`) сверена с текущим
|
||||
`dev` через `grep`/чтение файлов — расхождение только в точных номерах строк
|
||||
(код на `dev` сдвинулся на десятки строк с момента анализа), сама логика,
|
||||
сигнатуры и поведение подтверждены дословно:
|
||||
- `navigationSpaces = fixed.kind === 'valid' ? [space] : model` (`:15673`,
|
||||
рендер вкладок `:15746`) — подтверждает побочный эффект: при активном
|
||||
`floor` (#210) в панели всегда ровно один таб, значит правило AC5 «одно
|
||||
пространство — обработчики не навешиваются» само по себе накрывает и этот
|
||||
случай, отдельной ветки в контракте не требуется;
|
||||
- `firstSpaceId: this._model[0]?.id || ''` встречается дважды
|
||||
(`houseplan-card.ts:3350`, `:4273`), потребляется в `devices.ts` в
|
||||
`resolveExplicitMarkerPlacement` и в ветке virtual-маркера — подтверждён
|
||||
риск, ради которого написан §8.3/AC3;
|
||||
- `swipeTarget(dx, dy, zoom, spaceIds, current, minPx)` (`logic.ts:1870`)
|
||||
принимает `spaceIds` параметром, вызывающий код передаёт
|
||||
`this._model.map((m) => m.id)` (`houseplan-card.ts:5719`) — заявление §8.4
|
||||
«изменений не требует» подтверждено буквально;
|
||||
- `_fixedFloorState` (`:1165`) действительно имеет ветвь
|
||||
`out-of-range-index` — таблица в §«подтверждённая причина» точна;
|
||||
- `_canEdit` (`:845`, серверный `can_write`/`is_admin`) и `_mode` (тип
|
||||
`'view'|'plan'|'devices'|'decor'`, `mode-transition.ts:1`) — разные
|
||||
независимые гварды, оба перечислены в §8.1 корректно и раздельно;
|
||||
- `_writeConfig`/`expected_rev` (`:6702`) — существующий механизм, на
|
||||
который опирается AC6, подтверждён.
|
||||
- Итог: ни одного факта, выданного за решение без опоры на код или на
|
||||
явный owner-decision, не найдено. Все «подтверждённые причины» в ТЗ
|
||||
подтверждаются.
|
||||
- `docs/TOUCH-SUPPORT.md` прочитан целиком — раздел «Documentation rule»
|
||||
(строки 153–165) обязывает каждую спецификацию новой editor-фичи явно
|
||||
указать один из трёх ярлыков: `Touch editor: supported` /
|
||||
`best effort / intentionally degraded` / `not exposed`. В ТЗ #220 такой
|
||||
строки нет (см. находку M1).
|
||||
- `docs/CONFIG-COMPATIBILITY.md` прочитан как образец того, как описываются
|
||||
новые/условные поля конфигурации, для сверки с находкой M2.
|
||||
- AC1…AC8 проверены на однозначность и наличие способа доказательства —
|
||||
все восемь пронумерованы и каждый называет `unit`/`smoke`/`diff` (см.
|
||||
ниже раздел «AC»).
|
||||
- Блок §17 «Принятые предположения» проверен на то, что в нём действительно
|
||||
лежат только технические решения (не продуктовые) — да, все четыре пункта
|
||||
такие; продуктовые вопросы (тач, права, клавиатура) закрыты явными
|
||||
owner-decision в §4, что подтверждается вторым комментарием issue.
|
||||
|
||||
## Находки
|
||||
|
||||
### M1 — Medium, в скоупе. Отсутствует обязательная touch-классификация
|
||||
|
||||
`docs/TOUCH-SUPPORT.md` («Documentation rule», строки 153–165) требует, чтобы
|
||||
спецификация новой editor-фичи явно называла один из трёх статусов:
|
||||
`Touch editor: supported` / `best effort / intentionally degraded` /
|
||||
`not exposed`. Задача #220 добавляет новую editor-фичу (перетаскивание
|
||||
вкладок), которая по прямому решению владельца (§4.1 ТЗ) на сенсорных
|
||||
экранах не работает вовсе — это классический случай `not exposed`. ТЗ нигде
|
||||
не содержит этой строки (проверено `grep -n -i "touch editor"
|
||||
docs/specs/220-space-tab-reorder.md` → пусто).
|
||||
|
||||
**Почему это не формальность.** Правило существует специально для того, чтобы
|
||||
touch-ограничение было явным, а не выведенным читателем ТЗ из контекста: §103
|
||||
того же документа («Deliberate degradation rule») требует пяти условий,
|
||||
включая «touch limitation is deliberate, described in the user-facing
|
||||
limitations» — описание в USER-GUIDE запланировано (§15 ТЗ), но сама
|
||||
классификация как акт признания решения — нет.
|
||||
|
||||
**Чем закрывается:** одна строка вида `Touch editor: not exposed —
|
||||
перетаскивание вкладок работает только мышью в редакторах (§4.1, §7)` в ТЗ,
|
||||
рядом с §4 или §9. Не требует новых AC и не меняет скоуп.
|
||||
|
||||
### M2 — Medium, в скоупе. §9 утверждает «нет нового поля», хотя §8.3/§17.3 предполагают обратное
|
||||
|
||||
§9 («Данные») формулирует как решённый факт: *«только порядок элементов
|
||||
массива `spaces`. Ни одно поле не добавляется и не удаляется; миграции и
|
||||
compatibility-полей нет»*.
|
||||
|
||||
Это противоречит собственному §8.3/§17.3 того же документа. Причина
|
||||
противоречия не косметическая, а структурная: сегодня «первое пространство»
|
||||
физически **не имеет** другого представления, кроме позиции 0 в массиве —
|
||||
отдельного поля-якоря или временной метки создания не существует
|
||||
(подтверждено чтением `devices.ts` и `houseplan-card.ts`: `firstSpaceId`
|
||||
везде вычисляется как `_model[0]?.id`). Значит, чтобы якорь **пережил
|
||||
перезагрузку страницы** — а он обязан её пережить, иначе после reload
|
||||
`_model[0]` снова укажет на новый первый элемент (тот, что оказался на месте
|
||||
0 после перестановки), и AC3 перестанет выполняться при следующей сессии —
|
||||
единственный способ сохранить инвариант «маркер не меняет `space`» состоит в
|
||||
том, чтобы **где-то записать исходный id постоянно**. Собственный блок
|
||||
предположений автора (§17.3) прямо это и предлагает: «хранить якорь в
|
||||
`settings` при первой записи конфигурации».
|
||||
|
||||
То есть §9 отвечает «нет» на вопрос, который DoR (`PROCESS.md` §2.5: «миграция
|
||||
и compatibility-поля решены по `docs/CONFIG-COMPATIBILITY.md`») требует
|
||||
решить, а не задекларировать закрытым текстом, который тут же опровергается
|
||||
соседним разделом. Технический выбор (само устройство поля) законно оставлен
|
||||
автору — но факт «поле, скорее всего, появится» должен быть проговорён в §9,
|
||||
а не спрятан только в необязательном для чтения блоке предположений.
|
||||
|
||||
**Последствие, если не поправить.** Если реализация действительно добавит
|
||||
поле в `settings` (наиболее вероятный путь по собственному §17.3),
|
||||
`docs/CONFIG-COMPATIBILITY.md` не получит записи о новом поле, хотя документ
|
||||
существует ровно для таких случаев (сравните с трактовкой `known_devices`,
|
||||
`custom_fill`, `bg_mode` — все новые поля `settings` в этом файле
|
||||
задокументированы отдельным разделом с историей совместимости).
|
||||
|
||||
**Чем закрывается:** переформулировать §9 в духе «если якорь потребует нового
|
||||
поля в `settings` — оно аддитивно и forward-compatible: старые клиенты его
|
||||
игнорируют, при отсутствии поля fallback остаётся прежним (`_model[0]`)», и
|
||||
добавить короткую запись в `docs/CONFIG-COMPATIBILITY.md` в том же коммите,
|
||||
где появится реализация (не обязательно сейчас, но контракт должен быть
|
||||
проговорён в ТЗ, а не отрицаться).
|
||||
|
||||
### Low — не является блокирующим, не требует правки
|
||||
|
||||
- Гвард начала drag (`pointerdown` по `.tab`, порог 4px) не оговаривает
|
||||
явно элемент `.tabedit` (шестерёнка настройки пространства) внутри вкладки:
|
||||
текущий обработчик шестерёнки — `@click` с `e.stopPropagation()`
|
||||
(`houseplan-card.ts:15754`), но `pointerdown`, скорее всего, будет повешен
|
||||
на `.tab`, а не на кнопку, и `stopPropagation` в `@click` не остановит
|
||||
всплытие `pointerdown`. Реализация почти наверняка и так исключит `.tabedit`
|
||||
из зоны старта drag (иначе шестерёнка перестанет открывать диалог при
|
||||
случайном микросдвиге курсора) — это тривиальная деталь без продуктовой
|
||||
неоднозначности и без риска потери данных. Снимаю без правки: чисто
|
||||
реализационная деталь, накрываемая кодревью.
|
||||
|
||||
## AC — проверка на однозначность и доказуемость
|
||||
|
||||
| AC | Однозначен | Способ доказательства указан | Комментарий |
|
||||
|---|---|---|---|
|
||||
| AC1 | да | `smoke` | конкретное поведение: позиция + переживает reload |
|
||||
| AC2 | да | `smoke` | порог 4px, оба исхода (клик/drag) проверяемы |
|
||||
| AC3 | да | `unit` (`buildDevices`) | явно требует «тест красный до развязки» — хорошая практика |
|
||||
| AC4 | да | `unit`+`smoke` | подтверждено чтением кода: `swipeTarget` не требует правок |
|
||||
| AC5 | да | `unit`+`smoke` | четыре условия исключения перечислены явно и проверяемо |
|
||||
| AC6 | да | `unit`/`smoke` | опирается на существующий механизм `expected_rev` |
|
||||
| AC7 | да | `smoke` | «один раз за сессию» — паттерн уже есть в коде (`_onboardingShown`), не изобретён |
|
||||
| AC8 | да | `diff` + сверка бандлов | стандартный release-гейт |
|
||||
|
||||
Ни один AC не сформулирован расплывчато, у каждого назван корректный способ
|
||||
доказательства применительно к тому, что он утверждает.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Персона и job (`docs/SCOPE.md` J6) подобраны верно, сценарий и «что человек
|
||||
увидит» — в пользовательских терминах, без реализации.
|
||||
- Продуктовые решения (§4) действительно закрывают все три продуктовых
|
||||
вопроса из аналитики (тач/права/клавиатура) явной ссылкой на owner-decision
|
||||
2026-08-20, а не на догадку исполнителя.
|
||||
- Все три места, где порядок массива несёт скрытый смысл (`firstSpaceId`,
|
||||
`swipeTarget`, числовой `floor`), названы, подтверждены по коду и закрыты
|
||||
разделами контракта (§8.3, §8.4, §8.5) — это тот самый риск, ради которого
|
||||
задача вообще была оценена в сложность 4/10, а не 1/10.
|
||||
- Технические предположения (§17) действительно технические — ни один пункт
|
||||
не требует продуктового решения владельца, что соответствует правилу
|
||||
«размытое место не додумывается, а выносится владельцу» из PROCESS.md §7.1
|
||||
(здесь размытых продуктовых мест не осталось).
|
||||
- Мутационный гейт целится в самые опасные регрессии (потеря записи, съеденный
|
||||
клик, возврат порядко-зависимого `firstSpaceId`, снятие проверки
|
||||
`pointerType`) — это именно те четыре мутанта, которые действительно ловят
|
||||
типовые ошибки такой фичи.
|
||||
- Release-артефакты (§15) и откат (§16) присутствуют и покрывают оба
|
||||
changelog, оба USER-GUIDE, отсутствие влияния на golden/perf.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не проверял реализацию — её ещё нет (это ревью ТЗ, код не существует).
|
||||
- Не запускал никаких гейтов/тестов: на этапе ревью ТЗ гейты (`typecheck`,
|
||||
`npm test`, `npm run build`) неприменимы — класса A/B изменений нет, только
|
||||
документация (класс C).
|
||||
- Не проверял `docs/specs/README.md` на полноту сверх наличия строки про
|
||||
#220 — строка присутствует, этого достаточно для этого этапа.
|
||||
- Не оценивал абсолютную величину порога 4px и механику «вставка перед» —
|
||||
оба явно помечены автором как «принято предположительно, поменять
|
||||
свободно» (§17.1–17.2), оспаривать нечего: реализация вправе выбрать другое
|
||||
значение без нового цикла ревью.
|
||||
|
||||
## Итог
|
||||
|
||||
Два Medium-замечания, обе в скоупе задачи, обе устраняются точечной правкой
|
||||
текста ТЗ (без изменения AC, скоупа или контракта поведения). High-находок
|
||||
нет. Вердикт — жёлтый: автор правит §4/§9 ТЗ (добавляет touch-классификацию и
|
||||
снимает противоречие в разделе «Данные»), правка проходит второй цикл ревью
|
||||
ТЗ. Задача не `small`, лимит циклов ревью ТЗ — 4 (`PROCESS.md` §4); этот
|
||||
раунд — первый из них.
|
||||
@@ -0,0 +1,185 @@
|
||||
# SPEC-REVIEW-220-r2
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/220
|
||||
- **ТЗ:** `docs/specs/220-space-tab-reorder.md` (текущий коммит `e57e1d9`,
|
||||
«docs: close M1 and M2 from the spec review of #220»)
|
||||
- **Ревьюер:** Claude (роль «ревьюер ТЗ», PROCESS.md §2.4)
|
||||
- **Заход:** r2 · блокирующих циклов 1/4
|
||||
- **Вердикт:** жёлтый · High: 0 · Medium: 1 (в скоупе задачи)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Разбор по дельте (PROCESS.md §2.9, #214), а не заново — с одним расширением.
|
||||
Раунд r1 закрыл два Medium (M1, M2) точечной правкой текста ТЗ, но правка по
|
||||
M2 не косметическая: §8.3 полностью переписан — вместо «хранить якорь в
|
||||
`settings`» теперь «материализовать привязку в той же записи». Это смена
|
||||
контракта поведения (новая нормативная процедура записи конфигурации), а не
|
||||
переформулировка факта, поэтому именно этот кусок разобран полно — включая
|
||||
сверку с кодом `devices.ts`/`houseplan-card.ts` заново, а не по памяти r1.
|
||||
Остальное (§1–§7, §10–§16, AC1/2/4/5/6/7/8, мутационный гейт вне AC3, план
|
||||
автотестов) дельта не касается — унаследовано из r1 без повторной проверки
|
||||
(раздел ниже).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Найден вердикт r1 в комментариях issue #220
|
||||
(`https://github.com/Matysh/houseplan-card/issues/220#issuecomment-5361187761`,
|
||||
`2026-08-20T20:17:49Z`) и документ `docs/reviews/SPEC-REVIEW-220-r1.md`,
|
||||
зафиксировавший ТЗ на коммите `0fd2331`.
|
||||
2. Объявлена дельта: `git diff 0fd2331..HEAD -- docs/specs/220-space-tab-reorder.md`
|
||||
(HEAD = `e57e1d9`). Дельта — 58 вставок / 19 удалений, только §8.3, §9, AC3,
|
||||
мутационная таблица, §17.3; остальные разделы файла побитово не менялись.
|
||||
3. Комментарий владельца о закрытии r1 называет коммит `d0a5bfa` — такого
|
||||
объекта в репозитории нет (`git cat-file -t d0a5bfa` → `fatal: Not a valid
|
||||
object name`), вероятно переписан ребейзом веток issue. Не доверяю
|
||||
названному SHA, проверка сделана по факту — прямым диффом файла между
|
||||
`0fd2331` (зафиксирован в документе r1) и текущим `HEAD` (`e57e1d9`,
|
||||
единственный коммит после `c3278dd`/review-doc r1, который трогает файл
|
||||
ТЗ) — содержимое дословно совпадает с тем, что владелец описал в
|
||||
комментарии, расхождение только в имени SHA в тексте комментария.
|
||||
4. Каждая находка r1 (M1, M2) сверена не по заявлению автора, а по строке
|
||||
текста ТЗ — см. таблицу «Закрытие раунда r1».
|
||||
5. `docs/TOUCH-SUPPORT.md` (§153–165, «Documentation rule») прочитан повторно,
|
||||
построчно сверена ровно та формулировка ярлыка, которую требует правило.
|
||||
6. Новая нормативная процедура §8.3 (материализация в той же записи) сверена
|
||||
с фактическим кодом разрешения `firstSpaceId`:
|
||||
`src/devices.ts:1045-1065` (`resolveExplicitMarkerPlacement`, включая ветку
|
||||
`manualRoomWithoutArea`, ранее не разбиравшуюся отдельно ни в issue, ни в
|
||||
r1) и `src/devices.ts:1249` (виртуальный маркер). Оба случая подтверждают
|
||||
общий критерий ТЗ «нет ни `area`, ведущей в пространство, ни собственного
|
||||
`space`» — он корректно обобщает все три ветки кода, включая ветку
|
||||
marker-без-HA-area из #3, которую ТЗ не называет по номеру, но покрывает
|
||||
по содержанию.
|
||||
Также проверено третье использование `firstSpaceId`
|
||||
(`src/houseplan-card.ts:18818`, черновик маркера в диалоге) — это
|
||||
непостоянный preview, не пишется в конфиг, к обязательству §8.3
|
||||
(«ни один маркер не меняет `space`») не относится: диалог всегда
|
||||
пересчитывает его заново по актуальной модели.
|
||||
7. AC3 (единственный AC, задетый дельтой) перепроверен на однозначность и
|
||||
доказуемость с учётом новой формулировки. AC1, AC2, AC4–AC8 дельтой не
|
||||
задеты — унаследованы из r1 без повторной проверки.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **M1** — нет обязательной touch-классификации по `docs/TOUCH-SUPPORT.md` §153 | В §9 добавлена строка **`Touch editor: not exposed.`** с обоснованием (вкладки живут во View, где переключение — fully supported/release-blocking; жест на вкладке рискует съесть тап) и отдельно — что safety floor §69 соблюдён по построению | `docs/specs/220-space-tab-reorder.md:156-165`; формулировка ярлыка совпадает с требуемой буква в букву (сверено с `docs/TOUCH-SUPPORT.md:160-162`) |
|
||||
| **M2** — §9 отрицал новое поле конфигурации, §8.3/§17.3 предполагали якорь в `settings` | §8.3 переписан: вместо якоря — материализация существующего поля `marker.space` в той же записи, что и порядок; §9 теперь говорит «новых полей конфигурации не появляется... см. §8.3», и это уже не противоречит остальному документу, а согласуется с ним; §17.3 переписан в терминах материализации, старое предположение про якорь явно помечено отвергнутым | `docs/specs/220-space-tab-reorder.md:107-139` (§8.3), `:167-170` (§9), `:268-271` (§17.3); AC3 (`:202-206`) и мутационная таблица (`:240-241`) синхронно обновлены под новую механику |
|
||||
|
||||
Обе находки закрыты не декларативно: правка убирает саму причину
|
||||
противоречия (M2) и добавляет предметное содержание, а не просто ярлык (M1).
|
||||
Новых полей конфигурации в изменённом варианте действительно не появляется —
|
||||
проверено по коду: материализация пишет уже существующее поле маркера
|
||||
`space`, `CONFIG_SCHEMA` и `scripts/config-field-registry.mjs` в этой задаче
|
||||
не упоминаются как изменяемые, и это утверждение больше не противоречит §9.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Принято без повторной проверки в этом раунде — проверено и подтверждено в
|
||||
`docs/reviews/SPEC-REVIEW-220-r1.md` на коммите `0fd2331`, дельта эти разделы
|
||||
не касается:
|
||||
|
||||
- Персона, сценарий и job `docs/SCOPE.md` J6 (§1–§2 ТЗ).
|
||||
- Продуктовые решения владельца §4 (тач/права/клавиатура) и их соответствие
|
||||
owner-decision в комментариях issue.
|
||||
- §8.1, §8.2, §8.4, §8.5 контракта (порог 4px, `.tabadd`, запись через
|
||||
`_writeConfig`/`expected_rev`, `swipeTarget`, тост о числовом `floor`).
|
||||
- AC1, AC2, AC4, AC5, AC6, AC7, AC8 — однозначность и способ доказательства.
|
||||
- Мутанты `tab-reorder-not-persisted`, `tab-reorder-eats-click`,
|
||||
`tab-reorder-ignores-pointer-type` (не переписаны в дельте).
|
||||
- §11 (риски), §15 (release-артефакты), §16 (откат) кроме уже отражённых в
|
||||
дельте формулировок.
|
||||
- Low-находка r1 про `.tabedit`/`pointerdown` — оставлена без правки
|
||||
экспертным решением r1 («реализационная деталь, накрываемая кодревью»),
|
||||
дельта её не касается, пересматривать нет причины.
|
||||
|
||||
## Находки
|
||||
|
||||
### M3 — Medium, в скоупе. §17.3 маркирует нормативное требование как «свободно меняемое»
|
||||
|
||||
Заголовок §17 — «Принятые предположения (**техническое, менять свободно**)».
|
||||
Пункт 3 внутри него (`docs/specs/220-space-tab-reorder.md:268-271`) гласит:
|
||||
«Материализация привязки (§8.3) выполняется в том же `config/set`, что и
|
||||
порядок, а не отдельной записью: две записи дали бы окно, в котором порядок
|
||||
уже новый, а привязка ещё старая».
|
||||
|
||||
Но именно это же самое требование в §8.3 названо не предположением, а
|
||||
**«Норматив»** (`:113`, буквально это слово стоит заголовком абзаца) — и оно
|
||||
уже вошло в тестируемый контракт: AC3 требует «маркер получает явное `space`
|
||||
**в той же записи**» (`:203-204`), а мутант `reorder-skips-materialization`
|
||||
(`:240`) специально ловит его нарушение.
|
||||
|
||||
**Почему это не формальность.** §17.3 своим же текстом объясняет, почему
|
||||
атомарность нельзя менять свободно: если порядок и материализация уйдут
|
||||
двумя разными записями `config/set`, то в промежутке между ними
|
||||
`firstSpaceId = model[0]?.id` уже пересчитан по новому порядку — маркер,
|
||||
ещё не материализованный, в это окно резолвится в **не то** пространство.
|
||||
Если исполнитель прочитает заголовок §17 буквально («менять свободно») и
|
||||
раздельными записями, а не заголовок §8.3 («Норматив»), он получит ровно тот
|
||||
риск, ради которого писан весь раздел 8.3 и ради которого задача оценена в
|
||||
сложность 4/10 (см. §11, риск №2 «Маркеры уезжают» — главный риск задачи).
|
||||
Опасность усугубляется тем, что при таком (неверном) выборе реализации
|
||||
собственный юнит-тест AC3, написанный тем же исполнителем под ту же
|
||||
(неверную) модель, скорее всего будет проверять «согласованность после двух
|
||||
записей», а не «атомарность одной записи» — то есть мутационный гейт
|
||||
`reorder-skips-materialization` перестанет быть надёжным барьером именно в
|
||||
том сценарии, для которого его писали.
|
||||
|
||||
**Чем закрывается:** убрать пункт 3 из §17 (он не является свободным
|
||||
предположением — норма уже сформулирована в §8.3 как обязательная и
|
||||
проверяется AC3), либо явно пометить его как исключение из «менять свободно»
|
||||
с отсылкой на §8.3/AC3. Правка текстовая, AC и скоуп не меняются.
|
||||
|
||||
## AC — что перепроверено дельтой
|
||||
|
||||
| AC | Задет дельтой | Однозначен | Доказательство | Комментарий |
|
||||
|---|---|---|---|---|
|
||||
| AC1, AC2, AC4, AC5, AC6, AC7, AC8 | нет | — | — | унаследованы из r1 без повторной проверки |
|
||||
| AC3 | да | да | `unit` (`buildDevices` + запись) | формулировка усилена («маркеры с `area`/`space` — побитово прежними»), однозначна и доказуема; ровно вокруг него — находка M3 (не сам AC, а соседний раздел §17.3, который создаёт риск неверной трактовки реализации, проверяемой этим же AC) |
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- M1 и M2 закрыты предметно, а не декларативно — см. таблицу выше; §9 больше
|
||||
не противоречит §8.3.
|
||||
- Новая механика §8.3 (материализация) корректно обобщает все три реальных
|
||||
пути резолюции `firstSpaceId` в `devices.ts`, включая ветку
|
||||
`manualRoomWithoutArea` (маркер ручной комнаты без HA area, контракт #3),
|
||||
которую ни issue, ни r1 не разбирали пофамильно — критерий «нет `area`,
|
||||
ведущей в пространство, и нет собственного `space`» покрывает её без
|
||||
исключений.
|
||||
- Материализация не задевает preview-путь черновика маркера
|
||||
(`houseplan-card.ts:18818`) — он не персистентный, обязательство §8.3 на
|
||||
него не распространяется, и в ТЗ об этом ничего лишнего не заявлено.
|
||||
- Новая пара мутантов (`reorder-skips-materialization`,
|
||||
`materialization-touches-bound-markers`) корректно закрывает ровно два
|
||||
направления ошибки материализации (пропуск и избыточность), заменив собой
|
||||
устаревший `first-space-follows-order`.
|
||||
- Владелец сам в комментарии честно отделил решение от подгонки: назвал две
|
||||
альтернативы снятия противоречия M2 («признать поле» / «обойтись без
|
||||
него») и выбрал вторую с объяснением — это ровно то, что PROCESS требует
|
||||
от продуктовых/технических решений, выносимых в ревью.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не проверял реализацию — кода ещё нет, это ревью ТЗ.
|
||||
- Не запускал `tsc`/`npm test`/`npm run build` — класс изменения C
|
||||
(документация), гейты неприменимы, как и в r1.
|
||||
- Не пересматривал разделы, не тронутые дельтой (§1–§7, §10–§16, AC1/2/4-8,
|
||||
три из пяти мутантов) — они унаследованы из r1 на коммите `0fd2331`, дельта
|
||||
их не меняла ни байтом.
|
||||
- Не проверял точность SHA `d0a5bfa`, названного владельцем в комментарии о
|
||||
закрытии r1 — объект не существует в репозитории; вместо этого верификация
|
||||
сделана по прямому диффу файла до `HEAD` (`e57e1d9`), см. «Как
|
||||
проверялось», п.3. Расхождение SHA не влияет на вывод ревью, но зафиксировано
|
||||
как наблюдение.
|
||||
|
||||
## Итог
|
||||
|
||||
Обе находки r1 закрыты предметно. Правка §8.3 (материализация вместо якоря)
|
||||
— смена контракта поведения, а не косметика, и именно в ней найдена новая
|
||||
находка M3: §17 маркирует как «свободно меняемое» требование, которое сам же
|
||||
документ в §8.3 называет обязательным норматив и проверяет тестируемым AC3.
|
||||
Находка в скоупе, чинится точечной правкой текста (перенос/каветирование
|
||||
пункта 17.3), без изменения AC, скоупа или контракта поведения. High-находок
|
||||
нет. Вердикт — жёлтый: правка проходит третий цикл ревью ТЗ (заход r3, лимит
|
||||
циклов ревью ТЗ — 4).
|
||||
@@ -0,0 +1,150 @@
|
||||
# SPEC-REVIEW-220-r3
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/220
|
||||
- **ТЗ:** `docs/specs/220-space-tab-reorder.md` (текущий коммит `5ba46df`,
|
||||
«docs: take the atomic write out of the "free to change" block (#220 M3)»)
|
||||
- **Ревьюер:** Claude (роль «ревьюер ТЗ», PROCESS.md §2.4)
|
||||
- **Заход:** r3 · блокирующих циклов израсходовано 2 из 4
|
||||
- **Вердикт:** зелёный · High: 0 · Medium: 0
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Разбор по дельте (PROCESS.md §2.9, #214). Найден вердикт r2
|
||||
(`https://github.com/Matysh/houseplan-card/issues/220#issuecomment-5361277814`,
|
||||
`2026-08-20T20:26:51Z`) и документ `docs/reviews/SPEC-REVIEW-220-r2.md`,
|
||||
зафиксировавший ТЗ на коммите `e57e1d9`. Владелец в комментарии о закрытии r2
|
||||
(`https://github.com/Matysh/houseplan-card/issues/220#issuecomment-5361297569`)
|
||||
сам называет SHA после пуша — `5ba46df` — и это совпадает с текущим `HEAD`;
|
||||
в r2 автор один раз назвал несуществующий SHA (`d0a5bfa`, переписан
|
||||
`git pull --rebase`), в этом раунде расхождения нет, проверено
|
||||
`git rev-parse HEAD` = `5ba46df`.
|
||||
|
||||
`git diff e57e1d9..5ba46df -- docs/specs/220-space-tab-reorder.md` — единственный
|
||||
хунк, 12 вставок / 4 удаления, весь внутри пункта 3 §17 («Принятые
|
||||
предположения»). Ни §8.3 (норматив материализации), ни §9, ни AC12, ни таблица
|
||||
мутантов §14, ни любой другой раздел не тронуты ни байтом — это чисто
|
||||
текстовая правка одного абзаца, не смена контракта поведения (сам норматив
|
||||
и его формулировка в §8.3 не изменились, изменилось только то, как §17
|
||||
классифицирует уже существующее требование). Дельта локальна, полный
|
||||
повторный разбор не требуется.
|
||||
|
||||
Разобрана дельта полностью: сам абзац §17.3 и его согласованность с §8.3, AC3
|
||||
и мутантом `reorder-skips-materialization`, на которые он теперь явно
|
||||
ссылается. Остальное (§1–§16 кроме одного абзаца §17, AC1/2/4–8, весь
|
||||
мутационный гейт, план автотестов, release-артефакты, откат) дельта не
|
||||
касается — унаследовано из r2 (раздел ниже).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитан текущий пункт 3 §17 (`docs/specs/220-space-tab-reorder.md:268-279`)
|
||||
целиком и сверен построчно с находкой M3 ревью r2: требовалось либо убрать
|
||||
пункт из «менять свободно», либо явно пометить его исключением со ссылкой
|
||||
на §8.3/AC3. Автор выбрал второй вариант: первая фраза пункта —
|
||||
«Атомарность записи предположением не является», далее прямая ссылка на
|
||||
норматив §8.3, AC3 и мутант `reorder-skips-materialization`, и явное
|
||||
«свободно меняется всё остальное в этом разделе, но не это». Формулировка
|
||||
закрывает причину противоречия, а не переименовывает её: заголовок §17
|
||||
(«менять свободно») больше не может быть прочитан как разрешение развязать
|
||||
запись на две — конкретный пункт прямо и по имени объявляет себя
|
||||
исключением.
|
||||
2. Проверено, что новая формулировка не создаёт обратного противоречия: §8.3
|
||||
(`:107-139`, не тронут дельтой) называет атомарность «Норматив», AC3
|
||||
(`:194-224`, не тронут дельтой) и мутант `reorder-skips-materialization`
|
||||
(§14, не тронут дельтой) её проверяют — пункт §17.3 теперь на них ссылается,
|
||||
а не спорит с ними. Три источника (§8.3, AC3, мутант, §17.3) говорят одно и
|
||||
то же.
|
||||
3. Сохранённый второй абзац («историческая справка про якорь в `settings`,
|
||||
отвергнутый по M2 r1») — не норматив, а комментарий к истории решения;
|
||||
он не противоречит новой формулировке первого абзаца и не меняет статус
|
||||
пункта.
|
||||
4. `grep -n "17\.3\|§17"` по всему файлу — единственная ссылка на этот раздел
|
||||
находится в самом пункте 3 (самоссылка на находку M3), других мест
|
||||
документа, которые ожидали бы прежнюю формулировку, нет.
|
||||
5. AC, задетые дельтой — ни одного: AC3, единственный кандидат, ссылается на
|
||||
§8.3 как источник нормы, а не на §17, и текст AC3 не менялся. Проверено
|
||||
построчным сравнением AC3 в текущем файле с версией, зафиксированной в
|
||||
`SPEC-REVIEW-220-r2.md` — идентична.
|
||||
|
||||
## Закрытие раунда r2
|
||||
|
||||
| Находка r2 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **M3** — §17 маркирует нормативное требование атомарности записи как «свободно меняемое», хотя §8.3 называет его нормативом, а AC3 и мутант `reorder-skips-materialization` его проверяют | Пункт 3 §17 переписан: первая фраза — «Атомарность записи предположением не является», с прямой ссылкой на норматив §8.3, AC3 и мутант, и явной оговоркой «свободно меняется всё остальное в этом разделе, но не это» | `docs/specs/220-space-tab-reorder.md:268-275`; сверено с §8.3 (`:113`, слово «Норматив» как было), AC3 и таблицей мутантов §14 — все три не менялись и согласуются с новой формулировкой |
|
||||
|
||||
Находка закрыта не переименованием, а содержательно: раньше заголовок §17
|
||||
разрешал буквальное прочтение «эту запись можно разнести на две», теперь
|
||||
пункт сам называет себя исключением и объясняет, какому нормативу подчинён.
|
||||
|
||||
## Унаследовано из r2
|
||||
|
||||
Принято без повторной проверки в этом раунде — проверено и подтверждено в
|
||||
`docs/reviews/SPEC-REVIEW-220-r2.md` на коммите `e57e1d9`, дельта эти разделы
|
||||
не касается:
|
||||
|
||||
- Персона, сценарий и job `docs/SCOPE.md` J6 (§1–§2 ТЗ) — унаследовано из r2,
|
||||
которое само унаследовало это из r1 (`0fd2331`).
|
||||
- Продуктовые решения владельца §4 (тач/права/клавиатура).
|
||||
- §8.1, §8.2, §8.4, §8.5 контракта.
|
||||
- §8.3 целиком (материализация вместо якоря) и его согласованность с §9 —
|
||||
закрытие M1/M2 из r1, подтверждённое в r2 сверкой с кодом `devices.ts`
|
||||
(все три пути резолюции `firstSpaceId`, включая ветку ручной комнаты без
|
||||
HA area).
|
||||
- AC1–AC8 — однозначность и способ доказательства (AC3 переподтверждён в r2
|
||||
под новую механику материализации; в этом раунде не менялся).
|
||||
- Все пять записей мутационного гейта §14, включая
|
||||
`reorder-skips-materialization` и `materialization-touches-bound-markers`.
|
||||
- §9 (данные/i18n/touch: `Touch editor: not exposed`), §10 (performance),
|
||||
§11 (риски), §15 (release-артефакты), §16 (откат).
|
||||
- Low-находка r1 про `.tabedit`/`pointerdown` — оставлена без правки
|
||||
экспертным решением r1, дельта её не касается.
|
||||
- Наблюдение r2 про несуществующий SHA `d0a5bfa` в комментарии о закрытии r1 —
|
||||
закрыто содержательно в этом раунде: владелец подтвердил промах и в
|
||||
комментарии о закрытии r2 назвал SHA `5ba46df` уже после пуша, он совпадает
|
||||
с текущим `HEAD`.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет. High: 0, Medium: 0, Low: 0.
|
||||
|
||||
## AC — что перепроверено дельтой
|
||||
|
||||
| AC | Задет дельтой | Комментарий |
|
||||
|---|---|---|
|
||||
| AC1, AC2, AC4, AC5, AC6, AC7, AC8 | нет | унаследованы из r1/r2 без повторной проверки |
|
||||
| AC3 | нет (текст AC3 не менялся; менялась только классификация соседнего раздела §17) | сверен построчно с версией из r2 — идентичен |
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- M3 закрыта предметно: пункт §17.3 больше не может быть прочитан как
|
||||
разрешение разнести атомарную запись на два `config/set` — именно тот
|
||||
риск («маркеры уезжают»), ради предотвращения которого писан весь §8.3.
|
||||
- Три источника нормы (§8.3 «Норматив», AC3, мутант
|
||||
`reorder-skips-materialization`) и указатель на них в §17.3 теперь говорят
|
||||
одно и то же, без противоречий.
|
||||
- Историческая справка про отвергнутый якорь `settings` сохранена как
|
||||
контекст решения, не как действующая норма — не создаёт путаницы с текущей
|
||||
механикой материализации.
|
||||
- Дельта не расширилась дальше объявленной находки: диффом подтверждено, что
|
||||
ни один другой раздел документа (контракт, AC, мутанты, release-артефакты,
|
||||
откат) не менялся между `e57e1d9` и `5ba46df`.
|
||||
- Владелец сам указал на собственную ошибку с несуществующим SHA прошлого
|
||||
раунда и впредь называет SHA только после пуша — это снимает наблюдение r2,
|
||||
а не оставляет его висеть.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не проверял реализацию — кода ещё нет, это ревью ТЗ.
|
||||
- Не запускал `tsc`/`npm test`/`npm run build` — класс изменения C
|
||||
(документация), гейты неприменимы, как и в r1/r2.
|
||||
- Не пересматривал разделы, не тронутые дельтой (§1–§16 кроме одного абзаца
|
||||
§17, AC1/2/4–8, весь мутационный гейт) — они унаследованы из r2 на коммите
|
||||
`e57e1d9`, дельта их не меняла ни байтом (подтверждено `git diff`).
|
||||
- Не проверял `docs/specs/README.md` сверх того, что было проверено в r1 —
|
||||
дельта этого раунда файла не касается.
|
||||
|
||||
## Итог
|
||||
|
||||
Находка M3 закрыта предметно и без побочных противоречий. Правка — 16 строк
|
||||
внутри одного пункта §17, не меняет AC, скоуп, контракт поведения или
|
||||
мутационный гейт. Новых находок не появилось. Вердикт — зелёный: ТЗ готово к
|
||||
`S5-ready`.
|
||||
@@ -0,0 +1,282 @@
|
||||
# Issue #220 — порядок пространств перетаскиванием вкладок
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/220
|
||||
- **Связанные контракты:** #210 (фиксированный этаж), #170 (fallback-привязка маркера), #3 (комнаты без HA-зоны)
|
||||
- **Тип:** feature, обычный полный трек
|
||||
- **Приоритет:** P2
|
||||
- **Пользовательское изменение:** да
|
||||
|
||||
## 1. Сценарий и персона
|
||||
|
||||
**Персона:** администратор плана — тот, кто заводит пространства и поддерживает
|
||||
план в актуальном виде (`docs/SCOPE.md`, job J6).
|
||||
|
||||
**Сценарий:** дом рос не по порядку. Сначала завели «Квартиру», через месяц —
|
||||
«Подвал», потом «Мансарду». Вкладки стоят в порядке создания, а читается дом
|
||||
снизу вверх. Сегодня переставить их нечем: порядок вкладок — это порядок
|
||||
массива `config.spaces`, и единственный способ его изменить — удалить
|
||||
пространство и завести заново, потеряв планировку, устройства и привязки.
|
||||
|
||||
**Момент:** администратор в режиме редактора видит панель вкладок, берёт вкладку
|
||||
мышью и перетаскивает на новое место.
|
||||
|
||||
## 2. Что человек увидит до и после
|
||||
|
||||
**До:** вкладки стоят в порядке заведения; изменить порядок невозможно.
|
||||
|
||||
**После:** в режимах редактора вкладку можно взять мышью и перетащить; во время
|
||||
перетаскивания видно, куда она встанет; после отпускания порядок сохраняется и
|
||||
переживает перезагрузку. Свайп между этажами и стрелки киоска идут в новом
|
||||
порядке. Во View и на сенсорных экранах ничего не меняется: там вкладка
|
||||
по-прежнему только переключает пространство.
|
||||
|
||||
## 3. Подтверждённая причина
|
||||
|
||||
Порядок вкладок задан порядком массива: панель рендерится прямым проходом по
|
||||
модели (`houseplan-card.ts:15717`), `navigationSpaces.map(...)`, отдельного поля
|
||||
сортировки нет. Запись идёт штатным `_writeConfig` (`:6692`) через
|
||||
`houseplan/config/set` с `expected_rev`; изменение порядка — обычная правка
|
||||
массива, схема и миграции не нужны.
|
||||
|
||||
**Порядок массива несёт смысл в трёх местах.** Проверено по коду dev:
|
||||
|
||||
| Место | Код | Что зависит |
|
||||
|---|---|---|
|
||||
| Fallback-пространство маркера | `houseplan-card.ts:3350`, `:4273` → `devices.ts:1058`, `:1063`, `:1249` | `firstSpaceId = _model[0]?.id`: маркеры без явного пространства садятся в первое |
|
||||
| Свайп и карусель | `logic.ts:1870-1879` | `spaceIds[(i + 1) % n]` — сосед по индексу |
|
||||
| `floor` числом (#210) | `houseplan-card.ts:1165` `_fixedFloorState` | числовое значение разрешается как позиция, есть ветвь `out-of-range-index` |
|
||||
|
||||
Первое — риск потери данных на ровном месте, второе и третье обязаны следовать
|
||||
новому порядку либо предупреждать.
|
||||
|
||||
## 4. Продуктовые решения владельца (2026-08-20)
|
||||
|
||||
1. **Перетаскивание — только мышь и только в режимах редактора** (`plan`,
|
||||
`devices`, `decor`). Во View и киоске поведение вкладок не меняется вовсе.
|
||||
2. **Числовой `floor` из #210** — после успешной перестановки показать
|
||||
предупреждение один раз.
|
||||
3. **Клавиатурная альтернатива не нужна** — `docs/SCOPE.md` честно фиксирует
|
||||
отсутствие клавиатурной навигации в редакторах.
|
||||
|
||||
## 5. Цели
|
||||
|
||||
- Порядок пространств меняется без потери данных и переживает перезагрузку.
|
||||
- Ни один маркер не меняет своё размещение из-за перестановки.
|
||||
- Навигация (свайп, карусель) следует новому порядку немедленно.
|
||||
- Скрытая зависимость «позиция в массиве = смысл» становится явной и покрытой
|
||||
тестами.
|
||||
|
||||
## 6. Scope
|
||||
|
||||
- Панель вкладок: обработчики `pointerdown`/`pointermove`/`pointerup` на `.tab`,
|
||||
порог начала перетаскивания, индикатор места вставки, курсор.
|
||||
- Запись нового порядка `config.spaces` через существующий `_writeConfig`.
|
||||
- Развязка `firstSpaceId` с позицией в массиве (см. §8).
|
||||
- Предупреждение о числовом `floor` (§4.2), ключи i18n en+ru.
|
||||
- `docs/USER-GUIDE.md` и `docs/USER-GUIDE.ru.md`, оба changelog.
|
||||
|
||||
## 7. Не входит в задачу
|
||||
|
||||
- Перетаскивание на сенсорных экранах и во View — решение владельца §4.1.
|
||||
- Клавиатурная альтернатива — §4.3.
|
||||
- Порядок комнат, устройств, вложений, вкладок режимов (`plan`/`devices`/`decor`).
|
||||
- Сортировка «по имени» и любые автоматические порядки.
|
||||
- Изменение самого механизма `floor` из #210: числовая адресация остаётся как
|
||||
есть, задача только предупреждает.
|
||||
|
||||
## 8. Контракт поведения
|
||||
|
||||
### 8.1. Перетаскивание
|
||||
|
||||
- Drag начинается только при `_canEdit`, не в киоске, `this._mode !== 'view'`,
|
||||
`event.pointerType === 'mouse'` и после смещения ≥ 4 px по горизонтали.
|
||||
До порога это обычный клик — переключение пространства сохраняется.
|
||||
- Вкладка «+» (`.tabadd`) в перестановке не участвует и не может стать целью.
|
||||
- Во время перетаскивания видно место вставки; при отпускании вне панели
|
||||
порядок не меняется.
|
||||
- Одно пространство — перетаскивать нечего, обработчики не навешиваются.
|
||||
|
||||
### 8.2. Запись
|
||||
|
||||
- Новый порядок пишется целиком массивом `spaces` штатным `_writeConfig`
|
||||
с `expected_rev`; конфликт ревизий обрабатывается как у любой другой правки.
|
||||
- Активное пространство после перестановки не меняется: перетащили не ту, что
|
||||
открыта — открытая остаётся открытой; перетащили открытую — она открыта на
|
||||
новом месте.
|
||||
|
||||
### 8.3. Маркеры не двигаются: материализация вместо нового поля
|
||||
|
||||
Обязательное свойство одно: **ни один маркер не меняет `space` из-за изменения
|
||||
порядка**. Достигается оно не хранением якоря, а тем, что перестановка делает
|
||||
явной ту привязку, которая до неё держалась на позиции.
|
||||
|
||||
**Норматив.** В той же транзакции записи, что и новый порядок, каждый маркер,
|
||||
чьё пространство сегодня разрешается через `firstSpaceId` — то есть у него нет
|
||||
ни `area`, ведущей в пространство, ни собственного `space` — получает явное
|
||||
`space`, равное тому пространству, в котором он находится **сейчас**, до
|
||||
перестановки. После этого его размещение от порядка не зависит вовсе, и
|
||||
`firstSpaceId` перестаёт быть для него значимым.
|
||||
|
||||
**Почему так, а не якорь в `settings`.** Первая редакция ТЗ предполагала хранить
|
||||
id «первого» пространства отдельным полем. Ревью r1 (M2) справедливо указало,
|
||||
что это новое поле конфигурации, которое §9 в том же документе отрицал.
|
||||
Материализация решает ту же задачу без расширения схемы:
|
||||
|
||||
- новых полей нет — `CONFIG_SCHEMA`, `scripts/config-field-registry.mjs` и
|
||||
`docs/CONFIG-COMPATIBILITY.md` не трогаются;
|
||||
- запись идёт одним `config/set` с уже существующими полями маркеров;
|
||||
- правка данных минимальна и сохраняет наблюдаемое состояние: маркер остаётся
|
||||
ровно там, где пользователь его видел;
|
||||
- откат не нужен: явное `space` — валидное и предпочтительное состояние,
|
||||
которое карточка и так пишет при любом сохранении маркера.
|
||||
|
||||
**Граница.** Материализуются только маркеры, разрешавшиеся через
|
||||
`firstSpaceId`. Маркеры с `area` или с уже заданным `space` не трогаются —
|
||||
проверяется AC3.
|
||||
|
||||
Если исполнитель обнаружит случай, который материализация не покрывает
|
||||
(например, маркер вообще не попал в текущую модель), это не повод возвращать
|
||||
якорь молча: такой случай выносится в ревью как находка.
|
||||
|
||||
### 8.4. Навигация
|
||||
|
||||
`swipeTarget` продолжает работать по индексам — он получает уже
|
||||
переупорядоченный `spaceIds`, поэтому изменений не требует. AC4 фиксирует, что
|
||||
свайп идёт в новом порядке немедленно, без перезагрузки.
|
||||
|
||||
### 8.5. Предупреждение о числовом `floor`
|
||||
|
||||
После первой успешной перестановки в текущей сессии карточка показывает тост:
|
||||
«Порядок пространств изменён. Если где-то этаж карточки задан номером, проверьте
|
||||
такие панели». Показывается один раз за сессию, независимо от числа
|
||||
перестановок, и не блокирует работу.
|
||||
|
||||
## 9. Данные, i18n, a11y, touch, privacy, security
|
||||
|
||||
**Touch editor: not exposed.** Классификация по `docs/TOUCH-SUPPORT.md` §153:
|
||||
перетаскивание вкладок на сенсорных экранах не появляется вовсе — ни как
|
||||
degraded-вариант, ни как долгое нажатие. Это решение владельца §4.1, а не
|
||||
недоделка. Обоснование: вкладки живут во View, где переключение пространств —
|
||||
fully supported на тач и release-blocking; любой жест на вкладке рискует съесть
|
||||
тап. Порядок пространств меняется на десктопе, как и остальная работа с планом.
|
||||
|
||||
Safety floor §69 того же документа соблюдён по построению: на тач-устройстве
|
||||
поведение вкладок не меняется ни на йоту, значит ни потери данных, ни случайной
|
||||
мутации при мультитаче новая функция внести не может.
|
||||
|
||||
- **Данные:** порядок элементов массива `spaces` плюс материализация неявной
|
||||
привязки маркеров (§8.3). Новых полей конфигурации не появляется, схема и
|
||||
`CONFIG_SCHEMA` не меняются — см. §8.3 о том, почему выбран путь без нового
|
||||
поля и что это значит для `docs/CONFIG-COMPATIBILITY.md`.
|
||||
- **i18n:** один новый ключ тоста (en + ru) и, при необходимости, `title`
|
||||
вкладки в режиме редактора.
|
||||
- **a11y:** без изменений — клавиатурной навигации в редакторах нет и не
|
||||
обещано (`docs/SCOPE.md`).
|
||||
- **privacy / security:** новых данных и путей записи нет.
|
||||
|
||||
## 10. Performance
|
||||
|
||||
Панель вкладок — единицы элементов. Обработчики навешиваются только в режимах
|
||||
редактора и только для мыши. Влияния на бюджеты нет; performance-профили не
|
||||
затрагиваются.
|
||||
|
||||
## 11. Риски
|
||||
|
||||
1. **Drag съедает клик.** Порог 4 px и проверка `pointerType` — единственное,
|
||||
что отделяет перестановку от переключения. AC2 проверяет обе стороны.
|
||||
2. **Маркеры уезжают.** Главный риск задачи: `firstSpaceId` сегодня буквально
|
||||
«первый по порядку». Закрывается §8.3 + AC3 + мутант.
|
||||
3. **Числовой `floor`.** Предупреждение — смягчение, а не защита: чужие панели
|
||||
отсюда не видны. Записано в USER-GUIDE.
|
||||
4. **Конкурентная правка.** Перестановка уходит с `expected_rev`; на конфликт
|
||||
реагирует общий механизм — AC6.
|
||||
|
||||
## 12. Acceptance criteria
|
||||
|
||||
1. **AC1 — перестановка и запись.** Перетаскивание вкладки меняет её позицию;
|
||||
порядок сохраняется на сервере и переживает перезагрузку страницы.
|
||||
**Доказательство:** `smoke`.
|
||||
2. **AC2 — клик не сломан.** Клик по вкладке без смещения переключает
|
||||
пространство; смещение ≥ 4 px начинает перетаскивание и клик не срабатывает.
|
||||
**Доказательство:** `smoke`.
|
||||
3. **AC3 — маркеры не двигаются.** Конфигурация с маркером, чьё размещение
|
||||
опирается на fallback-пространство, после перестановки даёт то же `space`
|
||||
для каждого маркера; такой маркер получает явное `space` в той же записи, а
|
||||
маркеры с `area` или с уже заданным `space` остаются побитово прежними.
|
||||
Тест красный до §8.3. **Доказательство:** `unit` (`buildDevices` + запись).
|
||||
4. **AC4 — навигация следует порядку.** После перестановки `swipeTarget`
|
||||
возвращает нового соседа немедленно, без перезагрузки.
|
||||
**Доказательство:** `unit` + `smoke`.
|
||||
5. **AC5 — границы включения.** Обработчики не навешиваются во View, в киоске,
|
||||
при `pointerType !== 'mouse'` и при единственном пространстве; вкладка «+»
|
||||
не участвует. **Доказательство:** `unit` (чистая функция решения) + `smoke`
|
||||
для View.
|
||||
6. **AC6 — конкурентная правка.** Перестановка уходит с `expected_rev`;
|
||||
конфликт ревизий не молчит и не теряет порядок.
|
||||
**Доказательство:** `unit` либо `smoke` по существующему механизму.
|
||||
7. **AC7 — предупреждение о числовом `floor`.** После первой перестановки в
|
||||
сессии показан тост; повторные перестановки его не повторяют.
|
||||
**Доказательство:** `smoke`.
|
||||
8. **AC8 — release-артефакты.** Оба changelog, оба USER-GUIDE описывают
|
||||
перестановку и её ограничения (мышь, редактор, числовой `floor`); `dist`,
|
||||
demo и integration bundle идентичны друг другу.
|
||||
**Доказательство:** diff + сверка копий бандла.
|
||||
|
||||
## 13. План автотестов
|
||||
|
||||
- `test/` — юниты на чистую функцию «можно ли начать перетаскивание» (AC5) и на
|
||||
стабильность `firstSpaceId` в `buildDevices` (AC3); проверка `swipeTarget` на
|
||||
переупорядоченном списке (AC4).
|
||||
- `demo/smoke_space_tab_reorder.mjs` — новый смок: перестановка мышью, запись,
|
||||
сохранение активного пространства, клик без смещения, отсутствие drag во View,
|
||||
тост о числовом `floor` (AC1, AC2, AC4, AC5, AC7).
|
||||
- Существующие смоки навигации и `smoke_fixed_floor` (#210) — прогон без правок.
|
||||
|
||||
## 14. Мутационный гейт (`scripts/mutation-gate.mjs`)
|
||||
|
||||
| id | Патч | Guard |
|
||||
|---|---|---|
|
||||
| `tab-reorder-not-persisted` | перестановка меняет только локальную модель, `_writeConfig` не зовётся | смок AC1 |
|
||||
| `tab-reorder-eats-click` | убрать порог смещения — любой pointerdown начинает drag | смок AC2 |
|
||||
| `reorder-skips-materialization` | записывать новый порядок, не материализуя привязку маркеров | юнит AC3 |
|
||||
| `materialization-touches-bound-markers` | материализовать `space` и у маркеров с `area` | юнит AC3 |
|
||||
| `tab-reorder-ignores-pointer-type` | снять проверку `pointerType === 'mouse'` | юнит AC5 |
|
||||
|
||||
## 15. Release-артефакты
|
||||
|
||||
- `docs/CHANGELOG.md` + `docs/CHANGELOG.ru.md` — `User-Visible: yes`;
|
||||
- `docs/USER-GUIDE.md` + `docs/USER-GUIDE.ru.md` — раздел про панель вкладок:
|
||||
перестановка мышью в редакторе, ограничение по тачу, оговорка про числовой
|
||||
`floor`;
|
||||
- golden не затрагивается: панель вкладок в матрице не участвует, визуальных
|
||||
изменений в состоянии покоя нет;
|
||||
- performance-профили не затрагиваются.
|
||||
|
||||
## 16. Откат
|
||||
|
||||
Порядок — обычные данные, обратной миграции не требуется: администратор
|
||||
перетаскивает вкладки назад. Код откатывается снятием обработчиков; развязка
|
||||
`firstSpaceId` (§8.3) остаётся полезной сама по себе и откату не подлежит.
|
||||
|
||||
## 17. Принятые предположения (техническое, менять свободно)
|
||||
|
||||
1. **Порог 4 px** взят как минимально заметное намеренное движение; точное
|
||||
число не продуктовое решение и может быть изменено ревьюером.
|
||||
2. **Механика вставки** — вставка перетаскиваемой вкладки перед той, над
|
||||
серединой которой отпущена мышь. Альтернатива (обмен местами) отвергнута:
|
||||
при переносе через несколько позиций она даёт неожиданный результат.
|
||||
3. **Атомарность записи предположением не является.** Требование «порядок и
|
||||
материализация уходят одним `config/set`» — норматив §8.3, его проверяет AC3
|
||||
и стережёт мутант `reorder-skips-materialization`. Разносить запись на две
|
||||
нельзя: между ними возникает окно, где порядок уже новый, а привязка ещё
|
||||
старая — ровно тот риск, ради которого §8.3 написан. Здесь пункт оставлен
|
||||
только как указатель: свободно меняется всё остальное в этом разделе, но не
|
||||
это (находка M3 ревью r2; прежняя редакция §17 держала норматив под
|
||||
заголовком «менять свободно»).
|
||||
|
||||
Историческая справка к §8.3: первая редакция предлагала хранить якорь
|
||||
`firstSpaceId` в `settings` — отвергнута по M2 ревью r1 как новое поле
|
||||
конфигурации.
|
||||
4. **Тост о числовом `floor`** показывается всегда, а не только когда числовой
|
||||
`floor` действительно где-то используется: карточка не видит чужие панели,
|
||||
поэтому условие проверить нечем.
|
||||
@@ -60,6 +60,7 @@ GitHub Issues и GitHub Projects (v2) остаются единственным
|
||||
| [#218](https://github.com/Matysh/houseplan-card/issues/218) Floating-point шум комнаты не гасит Glow пространства | [218-glow-floor-geometry.md](218-glow-floor-geometry.md) |
|
||||
| [#219](https://github.com/Matysh/houseplan-card/issues/219) Единая палитра замков и glyph на оранжевых подложках | [219-lock-orange-palette.md](219-lock-orange-palette.md) |
|
||||
| [#205](https://github.com/Matysh/houseplan-card/issues/205) Продолжение следа после короткой остановки пылесоса | [205-vacuum-trail-resume-grace.md](205-vacuum-trail-resume-grace.md) |
|
||||
| [#220](https://github.com/Matysh/houseplan-card/issues/220) Порядок пространств перетаскиванием вкладок | [220-space-tab-reorder.md](220-space-tab-reorder.md) |
|
||||
| [#226](https://github.com/Matysh/houseplan-card/issues/226) Entity-marker не дублируется родительским HA-устройством | [226-entity-parent-dedup.md](226-entity-parent-dedup.md) |
|
||||
| [#223](https://github.com/Matysh/houseplan-card/issues/223) Optimize канонизирует координаты без floating-point шума | [223-optimize-coordinate-canonicalization.md](223-optimize-coordinate-canonicalization.md) |
|
||||
|
||||
|
||||
@@ -717,6 +717,85 @@ export const MUTANTS = [
|
||||
replace: ' .dev:not(.unavail):hover {',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'tab-reorder-not-persisted',
|
||||
guard: 'node demo/smoke_space_tab_reorder.mjs',
|
||||
because: 'перестановка вкладок, оставшаяся только в памяти, выглядит рабочей ровно до '
|
||||
+ 'перезагрузки страницы — смок обязан требовать запись на сервер',
|
||||
patches: [{
|
||||
file: 'src/houseplan-card.ts',
|
||||
find: ' cfg.spaces = applySpaceOrder(cfg.spaces || [], order);\n this._saveConfig();',
|
||||
replace: ' cfg.spaces = applySpaceOrder(cfg.spaces || [], order);',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'reorder-skips-materialization',
|
||||
guard: 'node demo/smoke_space_tab_reorder.mjs',
|
||||
because: 'без материализации привязки маркер без space и area уезжает вслед за порядком: '
|
||||
+ 'его пространство решает firstSpaceId, а тот после перестановки другой (#220 §8.3)',
|
||||
patches: [{
|
||||
file: 'src/houseplan-card.ts',
|
||||
find: ' const byId = new Map(pinned.map((entry) => [entry.id, entry.space]));',
|
||||
replace: ' const byId = new Map();',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'materialization-touches-bound-markers',
|
||||
guard: 'node --test --test-name-pattern="issue 220" test/space-order.test.mjs',
|
||||
because: 'материализация обязана трогать только маркеры, чьё пространство решает порядок; '
|
||||
+ 'запись space маркеру, закреплённому HA-областью, кладёт в конфиг поле, которое '
|
||||
+ 'сдвинет его в старое первое пространство в день смены области (ревью r1, H1)',
|
||||
patches: [{
|
||||
file: 'src/space-order.ts',
|
||||
find: ' if (area && areaToSpace[area]) continue;',
|
||||
replace: ' if (false) continue;',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'tab-drag-survives-release-outside',
|
||||
guard: 'node demo/smoke_space_tab_reorder.mjs',
|
||||
because: 'мышь, отпущенная мимо панели, обязана завершить жест: иначе перетаскивание '
|
||||
+ 'зависает с moved:true и съедает следующий клик по вкладке (ревью r1, M1)',
|
||||
patches: [{
|
||||
file: 'src/houseplan-card.ts',
|
||||
find: " window.addEventListener('pointerup', this._tabDragRelease);",
|
||||
replace: ' void 0;',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'tab-drag-outlives-the-card',
|
||||
guard: 'node demo/smoke_space_tab_reorder.mjs',
|
||||
because: 'слушатели жеста, пережившие disconnectedCallback, держат инстанс карточки '
|
||||
+ 'и дают невидимой карточке записать порядок по следующему pointerup на странице '
|
||||
+ '(ревью r2/r3, F1)',
|
||||
patches: [{
|
||||
file: 'src/houseplan-card.ts',
|
||||
find: ' this._endTabDrag();\n clearInterval(this._cycleTimer);',
|
||||
replace: ' clearInterval(this._cycleTimer);',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'tab-reorder-eats-click',
|
||||
guard: 'node --test --test-name-pattern="issue 220" test/space-order.test.mjs',
|
||||
because: 'нулевой порог превращает обычный клик по вкладке в перетаскивание, и '
|
||||
+ 'переключение пространств — основное действие панели — перестаёт работать',
|
||||
patches: [{
|
||||
file: 'src/space-order.ts',
|
||||
find: 'export const TAB_DRAG_THRESHOLD_PX = 4;',
|
||||
replace: 'export const TAB_DRAG_THRESHOLD_PX = 0;',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'tab-reorder-ignores-pointer-type',
|
||||
guard: 'node --test --test-name-pattern="issue 220" test/space-order.test.mjs',
|
||||
because: 'на тач-устройстве вкладки — это навигация View, где тап обязан оставаться тапом; '
|
||||
+ 'решение владельца «Touch editor: not exposed» держится именно этой проверкой',
|
||||
patches: [{
|
||||
file: 'src/space-order.ts',
|
||||
find: " if (ctx.pointerType !== 'mouse') return false;",
|
||||
replace: ' if (false) return false;',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'internal-path-ignores-query',
|
||||
guard: 'node scripts/backend-test-guard.mjs issue_225',
|
||||
|
||||
+157
-2
@@ -261,6 +261,10 @@ import {
|
||||
type OpeningPlacementCore, type OpeningPlacementPreset, type OpeningPlacementType,
|
||||
} from './opening-placement';
|
||||
import { safeStoredColor } from './color';
|
||||
import {
|
||||
applySpaceOrder, canStartTabDrag, markersNeedingPlacement, passedDragThreshold,
|
||||
reorderSpaceIds,
|
||||
} from './space-order';
|
||||
|
||||
const CARD_VERSION = '1.66.0';
|
||||
const DISPLAY_LABEL_KEYS: Record<DeviceDisplayMode, I18nKey> = {
|
||||
@@ -1238,6 +1242,130 @@ class HouseplanCard extends LitElement {
|
||||
}
|
||||
|
||||
/** Direct space tabs use the same motion as swipe/carousel navigation. */
|
||||
// ---- reordering the space tabs (issue #220) ------------------------------
|
||||
//
|
||||
// Mouse only, editors only: the same tabs switch spaces in View, where touch
|
||||
// is a first-class citizen, so a gesture here would compete with that tap.
|
||||
// docs/specs/220-space-tab-reorder.md §4.1, "Touch editor: not exposed".
|
||||
|
||||
private get _canReorderTabs(): boolean {
|
||||
return canStartTabDrag({
|
||||
canEdit: this._canEdit,
|
||||
kiosk: this._kiosk,
|
||||
mode: this._mode,
|
||||
pointerType: 'mouse',
|
||||
spaceCount: this._model.length,
|
||||
fixedFloor: this._hasFixedFloor,
|
||||
});
|
||||
}
|
||||
|
||||
private _tabPointerDown(event: PointerEvent, id: string): void {
|
||||
if (!canStartTabDrag({
|
||||
canEdit: this._canEdit,
|
||||
kiosk: this._kiosk,
|
||||
mode: this._mode,
|
||||
pointerType: event.pointerType,
|
||||
spaceCount: this._model.length,
|
||||
fixedFloor: this._hasFixedFloor,
|
||||
})) return;
|
||||
// A mouse released past the edge of the panel fires neither pointerup nor
|
||||
// pointercancel on any tab, and the gesture would stay stuck mid-drag —
|
||||
// taking the next click with it, since _tabClick swallows clicks that
|
||||
// follow a drag (review CODE-REVIEW-220-r1, M1).
|
||||
//
|
||||
// Capture is the usual answer and this file uses it everywhere, but it is
|
||||
// not a guarantee: the browser grants it only for a live pointer, so a
|
||||
// gesture that starts any other way keeps no capture at all. The window
|
||||
// listener below is what actually closes the gesture; capture merely keeps
|
||||
// the moves flowing to the tab while the button is held.
|
||||
capturePointer(event);
|
||||
this._tabDragRelease = (release: PointerEvent) => this._tabPointerUp(release);
|
||||
window.addEventListener('pointerup', this._tabDragRelease);
|
||||
window.addEventListener('pointercancel', this._tabDragRelease);
|
||||
this._tabDrag = {
|
||||
id, pointerId: event.pointerId, x: event.clientX, y: event.clientY,
|
||||
moved: false, overId: id,
|
||||
};
|
||||
}
|
||||
|
||||
private _tabPointerMove(event: PointerEvent, overId: string): void {
|
||||
const drag = this._tabDrag;
|
||||
if (!drag || drag.pointerId !== event.pointerId) return;
|
||||
if (!drag.moved
|
||||
&& !passedDragThreshold(event.clientX - drag.x, event.clientY - drag.y)) return;
|
||||
// Past the threshold the gesture is a drag: the click that would otherwise
|
||||
// follow is suppressed in _tabClick, and the panel shows where it lands.
|
||||
if (drag.moved && drag.overId === overId) return;
|
||||
this._tabDrag = { ...drag, moved: true, overId };
|
||||
}
|
||||
|
||||
private _tabPointerUp(event: PointerEvent): void {
|
||||
const drag = this._tabDrag;
|
||||
if (drag && drag.pointerId !== event.pointerId) return;
|
||||
this._endTabDrag();
|
||||
if (!drag || !drag.moved) return;
|
||||
this._commitTabOrder(drag.id, drag.overId);
|
||||
}
|
||||
|
||||
/** Drop the gesture and its window listeners, wherever the release happened. */
|
||||
private _endTabDrag(): void {
|
||||
this._tabDrag = null;
|
||||
if (!this._tabDragRelease) return;
|
||||
window.removeEventListener('pointerup', this._tabDragRelease);
|
||||
window.removeEventListener('pointercancel', this._tabDragRelease);
|
||||
this._tabDragRelease = null;
|
||||
}
|
||||
|
||||
/** A click that followed a real drag must not also switch the space. */
|
||||
private _tabClick(id: string): void {
|
||||
if (this._tabDrag?.moved) return;
|
||||
this._pickSpace(id);
|
||||
}
|
||||
|
||||
/**
|
||||
* Write the new order — and, in the same write, the placement that used to
|
||||
* depend on it.
|
||||
*
|
||||
* A marker with neither an explicit space nor an area that names one renders
|
||||
* in whatever space sits first. Reordering would silently hand it to another
|
||||
* space, so the answer it has right now is written down first. This is the
|
||||
* whole reason the two changes may not be split into two saves.
|
||||
*/
|
||||
private _commitTabOrder(movedId: string, targetId: string): void {
|
||||
const cfg = this._serverCfg;
|
||||
if (!cfg || !this._canReorderTabs) return;
|
||||
const ids = this._model.map((space) => space.id);
|
||||
const order = reorderSpaceIds(ids, movedId, targetId);
|
||||
if (order === ids) return;
|
||||
// The area in force, not merely the one stored on the marker: a marker that
|
||||
// binds an HA device inherits its area from the registry, and such a marker
|
||||
// never depended on the order (review CODE-REVIEW-220-r1, H1).
|
||||
const areaById = new Map(
|
||||
this._devices.map((device) => [String(device.id), String(device.area || '')]),
|
||||
);
|
||||
const pinned = markersNeedingPlacement(
|
||||
cfg.markers || [],
|
||||
Object.fromEntries(
|
||||
Object.entries(this._areaToSpace).map(([area, value]) => [area, value.space]),
|
||||
),
|
||||
ids[0] || '',
|
||||
(markerId) => areaById.get(markerId) || '',
|
||||
);
|
||||
if (pinned.length) {
|
||||
const byId = new Map(pinned.map((entry) => [entry.id, entry.space]));
|
||||
for (const marker of cfg.markers || []) {
|
||||
const space = byId.get(String((marker as any).id));
|
||||
if (space) (marker as any).space = space;
|
||||
}
|
||||
}
|
||||
cfg.spaces = applySpaceOrder(cfg.spaces || [], order);
|
||||
this._saveConfig();
|
||||
if (!this._tabOrderWarned) {
|
||||
this._tabOrderWarned = true;
|
||||
this._showToast(this._t('toast.space_order_changed'));
|
||||
}
|
||||
}
|
||||
|
||||
private _pickSpace(id: string): void {
|
||||
if (id === this._space) return;
|
||||
const ids = this._model.map((sp) => sp.id);
|
||||
@@ -1827,6 +1955,17 @@ class HouseplanCard extends LitElement {
|
||||
private _cycleTimer?: number;
|
||||
private _cyclePausedUntil = 0;
|
||||
private _swipeStart: { x: number; y: number; id: number } | null = null;
|
||||
|
||||
/** Live tab reorder: which tab is held, where it started, where it would land. */
|
||||
private _tabDrag: {
|
||||
id: string; pointerId: number; x: number; y: number; moved: boolean; overId: string;
|
||||
} | null = null;
|
||||
|
||||
/** Window-level release handler while a tab is held; see _tabPointerDown. */
|
||||
private _tabDragRelease: ((event: PointerEvent) => void) | null = null;
|
||||
|
||||
/** The positional-`floor` warning is worth saying once, not on every drop. */
|
||||
private _tabOrderWarned = false;
|
||||
private _lastTap = 0;
|
||||
private get _labsIso(): boolean {
|
||||
return this._labs.active.includes('iso');
|
||||
@@ -1916,6 +2055,7 @@ class HouseplanCard extends LitElement {
|
||||
private _holdFired = false;
|
||||
|
||||
static properties = {
|
||||
_tabDrag: { state: true },
|
||||
_hdrH: { state: true },
|
||||
_booting: { state: true },
|
||||
_bootFading: { state: true },
|
||||
@@ -2096,6 +2236,13 @@ class HouseplanCard extends LitElement {
|
||||
this._bootSettling = false;
|
||||
for (const rt of this._activityRt.values()) clearTimeout(rt.timer); // pending activity-window repaints
|
||||
window.removeEventListener('keydown', this._keyHandler);
|
||||
// A tab drag holds window listeners for the length of the gesture. Losing
|
||||
// the card mid-drag — Lovelace rebuilding its tree, the user leaving the
|
||||
// view with the button still down — would leave them alive: the closure
|
||||
// keeps this instance (and its config) from being collected, and the next
|
||||
// pointerup anywhere on the page would make an invisible card write its
|
||||
// order (review CODE-REVIEW-220-r2/r3, F1).
|
||||
this._endTabDrag();
|
||||
clearInterval(this._cycleTimer);
|
||||
clearTimeout(this._kioskDotsTimer);
|
||||
clearTimeout(this._kioskHoldTimer);
|
||||
@@ -15746,8 +15893,16 @@ class HouseplanCard extends LitElement {
|
||||
${navigationSpaces.map(
|
||||
(s) => html`<button
|
||||
data-hp="space-tab" data-id="${s.id}"
|
||||
class="tab ${this._space === s.id ? 'active' : ''}"
|
||||
@click=${() => this._pickSpace(s.id)}
|
||||
class="tab ${this._space === s.id ? 'active' : ''}${
|
||||
this._tabDrag?.moved && this._tabDrag.id === s.id ? ' dragging' : ''}${
|
||||
this._tabDrag?.moved && this._tabDrag.overId === s.id
|
||||
&& this._tabDrag.id !== s.id ? ' droptarget' : ''}"
|
||||
?data-reorderable=${this._canReorderTabs}
|
||||
@pointerdown=${(e: PointerEvent) => this._tabPointerDown(e, s.id)}
|
||||
@pointermove=${(e: PointerEvent) => this._tabPointerMove(e, s.id)}
|
||||
@pointerup=${(e: PointerEvent) => this._tabPointerUp(e)}
|
||||
@pointercancel=${() => this._endTabDrag()}
|
||||
@click=${() => this._tabClick(s.id)}
|
||||
>
|
||||
${s.title}${this._norm && this._canEdit
|
||||
? html`<ha-icon class="tabedit" icon="mdi:cog-outline"
|
||||
|
||||
@@ -287,6 +287,7 @@
|
||||
"toast.ha_disabled_add": "A disabled Home Assistant object cannot be added to the plan. Enable it in Home Assistant first.",
|
||||
"toast.ha_binding_unverified": "The object status could not be verified through the Home Assistant registry. Display and actions are temporarily unavailable.",
|
||||
"toast.markup_needs_server": "Markup is available after the config is moved to the server",
|
||||
"toast.space_order_changed": "Order changed. If any card pins its floor by number, check those panels.",
|
||||
"toast.conflict": "Config was changed in another window — data refreshed, repeat your last action",
|
||||
"toast.cfg_save_failed": "Failed to save config: {err}",
|
||||
"toast.room_overlap": "The outline overlaps room “{name}” — rooms must not overlap",
|
||||
|
||||
@@ -287,6 +287,7 @@
|
||||
"toast.ha_disabled_add": "Деактивированный объект Home Assistant нельзя добавить на план. Сначала активируйте его в Home Assistant.",
|
||||
"toast.ha_binding_unverified": "Статус объекта не удалось подтвердить по реестру Home Assistant. Отображение и действия временно недоступны.",
|
||||
"toast.markup_needs_server": "Разметка доступна после переноса конфига на сервер",
|
||||
"toast.space_order_changed": "Порядок изменён. Если где-то этаж карточки задан номером, проверьте такие панели.",
|
||||
"toast.conflict": "Конфиг изменён в другом окне — данные обновлены, повторите последнее действие",
|
||||
"toast.cfg_save_failed": "Не удалось сохранить конфиг: {err}",
|
||||
"toast.room_overlap": "Контур накладывается на комнату «{name}» — комнаты не должны накладываться",
|
||||
|
||||
@@ -0,0 +1,131 @@
|
||||
/**
|
||||
* Reordering the space tabs.
|
||||
*
|
||||
* The order of `config.spaces` is not decoration: it feeds the marker
|
||||
* placement fallback (`firstSpaceId`), the swipe neighbour and the positional
|
||||
* `floor` of a fixed-floor card. Moving a tab must therefore move nothing else
|
||||
* — see docs/specs/220-space-tab-reorder.md §8.
|
||||
*
|
||||
* Everything here is pure so the rules can be tested without a browser.
|
||||
*/
|
||||
|
||||
/** How far the pointer must travel before a click becomes a drag. */
|
||||
export const TAB_DRAG_THRESHOLD_PX = 4;
|
||||
|
||||
export interface TabDragContext {
|
||||
/** The card allows editing at all. */
|
||||
canEdit: boolean;
|
||||
/** A wall panel never reorders anything. */
|
||||
kiosk: boolean;
|
||||
/** Current mode; reordering lives in the editors only (owner, 2026-08-20). */
|
||||
mode: 'view' | 'plan' | 'devices' | 'decor';
|
||||
/** Pointer that started the gesture. */
|
||||
pointerType: string;
|
||||
/** How many spaces the panel shows right now. */
|
||||
spaceCount: number;
|
||||
/** A card pinned to one floor shows a single tab and must not reorder. */
|
||||
fixedFloor: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a pointerdown on a tab may begin a reorder.
|
||||
*
|
||||
* Touch is deliberately excluded rather than degraded: the tabs live in View
|
||||
* as well, where switching spaces is a fully supported touch interaction, and
|
||||
* any gesture on a tab would compete with the tap that switches. The product
|
||||
* decision is recorded as `Touch editor: not exposed`.
|
||||
*/
|
||||
export function canStartTabDrag(ctx: TabDragContext): boolean {
|
||||
if (!ctx.canEdit || ctx.kiosk || ctx.fixedFloor) return false;
|
||||
if (ctx.mode === 'view') return false;
|
||||
if (ctx.pointerType !== 'mouse') return false;
|
||||
return ctx.spaceCount > 1;
|
||||
}
|
||||
|
||||
/** Has the pointer moved far enough to mean "drag" rather than "click"? */
|
||||
export function passedDragThreshold(dx: number, dy: number): boolean {
|
||||
return Math.hypot(dx, dy) >= TAB_DRAG_THRESHOLD_PX;
|
||||
}
|
||||
|
||||
/**
|
||||
* Move `movedId` so that it sits where `targetId` is, keeping every other id
|
||||
* in its relative order. Returns the same array instance when nothing moves,
|
||||
* so a caller can skip the write without comparing element by element.
|
||||
*/
|
||||
export function reorderSpaceIds(
|
||||
ids: readonly string[], movedId: string, targetId: string,
|
||||
): string[] {
|
||||
const from = ids.indexOf(movedId);
|
||||
const to = ids.indexOf(targetId);
|
||||
if (from < 0 || to < 0 || from === to) return ids as string[];
|
||||
const next = ids.slice();
|
||||
next.splice(from, 1);
|
||||
next.splice(to, 0, movedId);
|
||||
return next;
|
||||
}
|
||||
|
||||
/** Reorder the stored spaces to match `order`; ids missing from it keep their tail. */
|
||||
export function applySpaceOrder<T extends { id?: unknown }>(
|
||||
spaces: readonly T[], order: readonly string[],
|
||||
): T[] {
|
||||
const rank = new Map(order.map((id, index) => [id, index]));
|
||||
// A stable sort keeps unknown ids (there should be none) in their old order
|
||||
// instead of shuffling them by an accidental comparison result.
|
||||
return spaces
|
||||
.map((space, index) => ({ space, index }))
|
||||
.sort((a, b) => {
|
||||
const ra = rank.get(String(a.space?.id)) ?? Number.MAX_SAFE_INTEGER;
|
||||
const rb = rank.get(String(b.space?.id)) ?? Number.MAX_SAFE_INTEGER;
|
||||
return ra - rb || a.index - b.index;
|
||||
})
|
||||
.map((entry) => entry.space);
|
||||
}
|
||||
|
||||
export interface PlacementMarker {
|
||||
id?: unknown;
|
||||
space?: unknown;
|
||||
area?: unknown;
|
||||
removed?: unknown;
|
||||
}
|
||||
|
||||
/**
|
||||
* Markers whose space is decided by the "first space" fallback, and where that
|
||||
* fallback currently lands.
|
||||
*
|
||||
* Such a marker has neither an explicit `space` nor an area that names a
|
||||
* space. Today it renders in whichever space happens to sit first; after a
|
||||
* reorder that would be a different one — the marker would move on its own,
|
||||
* which is the one thing a reorder may never do. Writing the answer it has
|
||||
* right now makes the placement explicit and independent of order for good.
|
||||
*
|
||||
* **The area is not only the marker's own field.** `resolveExplicitMarkerPlacement`
|
||||
* (`devices.ts`) reads `marker.area || <area of the HA device or entity>`, so a
|
||||
* marker that simply binds an existing HA device — the ordinary case, saved
|
||||
* without `area` or `space` — is anchored by the registry and never depended on
|
||||
* the order at all. Judging by `marker.area` alone would classify it as
|
||||
* order-dependent and write it a `space` it never asked for: a field that is
|
||||
* dormant today and moves the marker the day its HA area changes. Hence
|
||||
* `effectiveArea`, which answers with the area actually in force (review
|
||||
* CODE-REVIEW-220-r1, H1).
|
||||
*/
|
||||
export function markersNeedingPlacement(
|
||||
markers: readonly PlacementMarker[],
|
||||
areaToSpace: Readonly<Record<string, string>>,
|
||||
firstSpaceId: string,
|
||||
effectiveArea: (markerId: string) => string = () => '',
|
||||
): { id: string; space: string }[] {
|
||||
if (!firstSpaceId) return [];
|
||||
const out: { id: string; space: string }[] = [];
|
||||
for (const marker of markers) {
|
||||
if (!marker || marker.removed === true) continue;
|
||||
const id = typeof marker.id === 'string' ? marker.id : '';
|
||||
if (!id) continue;
|
||||
const explicit = typeof marker.space === 'string' ? marker.space : '';
|
||||
if (explicit) continue;
|
||||
const own = typeof marker.area === 'string' ? marker.area : '';
|
||||
const area = own || effectiveArea(id) || '';
|
||||
if (area && areaToSpace[area]) continue;
|
||||
out.push({ id, space: firstSpaceId });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
@@ -1692,6 +1692,10 @@ export const cardStyles = css`
|
||||
}
|
||||
.modetab:active { transform: scale(0.97); }
|
||||
.modetab ha-icon { --mdc-icon-size: 15px; }
|
||||
/* issue #220: a tab can be dragged to a new position in the editors */
|
||||
.tab[data-reorderable] { cursor: grab; }
|
||||
.tab.dragging { cursor: grabbing; opacity: 0.55; }
|
||||
.tab.droptarget { box-shadow: inset 2px 0 0 0 var(--primary-color, #03a9f4); }
|
||||
.modetab .closex {
|
||||
--mdc-icon-size: 13px;
|
||||
box-sizing: border-box;
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
import test from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import {
|
||||
TAB_DRAG_THRESHOLD_PX, applySpaceOrder, canStartTabDrag, markersNeedingPlacement,
|
||||
passedDragThreshold, reorderSpaceIds,
|
||||
} from '../test-build/space-order.js';
|
||||
|
||||
const ctx = (over = {}) => ({
|
||||
canEdit: true, kiosk: false, mode: 'plan', pointerType: 'mouse',
|
||||
spaceCount: 3, fixedFloor: false, ...over,
|
||||
});
|
||||
|
||||
// --- AC5: где перетаскивание вообще включается -------------------------------
|
||||
|
||||
test('issue 220 tab drag is available to an editor with a mouse', () => {
|
||||
assert.equal(canStartTabDrag(ctx()), true);
|
||||
assert.equal(canStartTabDrag(ctx({ mode: 'devices' })), true);
|
||||
assert.equal(canStartTabDrag(ctx({ mode: 'decor' })), true);
|
||||
});
|
||||
|
||||
test('issue 220 tab drag is not exposed to touch, View, kiosk or a fixed floor', () => {
|
||||
// Touch is excluded by product decision, not by omission: the same tabs
|
||||
// switch spaces in View, where a tap must stay a tap.
|
||||
assert.equal(canStartTabDrag(ctx({ pointerType: 'touch' })), false);
|
||||
assert.equal(canStartTabDrag(ctx({ pointerType: 'pen' })), false);
|
||||
assert.equal(canStartTabDrag(ctx({ mode: 'view' })), false);
|
||||
assert.equal(canStartTabDrag(ctx({ kiosk: true })), false);
|
||||
assert.equal(canStartTabDrag(ctx({ canEdit: false })), false);
|
||||
assert.equal(canStartTabDrag(ctx({ fixedFloor: true })), false);
|
||||
assert.equal(canStartTabDrag(ctx({ spaceCount: 1 })), false);
|
||||
});
|
||||
|
||||
test('issue 220 a click stays a click until the pointer really travels', () => {
|
||||
assert.equal(passedDragThreshold(0, 0), false);
|
||||
assert.equal(passedDragThreshold(TAB_DRAG_THRESHOLD_PX - 1, 0), false);
|
||||
assert.equal(passedDragThreshold(TAB_DRAG_THRESHOLD_PX, 0), true);
|
||||
assert.equal(passedDragThreshold(0, TAB_DRAG_THRESHOLD_PX), true);
|
||||
});
|
||||
|
||||
// --- AC1: сама перестановка --------------------------------------------------
|
||||
|
||||
test('issue 220 a tab lands where it was dropped and the rest keep their order', () => {
|
||||
assert.deepEqual(reorderSpaceIds(['a', 'b', 'c'], 'c', 'a'), ['c', 'a', 'b']);
|
||||
assert.deepEqual(reorderSpaceIds(['a', 'b', 'c'], 'a', 'c'), ['b', 'c', 'a']);
|
||||
assert.deepEqual(reorderSpaceIds(['a', 'b', 'c'], 'b', 'b'), ['a', 'b', 'c']);
|
||||
assert.deepEqual(reorderSpaceIds(['a', 'b', 'c'], 'b', 'zz'), ['a', 'b', 'c']);
|
||||
});
|
||||
|
||||
test('issue 220 stored spaces follow the new order', () => {
|
||||
const spaces = [{ id: 'a' }, { id: 'b' }, { id: 'c' }];
|
||||
assert.deepEqual(applySpaceOrder(spaces, ['c', 'a', 'b']).map((s) => s.id), ['c', 'a', 'b']);
|
||||
// an id the order does not mention keeps its tail position instead of moving
|
||||
assert.deepEqual(
|
||||
applySpaceOrder([...spaces, { id: 'd' }], ['c', 'a', 'b']).map((s) => s.id),
|
||||
['c', 'a', 'b', 'd'],
|
||||
);
|
||||
});
|
||||
|
||||
// --- AC3: маркеры не двигаются ----------------------------------------------
|
||||
|
||||
test('issue 220 only order-dependent markers are pinned, and to where they are now', () => {
|
||||
const markers = [
|
||||
{ id: 'dangling' }, // no space, no area
|
||||
{ id: 'by-area', area: 'kitchen' }, // area names a space
|
||||
{ id: 'explicit', space: 'f2' }, // already explicit
|
||||
{ id: 'area-unknown', area: 'nowhere' }, // area names nothing
|
||||
{ id: 'gone', removed: true }, // tombstone
|
||||
];
|
||||
const pinned = markersNeedingPlacement(markers, { kitchen: 'f1' }, 'f1');
|
||||
assert.deepEqual(pinned, [
|
||||
{ id: 'dangling', space: 'f1' },
|
||||
{ id: 'area-unknown', space: 'f1' },
|
||||
]);
|
||||
});
|
||||
|
||||
test('issue 220 a marker anchored by its HA area is left alone (review r1 H1)', () => {
|
||||
// The ordinary marker: it binds an HA device and stores neither area nor
|
||||
// space, because resolveExplicitMarkerPlacement reads the area from the
|
||||
// registry. Such a marker never depended on the order, so writing it a space
|
||||
// would plant a field that moves it the day its HA area changes.
|
||||
const markers = [{ id: 'ha-device' }];
|
||||
const areaOf = (id) => (id === 'ha-device' ? 'kitchen' : '');
|
||||
assert.deepEqual(markersNeedingPlacement(markers, { kitchen: 'f1' }, 'f1', areaOf), []);
|
||||
// …while a registry area that names no space leaves the marker order-bound.
|
||||
assert.deepEqual(markersNeedingPlacement(markers, { hall: 'f1' }, 'f1', areaOf), [
|
||||
{ id: 'ha-device', space: 'f1' },
|
||||
]);
|
||||
// The marker's own area still wins over the registry, as in devices.ts.
|
||||
assert.deepEqual(
|
||||
markersNeedingPlacement([{ id: 'ha-device', area: 'hall' }], { hall: 'f2' }, 'f1', areaOf),
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
test('issue 220 pinning writes the space the marker has before the reorder', () => {
|
||||
// The fallback is the FIRST space of the current order. If the write used
|
||||
// the order after the move, the marker would follow the reorder — the very
|
||||
// thing this pinning exists to prevent.
|
||||
const markers = [{ id: 'dangling' }];
|
||||
assert.deepEqual(markersNeedingPlacement(markers, {}, 'garden'), [
|
||||
{ id: 'dangling', space: 'garden' },
|
||||
]);
|
||||
assert.deepEqual(markersNeedingPlacement(markers, {}, ''), []);
|
||||
});
|
||||
@@ -20,6 +20,7 @@
|
||||
"src/virtual-light-state.ts",
|
||||
"src/types.ts",
|
||||
"src/space-geometry.ts",
|
||||
"src/space-order.ts",
|
||||
"src/signing.ts", "src/initial-load.ts", "src/space-model-selection.ts", "src/space-dialog.ts",
|
||||
"src/visual-continuity.ts", "src/mode-transition.ts", "src/pointer-modality.ts",
|
||||
"src/render-device-snapshot.ts",
|
||||
|
||||
Reference in New Issue
Block a user