Repository navigation
fix(muya): restore cross-cell table selection + resolve dead TableChessboard picker (Phase G) - #4435
Conversation
There was a problem hiding this comment.
Pull request overview
Restores legacy-style cross-cell table cell selection behavior in @muyajs/core (drag-select a rectangular range of table cells and make copy/cut operate on just that sub-rectangle) and removes the dead/unreachable TableChessboard table grid picker plugin that had no dispatch trigger post-migration.
Changes:
- Removed
TableChessboardUI plugin, its styling, exports, and desktop registration; added a regression test to pin its absence. - Added
TableCellSelectioneditor module + table helpers (cellAt,getSubTableState) to support cross-cell drag selection and clipboard integration (copy/cut). - Added unit + e2e coverage for selection highlighting, clipboard copy/cut behavior, and sub-table state extraction.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private _onMouseMove = (event: Event): void => { | ||
| if (!isMouseEvent(event) || this._anchor == null || this._table == null) | ||
| return; | ||
|
|
||
| const position = this._cellPositionFromEvent(event); | ||
| const overSameTable | ||
| = position != null && position.cell.table === this._table; | ||
|
|
||
| // Begin selecting only once the pointer leaves the anchor cell — within | ||
| // a single cell the user is just placing/extending a text caret. | ||
| if ( | ||
| overSameTable | ||
| && position.cell !== this._anchor.cell | ||
| && !this._isSelecting | ||
| ) { | ||
| this._isSelecting = true; | ||
| // Collapse the native text range to a caret in the anchor cell so | ||
| // the rectangle highlight is the only visible *range* selection, | ||
| // while the editor stays focused — copy/cut events fire only on the | ||
| // focused element, so a full blur would break the clipboard. | ||
| this._collapseCaretToAnchor(); | ||
| } | ||
|
|
||
| if (!this._isSelecting) | ||
| return; | ||
|
|
||
| // The browser keeps trying to extend a native text selection during the | ||
| // drag; collapse it again each move so only the cell rectangle shows. | ||
| this._collapseCaretToAnchor(); | ||
|
|
||
| this._focus = overSameTable ? position : this._anchor; | ||
| this._renderHighlight(); | ||
| }; | ||
|
|
||
| private _onMouseUp = (): void => { | ||
| this._detachDragEvents(); | ||
|
|
||
| // No drag happened (a plain click) — leave nothing frozen. | ||
| if (!this._isSelecting) | ||
| this.clear(); | ||
| }; |
There was a problem hiding this comment.
Good catch — fixed. _onMouseMove now sets _focus = null on off-table moves (mirroring legacy tableSelectCellsCtrl's focus = null), and _onMouseUp bails/clears when _focus == null. So releasing the drag outside the table cancels the selection instead of freezing a 1×1 anchor range. Added a unit test (cancels the selection when the pointer is released outside the table) covering it.
The `tableChessboard` plugin subscribed to the `muya-table-picker`
eventCenter event, but nothing in @muyajs/core or the desktop app ever
dispatched it — and across the entire upstream marktext/muya tree the
only reference is the same `subscribe`, so it was dead-on-arrival in the
TS rewrite. The legacy trigger (`paragraphCtrl.showTablePicker`) was
never ported.
Table dimensions are picked elsewhere with full parity:
- desktop "Paragraph -> Table" opens a rows x columns dialog that
drives `Muya.createTable`;
- the in-editor quick-insert (`/`) menu inserts a default table via
`replaceBlockByLabel`.
Wiring the chessboard would add a trigger neither engine ever had
post-migration (new feature work), not restore a regression. Remove the
registered-but-untriggered plugin cleanly instead:
- drop the `TableChessboard` export and the plugin dir from muya;
- drop the dead `'mu-table-picker'` float-tool name guard in
`content.ts` and refresh the stale baseFloat doc-comment;
- drop `Muya.use(TableChessboard)`, its import, and the
`muya-core.d.ts` decl in desktop.
A removal test pins that the export is gone and no listener waits on
`muya-table-picker` after boot.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2580737 to
27a8c16
Compare
Port of legacy `tableSelectCellsCtrl`: dragging a rectangle of table
cells now selects that contiguous range (visual highlight) and copy/cut
operate on just those cells rather than the whole table.
- `TableCellSelection` (editor module, attached in `editor.init`):
mousedown records the anchor cell; once the pointer leaves it the
drag arms, the native text range is collapsed to a caret in the
anchor (keeping the editor focused so clipboard events still fire)
and the anchor->focus rectangle gets the `.mu-table-cell-selected`
class via each cell's `::before` overlay. A new mousedown clears the
previous selection. Listeners go through `attachDOMEvent`, so
`detachAllDomEvents` cleans them up.
- `Table.cellAt(row, column)` / `Table.getSubTableState(...)` extract a
normalised, dimension-clamped rectangle as an `ITableState`.
- `Clipboard` consults the frozen selection first: a single selected
cell copies its plain text (no html), a larger rectangle serialises
to GFM table markdown via `StateToMarkdown`; cut empties just those
cells in place (json-op'd through the content `text` setter).
Covered by unit specs (cellAt / getSubTableState, drag highlight +
copy/cut sub-range) and a Chromium e2e exercising a real pointer drag
and OS clipboard round-trip.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
27a8c16 to
55b24a1
Compare
|
Build artifacts for PR #4435: Run: https://github.com/marktext/marktext/actions/runs/27254612809 |
The `slash menu /table creates a table` e2e encoded the regressed
behaviour (the quick-insert dropped a default table directly). With the
grid picker restored, `/table` now opens the picker and the table is
created at the picked size. Rework the spec accordingly and register the
plugin in the e2e host + examples demo so the picker actually mounts:
- register `TableChessboard` in `e2e/host/main.ts` (the host did not
`Muya.use` it, so the dispatched `muya-table-picker` had no
subscriber) and in `examples/src/main.ts` for dev parity;
- add `floats.tablePicker` + `tablePickerCell(row, column)` selectors;
- the `/table` spec now asserts the picker shows (and no table exists
yet), then picks the (1, 2) cell and asserts a 2×3 table is created
and the picker hides (probing the float wrapper opacity, matching the
emoji-picker convention).
The cross-cell table-selection e2e specs (#4435) are untouched and still
pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sboard deletion + wire trigger) (#4437) * revert(muya): restore the deleted TableChessboard grid picker plugin #4435 deleted `src/ui/tableChessboard/` (the table grid dimension picker) on the premise that it was dead code — registered-but-untriggered because the @muyajs/core TS rewrite never ported the legacy `showTablePicker` trigger. The plugin itself is a real, working muyajs feature; only its trigger was missing. Restore the plugin verbatim from the pre-#4435 tree and re-wire its engine-side hooks so the trigger (added in the next commit) has something to drive: - recover `tableChessboard/index.ts` + `index.css` byte-for-byte; - re-export `TableChessboard` from the package entrypoint; - restore the `'mu-table-picker'` float-tool name guard in `content.ts` (keeps Enter/Tab/arrow keys captured by the float while the picker is open) and the baseFloat `(row, column)` doc-comment; - drop the now-obsolete `tableChessboardRemoved` removal spec; its restoration counterpart lands with the trigger. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * build(desktop): re-register TableChessboard plugin with Muya #4435 also removed the desktop wiring for the table grid picker. Restore it so the renderer registers the plugin again: - `Muya.use(TableChessboard)` + its import in `editor.vue`; - the `TableChessboard` declaration in the `@muyajs/core` type shim. The separate desktop "Paragraph -> Table" menu flow (el-dialog -> `createTable`) is untouched and still works. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(muya): wire the in-editor "table" insert to the grid picker Port legacy muyajs `paragraphCtrl.showTablePicker` into the engine so the in-editor table insert shows the restored hover-grid dimension picker instead of dropping a fixed-size table. Both in-editor "table" entry points — the `/` quick-insert menu and the paragraph front-menu — route through `replaceBlockByLabel`, so a single interception there covers both: `label === 'table'` now emits `muya-table-picker` with a caret position reference (falling back to the block's DOM node) and an `(row, column)` callback that calls `Muya.createTable({ rows: row + 1, columns: column + 1 })`. The restored `TableChessboard` subscribes to that event and invokes the callback on pick, so the picker works end-to-end. The dead `case 'table'` fall-through in the switch is removed. This is the trigger the TS rewrite dropped — in legacy muyajs every in-editor "table" insert (quick-insert, front-menu, and the paragraph menu's `case 'table'`) went through `showTablePicker`; this restores that behaviour. The desktop "Paragraph -> Table" menu keeps its own el-dialog -> `createTable` flow (it intercepts before `updateParagraph`), so it is unaffected. Tests: - `replaceBlockByLabel` for `label: 'table'` dispatches `muya-table-picker` (not a default table) and its handler creates a table at (row + 1) x (column + 1); - `TableChessboard` is exported, registers, subscribes to the event, renders the 6x8 grid, and reports the picked `(row, column)` on click. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(muya): update e2e for the restored table grid picker The `slash menu /table creates a table` e2e encoded the regressed behaviour (the quick-insert dropped a default table directly). With the grid picker restored, `/table` now opens the picker and the table is created at the picked size. Rework the spec accordingly and register the plugin in the e2e host + examples demo so the picker actually mounts: - register `TableChessboard` in `e2e/host/main.ts` (the host did not `Muya.use` it, so the dispatched `muya-table-picker` had no subscriber) and in `examples/src/main.ts` for dev parity; - add `floats.tablePicker` + `tablePickerCell(row, column)` selectors; - the `/table` spec now asserts the picker shows (and no table exists yet), then picks the (1, 2) cell and asserts a 2×3 table is created and the picker hides (probing the float wrapper opacity, matching the emoji-picker convention). The cross-cell table-selection e2e specs (#4435) are untouched and still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(muya): harden table picker footer input (NaN guard) and _select aliasing The footer row/column `keyupHandler` guarded the numeric path with `typeof value === 'number'`, which is also true for `NaN` — typing a non-numeric value parsed to NaN and set the dimension to NaN. Guard with `!Number.isNaN(value)` instead so only real numbers update the count. `showPicker` also aliased `_current` and `_select` to the same object, so footer-input edits mutating `_select[type]` (or the Math.max clamp in `keyupHandler`) also corrupted the immutable `_current` start position and the 'current' grid highlight could drift. Give `_select` its own shallow clone (ICheckerCount is a flat { row, column }). No code relies on `_current === _select` identity — render() destructures each separately and mouseenter reassigns `_select` to a fresh object. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(muya): tear down table picker floats between specs The tableChessboard suite builds real BaseFloat instances, which append a floatBox to <body>, run floating-ui autoUpdate, and observe their container with a ResizeObserver. `afterEach` only restored mocks, so those listeners/observers/nodes leaked across specs. Call `picker.destroy()` (BaseFloat unobserves the ResizeObserver, runs the autoUpdate cleanup, and removes its floatBox) and detach the host / reference nodes the fakes appended to <body>, so nothing leaks into the next spec. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Persist the migration TODO / decision log under .claude/ on develop as the record of the desktop editor engine migration: public-API build-out (phases A–C), the one-shot desktop swap (D), theme migration (E), test cleanup (F), the per-feature verification audit (G, 8 real bugs), parity 15/15, and the table regression fixes (#4435 cross-cell selection, #4437 grid-picker restoration). Phase H (delete packages/muyajs) is deferred to after the 0.20.0 release. The file is force-added; the rest of .claude/ stays gitignored. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Summary
Two table regressions surfaced by the Phase-G regression audit of the
@muyajs/coremigration. Engine work inpackages/muya(+ desktopplumbing). Two themed commits.
Finding 1 — dead
TableChessboardgrid picker → removedtableChessboardsubscribed to themuya-table-pickerevent, butnothing in
@muyajs/coreor the desktop ever dispatched it. Verifiedacross the entire upstream marktext/muya tree: the only reference is
the same
subscribe— it was dead-on-arrival in the TS rewrite (thelegacy trigger
paragraphCtrl.showTablePickerwas never ported).Table dimensions are already picked with full parity:
Muya.createTable;/) menu inserts a default table viareplaceBlockByLabel.Wiring the chessboard would add a trigger neither engine had
post-migration (new feature work), not restore a regression. So the
registered-but-untriggered plugin is removed cleanly: muya export +
plugin dir, the dead
'mu-table-picker'float-tool guard incontent.ts, the stalebaseFloatcomment, and on desktop theMuya.use(TableChessboard)call + import +muya-core.d.tsdecl. Aremoval test pins that the export is gone and no listener waits on the
event after boot.
Finding 2 — cross-cell table selection → restored
Port of legacy
tableSelectCellsCtrl. Dragging a rectangle of cellsselects that contiguous range (highlight) and copy/cut act on just those
cells:
TableCellSelectioneditor module (attached ineditor.init,cleaned via
detachAllDomEvents): mousedown→anchor; leaving the anchorarms the drag, collapses the native range to a caret in the anchor
(editor stays focused so clipboard events still fire), and highlights
the anchor→focus rectangle with
.mu-table-cell-selectedon eachcell's
::beforeoverlay. A new mousedown clears the prior selection.Table.cellAt/Table.getSubTableStateextract a normalised,dimension-clamped rectangle as an
ITableState.Clipboardconsults the frozen selection first: a single cellcopies plain text (no html); a larger rectangle serialises to GFM
table markdown via
StateToMarkdown. Cut empties just those cells inplace (json-op'd through the content
textsetter).Cut intentionally clears the selected cells (the core, non-destructive
contract); legacy's empty-row/column auto-removal is deliberately out of
scope.
Tests
cellAt/getSubTableState; drag highlight + copy of asub-rectangle + single-cell plain-text copy + cut clears only the
selection; chessboard removal-is-clean.
and cut.
Verification
pnpm -C packages/muya lint(0 errors) ·lint:types·lint:css(0 errors) ·
check-circular(clean) ·test(80 files / 637 passed) ·test:spec(1347 passed). Desktoptypecheckpasses. New Chromium e2egreen; existing table e2e unaffected.
🤖 Generated with Claude Code