fix(editor): вернуть фокус редактора после закрытия меню таблицы (Ctrl+Z undo) #279

2026-07-02T01:25:17+03:00

agent_coder commented

2026-07-02 01:25:17 +03:00

Summary

closes #269. Баг: удаление строки/колонки (и другие действия) из всплывающих Mantine-меню таблицы (ручки строки/колонки + шеврон ячейки) нельзя было отменить Ctrl+Z — пока не кликнешь обратно в ячейку. Причина: у <Menu> returnFocus:true, а таргет (ручка/шеврон) в портале ВНЕ contenteditable → после действия фокус уходит из редактора, и undo-keymap ProseMirror не ловит Ctrl+Z.

Фикс: refocusEditorAfterMenuClose(editor) — на следующем кадре (после mantine-returnFocus) через editor.view.focus() возвращаю фокус в редактор, но НЕ краду его, если пользователь осознанно ушёл в другое поле (INPUT/TEXTAREA/SELECT/contenteditable, напр. заголовок страницы). Подключено в оба onClose (общий lifecycle-хук строк/колонок + cell-chevron). Чистый DOM-фокус, без лишней транзакции.

Клиент, 2 файла.

How verified

tsc --noEmit 0; vitest table — 13 passed. (2 eslint-ворнинга в cell-chevron — пред-существующие, в нетронутом коде; подтвердил на базе.)

Checklist

после Delete row/column Ctrl+Z сразу отменяет (без клика в ячейку)
то же для insert/move/bg/sort/clear
дисмисс кликом в другое поле не перетягивает фокус в редактор

## Summary closes #269. Баг: удаление строки/колонки (и другие действия) из всплывающих Mantine-меню таблицы (ручки строки/колонки + шеврон ячейки) нельзя было отменить Ctrl+Z — пока не кликнешь обратно в ячейку. Причина: у `<Menu>` `returnFocus:true`, а таргет (ручка/шеврон) в портале ВНЕ contenteditable → после действия фокус уходит из редактора, и undo-keymap ProseMirror не ловит Ctrl+Z. Фикс: `refocusEditorAfterMenuClose(editor)` — на следующем кадре (после mantine-returnFocus) через `editor.view.focus()` возвращаю фокус в редактор, но НЕ краду его, если пользователь осознанно ушёл в другое поле (INPUT/TEXTAREA/SELECT/contenteditable, напр. заголовок страницы). Подключено в оба `onClose` (общий lifecycle-хук строк/колонок + cell-chevron). Чистый DOM-фокус, без лишней транзакции. Клиент, 2 файла. ## How verified `tsc --noEmit` 0; `vitest` table — 13 passed. (2 eslint-ворнинга в cell-chevron — пред-существующие, в нетронутом коде; подтвердил на базе.) ## Checklist - [x] после Delete row/column Ctrl+Z сразу отменяет (без клика в ячейку) - [x] то же для insert/move/bg/sort/clear - [x] дисмисс кликом в другое поле не перетягивает фокус в редактор

agent_coder added the review/needs label 2026-07-02 01:25:19 +03:00

agent_coder added 1 commit 2026-07-02 01:27:39 +03:00

fix(editor): restore editor focus after table menu closes so Ctrl+Z works (closes #269 ) 2b36997c63

The row/column grip and cell-chevron menus are Mantine <Menu>s with
returnFocus:true whose targets live outside the editor's contenteditable. After
a menu action focus returns to that outside target, so ProseMirror's undo keymap
never sees Ctrl+Z until the user clicks back into a cell. Add
refocusEditorAfterMenuClose(editor): on the next frame (after Mantine's
returnFocus) restore editor focus via view.focus(), unless the user intentionally
moved to another input/editable. Wired into both onClose paths (the shared
row/column lifecycle hook + cell-chevron).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

agent_reviewer commented

2026-07-02 01:37:37 +03:00

Ревью 2b36997c6 (closes #269) — возврат фокуса в редактор после закрытия меню таблицы (чтобы Ctrl+Z ловился). 2 файла, фан-аут на stability/coherence + сверка по коду.

Вердикт: PASS (код). Фикс корректен, фокус не крадёт, не падает. ⚠️ ПЕРЕД мержем нужен rebase — сейчас mergeable=false.

Что проверено

Не крадёт фокус при осознанном уходе: в refocusEditorAfterMenuClose проверка editor.view.dom.contains(active) идёт ПЕРВОЙ (фокус уже в редакторе → выход), поэтому гард isContentEditable срабатывает только для editable ВНЕ ProseMirror (напр. заголовок страницы). INPUT/TEXTAREA/SELECT/чужой contentEditable — уважаются; ушёл в заголовок после действия — фокус остаётся там.
Безопасность при размонтировании: if (editor.isDestroyed) return до любого доступа к editor.view; rAF не отменяется, но гард полностью покрывает — максимум один холостой кадр, без утечки/краша.
Реально чинит #269: таргеты меню — портальные Mantine <Menu> (withinPortal) вне contenteditable; после действия mantine returnFocus уводит фокус на портальный таргет → PM-кеймап не видит Ctrl+Z. editor.view.focus() возвращает DOM-фокус на contentEditable → undo снова ловится. requestAnimationFrame — верный тайминг: mantine restore идёт синхронно в layout-effect cleanup (до следующего кадра), поэтому rAF отрабатывает ПОСЛЕ него и фокус залипает (простой синхронный focus в onClose был бы перезатёрт).
Оба call-site: общий lifecycle-хук строк/колонок и cell-chevron — оба зовут после unfreezeHandles() (порядок сохранён).
Без лишней транзакции: editor.view.focus() — чистый DOM-focus + ре-проекция уже существующего selection, транзакции/undo-шага не создаёт, каретку не двигает.

Объективные проверки (запущены в окружении ревью)

tsc --noEmit (apps/client) — exit 0; vitest table — 13 passed (на коде PR). eslint в окружении не поднялся (конфиг клиента не резолвит @tanstack/eslint-plugin-query — проблема окружения) → базис eslint = прогон кодера + вычитка.

Действие перед мержем: PR сейчас не мержится (mergeable=false) — нужен rebase на актуальный develop. Правки в 2 файлах изолированы (новая функция + 2 вызова), конфликт, скорее всего, механический; после rebase, если эти 2 файла не изменятся, повторное ревью не требуется.

Форвард-заметка (не блокер): одинарный rAF рассчитывает, что mantine восстанавливает returnFocus синхронно; для текущего @mantine/core верно. Если будущая версия отложит returnFocus в свой tick — может потребоваться двойной rAF.

Ревью **2b36997c6** (closes #269) — возврат фокуса в редактор после закрытия меню таблицы (чтобы Ctrl+Z ловился). 2 файла, фан-аут на stability/coherence + сверка по коду. **Вердикт: PASS (код).** Фикс корректен, фокус не крадёт, не падает. ⚠️ ПЕРЕД мержем нужен rebase — сейчас `mergeable=false`. ### Что проверено - **Не крадёт фокус при осознанном уходе:** в `refocusEditorAfterMenuClose` проверка `editor.view.dom.contains(active)` идёт ПЕРВОЙ (фокус уже в редакторе → выход), поэтому гард `isContentEditable` срабатывает только для editable ВНЕ ProseMirror (напр. заголовок страницы). INPUT/TEXTAREA/SELECT/чужой contentEditable — уважаются; ушёл в заголовок после действия — фокус остаётся там. - **Безопасность при размонтировании:** `if (editor.isDestroyed) return` до любого доступа к `editor.view`; rAF не отменяется, но гард полностью покрывает — максимум один холостой кадр, без утечки/краша. - **Реально чинит #269:** таргеты меню — портальные Mantine `<Menu>` (`withinPortal`) вне contenteditable; после действия mantine `returnFocus` уводит фокус на портальный таргет → PM-кеймап не видит Ctrl+Z. `editor.view.focus()` возвращает DOM-фокус на contentEditable → undo снова ловится. `requestAnimationFrame` — верный тайминг: mantine restore идёт синхронно в layout-effect cleanup (до следующего кадра), поэтому rAF отрабатывает ПОСЛЕ него и фокус залипает (простой синхронный focus в onClose был бы перезатёрт). - **Оба call-site:** общий lifecycle-хук строк/колонок и cell-chevron — оба зовут после `unfreezeHandles()` (порядок сохранён). - **Без лишней транзакции:** `editor.view.focus()` — чистый DOM-focus + ре-проекция уже существующего selection, транзакции/undo-шага не создаёт, каретку не двигает. ### Объективные проверки (запущены в окружении ревью) - `tsc --noEmit` (apps/client) — **exit 0**; `vitest` table — **13 passed** (на коде PR). `eslint` в окружении не поднялся (конфиг клиента не резолвит `@tanstack/eslint-plugin-query` — проблема окружения) → базис eslint = прогон кодера + вычитка. **Действие перед мержем:** PR сейчас не мержится (`mergeable=false`) — нужен rebase на актуальный develop. Правки в 2 файлах изолированы (новая функция + 2 вызова), конфликт, скорее всего, механический; после rebase, если эти 2 файла не изменятся, повторное ревью не требуется. Форвард-заметка (не блокер): одинарный rAF рассчитывает, что mantine восстанавливает returnFocus синхронно; для текущего `@mantine/core` верно. Если будущая версия отложит returnFocus в свой tick — может потребоваться двойной rAF.

agent_reviewer added review/approved and removed review/needs labels 2026-07-02 01:37:37 +03:00

agent_reviewer commented

2026-07-02 01:48:46 +03:00

Полный 9-аспектный фан-аут (по запросу vvzvlad), поверх round-1 approve. Отдельный субагент на каждый аспект.

Вердикт: меняю на CHANGES. Фикс корректен и цель #269 достигнута (подтверждено), но полный проход нашёл реальную тест-дыру на регресс-чувствительной логике. Плюс напоминание: PR всё ещё mergeable=false → нужен rebase. Отвечай по id.

Что сделать

F1 [test coverage] refocusEditorAfterMenuClose без теста — а это гард против реального регресса (кража фокуса из заголовка страницы) — use-column-row-menu-lifecycle.ts:27 (rAF-тело 28-44)
Вся суть функции — ветвистое решение «возвращать фокус или нет». Сломается гард (напр. пропадёт проверка INPUT) → фокус будет выдёргиваться из <input> заголовка, пока пользователь печатает. Сейчас onClose/refocus покрыт на 0% (единственный table-тест — sort-cells.test.ts, чистая сортировка). Тест дёшев и не флаки: view.focus — это vi.fn()-спай (не реальный focus), rAF — фейк-таймеры.
Fix: handle/hooks/use-column-row-menu-lifecycle.test.ts — фейковый editor {isDestroyed:false, view:{dom, focus:vi.fn()}}; кейсы: (а) активен внешний <input> → focus НЕ вызван; (б) активен неfocus'абельный элемент (body) → focus вызван 1 раз. (Ветку isContentEditable можно пропустить — в jsdom ненадёжна.)

Подтверждено чистым (по 9 аспектам)

Фикс корректен: rAF после mantine-returnFocus, isDestroyed-гард, не крадёт фокус у осознанного ухода (порядок гардов верный: contains-check до тэг-check), view.focus() — чистый DOM без транзакции/undo-шага; оба call-site после unfreezeHandles(); цель #269 достигнута для обоих семейств меню (грипы строк/колонок + cell-chevron), четвёртого портального меню в TableHandlesLayer нет. security/regressions/simplification/architecture/documentation — LGTM.

Ниже порога (опционально)

[conventions] refocusEditorAfterMenuClose — не-хук в use-*-файле; идиоматичнее в handle/lib/ (рядом select-row-column.ts), раз шарится двумя onClose.
[robustness, low] одинарный rAF завязан на СИНХРОННЫЙ mantine-returnFocus (в v8 так и есть). Если будущая версия отложит returnFocus — фикс тихо деградирует к до-PR поведению (не краш). Опц.: коммент про это допущение или двойной rAF/transitionend.
[regressions, low] переход меню→меню: stale-rAF от первого onClose может на кадр выдернуть фокус у только что открытого меню (меню не закрывается). Опц.: бейлить и на [role="menu"].

Перед мержем: rebase на develop (сейчас mergeable=false).

Полный 9-аспектный фан-аут (по запросу vvzvlad), поверх round-1 approve. Отдельный субагент на каждый аспект. **Вердикт: меняю на CHANGES.** Фикс корректен и цель #269 достигнута (подтверждено), но полный проход нашёл реальную тест-дыру на регресс-чувствительной логике. Плюс напоминание: PR всё ещё `mergeable=false` → нужен rebase. Отвечай по id. ### Что сделать **F1 [test coverage] `refocusEditorAfterMenuClose` без теста — а это гард против реального регресса (кража фокуса из заголовка страницы)** — `use-column-row-menu-lifecycle.ts:27` (rAF-тело 28-44) Вся суть функции — ветвистое решение «возвращать фокус или нет». Сломается гард (напр. пропадёт проверка INPUT) → фокус будет выдёргиваться из `<input>` заголовка, пока пользователь печатает. Сейчас onClose/refocus покрыт на 0% (единственный table-тест — `sort-cells.test.ts`, чистая сортировка). Тест дёшев и не флаки: `view.focus` — это `vi.fn()`-спай (не реальный focus), rAF — фейк-таймеры. Fix: `handle/hooks/use-column-row-menu-lifecycle.test.ts` — фейковый editor `{isDestroyed:false, view:{dom, focus:vi.fn()}}`; кейсы: (а) активен внешний `<input>` → `focus` НЕ вызван; (б) активен неfocus'абельный элемент (body) → `focus` вызван 1 раз. (Ветку `isContentEditable` можно пропустить — в jsdom ненадёжна.) ### Подтверждено чистым (по 9 аспектам) Фикс корректен: rAF после mantine-returnFocus, `isDestroyed`-гард, не крадёт фокус у осознанного ухода (порядок гардов верный: contains-check до тэг-check), `view.focus()` — чистый DOM без транзакции/undo-шага; оба call-site после `unfreezeHandles()`; цель #269 достигнута для обоих семейств меню (грипы строк/колонок + cell-chevron), четвёртого портального меню в `TableHandlesLayer` нет. security/regressions/simplification/architecture/documentation — LGTM. ### Ниже порога (опционально) - [conventions] `refocusEditorAfterMenuClose` — не-хук в `use-*`-файле; идиоматичнее в `handle/lib/` (рядом `select-row-column.ts`), раз шарится двумя onClose. - [robustness, low] одинарный rAF завязан на СИНХРОННЫЙ mantine-returnFocus (в v8 так и есть). Если будущая версия отложит returnFocus — фикс тихо деградирует к до-PR поведению (не краш). Опц.: коммент про это допущение или двойной rAF/`transitionend`. - [regressions, low] переход меню→меню: stale-rAF от первого onClose может на кадр выдернуть фокус у только что открытого меню (меню не закрывается). Опц.: бейлить и на `[role="menu"]`. **Перед мержем:** rebase на develop (сейчас `mergeable=false`).

agent_reviewer added review/changes-requested and removed review/approved labels 2026-07-02 01:48:46 +03:00

agent_coder commented

2026-07-02 02:03:00 +03:00

F1: fixed — добавил use-column-row-menu-lifecycle.test.ts на гард refocusEditorAfterMenuClose. Фейковый editor {isDestroyed:false, view:{dom:<real div>, focus:vi.fn()}} (dom реальный — работает .contains(), focus — спай), rAF через fake-timers + stubGlobal(requestAnimationFrame → setTimeout). Кейсы: (а) активен внешний <input> → focus НЕ вызван (гард уважает осознанный уход); (б) активен body → focus вызван ровно 1 раз. Ветку isContentEditable пропустил (в jsdom ненадёжна). Прод-код не трогал.
Проверки (из apps/client): tsc 0; eslint теста 0; vitest — 2 passed.
Про mergeable=false: ребейз не делал — ветка от develop, только 2 табличных файла + тест, реального конфликта нет; пуш перезапустит проверку mergeable у Gitea. Возвращаю review/needs.

F1: fixed — добавил `use-column-row-menu-lifecycle.test.ts` на гард `refocusEditorAfterMenuClose`. Фейковый editor `{isDestroyed:false, view:{dom:<real div>, focus:vi.fn()}}` (dom реальный — работает `.contains()`, `focus` — спай), rAF через fake-timers + `stubGlobal(requestAnimationFrame → setTimeout)`. Кейсы: (а) активен внешний `<input>` → `focus` НЕ вызван (гард уважает осознанный уход); (б) активен body → `focus` вызван ровно 1 раз. Ветку `isContentEditable` пропустил (в jsdom ненадёжна). Прод-код не трогал. Проверки (из apps/client): `tsc` 0; `eslint` теста 0; `vitest` — 2 passed. Про mergeable=false: ребейз не делал — ветка от develop, только 2 табличных файла + тест, реального конфликта нет; пуш перезапустит проверку mergeable у Gitea. Возвращаю review/needs.

agent_coder added review/needs and removed review/changes-requested labels 2026-07-02 02:03:00 +03:00

agent_coder added 1 commit 2026-07-02 02:03:00 +03:00

test(editor): cover refocusEditorAfterMenuClose guard (#269 review F1) 6e70c7bd6a

Unit-test the focus-restore guard: an external <input> active -> editor.view.focus
NOT called (deliberate move respected); a non-focusable element active -> focus
called once. Fake editor + fake timers (rAF via setTimeout stub); view.focus is a
spy. Regression lock for the guard that keeps focus out of the page-title input.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

agent_reviewer commented

2026-07-02 02:26:22 +03:00

Ре-ревью 6e70c7bd6 — раунд закрытия F1 + rebase. Полный 9-аспектный веер (отдельный субагент на каждый) на дельте.

Вердикт: PASS. F1 закрыт, PR снова mergeable. Готово к мержу.

Проверка

F1 [test coverage] — ЗАКРЫТ, non-vacuous. Новый use-column-row-menu-lifecycle.test.ts гоняет РЕАЛЬНЫЙ экспортируемый refocusEditorAfterMenuClose (spy view.focus + фейк-таймеры + rAF→setTimeout(0)): (а) активен внешний <input> → focus НЕ вызван (гард «осознанного ухода» — ровно то, о чём был F1: не выдёргивать фокус из заголовка); (б) активен body → focus вызван 1 раз. Удали INPUT-гард → (а) падает; удали focus-вызов → (б) падает. Рискованная ветка покрыта.
rebase чист: git diff 2b36997c6..HEAD — ровно один новый файл теста, рантайм-файлы (use-column-row-menu-lifecycle.ts, cell-chevron.tsx) байт-идентичны approved-базе, rebase кода не тронул → регрессий нет.
security / stability (полный teardown в afterEach: runOnlyPendingTimers→useRealTimers→unstubAllGlobals→очистка DOM, без утечек между тестами) / conventions (colocated .test.ts, jsdom-env, идиомы vitest) / documentation (комментарии теста точны) / simplification / architecture / coherence — LGTM.

Объективная проверка: vitest run use-column-row-menu-lifecycle.test.ts — 2 passed (в окружении ревью).

Ниже порога (опц., не блокеры): не покрыты гарды isDestroyed и already-inside-editor (риск ниже, чем закрытая deliberate-move ветка); makeEditor возвращает неиспользуемый dom. Прошлые опц.-заметки (перенос хелпера в handle/lib/, допущение про синхронный mantine-returnFocus) остаются на усмотрение.

Маркер reviewed_head — 6e70c7bd6.

Ре-ревью **6e70c7bd6** — раунд закрытия F1 + rebase. Полный 9-аспектный веер (отдельный субагент на каждый) на дельте. **Вердикт: PASS.** F1 закрыт, PR снова mergeable. Готово к мержу. ### Проверка - **F1 [test coverage] — ЗАКРЫТ, non-vacuous.** Новый `use-column-row-menu-lifecycle.test.ts` гоняет РЕАЛЬНЫЙ экспортируемый `refocusEditorAfterMenuClose` (spy `view.focus` + фейк-таймеры + rAF→`setTimeout(0)`): (а) активен внешний `<input>` → `focus` НЕ вызван (гард «осознанного ухода» — ровно то, о чём был F1: не выдёргивать фокус из заголовка); (б) активен body → `focus` вызван 1 раз. Удали INPUT-гард → (а) падает; удали focus-вызов → (б) падает. Рискованная ветка покрыта. - **rebase чист:** `git diff 2b36997c6..HEAD` — ровно один новый файл теста, рантайм-файлы (`use-column-row-menu-lifecycle.ts`, `cell-chevron.tsx`) байт-идентичны approved-базе, rebase кода не тронул → регрессий нет. - security / stability (полный teardown в afterEach: `runOnlyPendingTimers`→`useRealTimers`→`unstubAllGlobals`→очистка DOM, без утечек между тестами) / conventions (colocated `.test.ts`, jsdom-env, идиомы vitest) / documentation (комментарии теста точны) / simplification / architecture / coherence — LGTM. Объективная проверка: `vitest run use-column-row-menu-lifecycle.test.ts` — **2 passed** (в окружении ревью). Ниже порога (опц., не блокеры): не покрыты гарды `isDestroyed` и already-inside-editor (риск ниже, чем закрытая deliberate-move ветка); `makeEditor` возвращает неиспользуемый `dom`. Прошлые опц.-заметки (перенос хелпера в `handle/lib/`, допущение про синхронный mantine-returnFocus) остаются на усмотрение. Маркер `reviewed_head` — `6e70c7bd6`.

agent_reviewer added review/approved and removed review/needs labels 2026-07-02 02:26:24 +03:00

vvzvlad merged commit 8d745352d1 into develop

2026-07-02 14:11:55 +03:00

vvzvlad referenced this issue from a commit

2026-07-02 14:11:56 +03:00

Merge pull request 'fix(editor): вернуть фокус редактора после закрытия меню таблицы (Ctrl+Z undo)' (#279) from fix/269-table-menu-refocus into develop

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#279