fix(muya): restore cross-cell table selection + resolve dead TableChessboard picker (Phase G) by Jocs · Pull Request #4435 · marktext/marktext · GitHub
Skip to content

fix(muya): restore cross-cell table selection + resolve dead TableChessboard picker (Phase G) - #4435

Merged
Jocs merged 2 commits into
developfrom
fix/muya-table-chessboard-multicell
Jun 10, 2026
Merged

Jocs merged 2 commits into
developfrom
fix/muya-table-chessboard-multicell

Conversation

@Jocs

@Jocs Jocs commented Jun 10, 2026

Copy link
Copy Markdown
Member

Summary

Two table regressions surfaced by the Phase-G regression audit of the
@muyajs/core migration. Engine work in packages/muya (+ desktop
plumbing). Two themed commits.

Finding 1 — dead TableChessboard grid picker → removed

tableChessboard subscribed to the muya-table-picker event, but
nothing in @muyajs/core or the desktop ever dispatched it. Verified
across the entire upstream marktext/muya tree: the only reference is
the same subscribe — it was dead-on-arrival in the TS rewrite (the
legacy trigger paragraphCtrl.showTablePicker was never ported).

Table dimensions are already picked with full parity:

  • desktop Paragraph → Table opens a rows × columns dialog →
    Muya.createTable;
  • the in-editor quick-insert (/) menu inserts a default table via
    replaceBlockByLabel.

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 in
content.ts, the stale baseFloat comment, and on desktop the
Muya.use(TableChessboard) call + import + muya-core.d.ts decl. A
removal 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 cells
selects that contiguous range (highlight) and copy/cut act on just those
cells:

  • TableCellSelection editor module (attached in editor.init,
    cleaned via detachAllDomEvents): mousedown→anchor; leaving the anchor
    arms 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-selected on each
    cell's ::before overlay. A new mousedown clears the prior selection.
  • Table.cellAt / Table.getSubTableState extract a normalised,
    dimension-clamped rectangle as an ITableState.
  • Clipboard consults the frozen selection first: a single cell
    copies 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).

Cut intentionally clears the selected cells (the core, non-destructive
contract); legacy's empty-row/column auto-removal is deliberately out of
scope.

Tests

  • Unit: cellAt / getSubTableState; drag highlight + copy of a
    sub-rectangle + single-cell plain-text copy + cut clears only the
    selection; chessboard removal-is-clean.
  • E2E (Chromium): real pointer drag + OS clipboard round-trip for copy
    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). Desktop typecheck passes. New Chromium e2e
green; existing table e2e unaffected.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings June 10, 2026 04:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 TableChessboard UI plugin, its styling, exports, and desktop registration; added a regression test to pin its absence.
  • Added TableCellSelection editor 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
File Description
packages/muya/src/ui/tableChessboard/index.ts Deletes the unused chessboard picker implementation.
packages/muya/src/ui/tableChessboard/index.css Deletes chessboard picker styles.
packages/muya/src/ui/baseFloat/index.ts Updates internal comment to remove chessboard reference.
packages/muya/src/ui/tests/tableChessboardRemoved.spec.ts Adds tests asserting the chessboard export/event subscription is gone.
packages/muya/src/index.ts Removes TableChessboard export from package entrypoint.
packages/muya/src/editor/tableCellSelection.ts Introduces cross-cell drag selection controller (highlight + frozen selection state).
packages/muya/src/editor/index.ts Wires TableCellSelection into editor initialization.
packages/muya/src/editor/tests/tableCellSelection.spec.ts Adds unit tests for drag highlight + clipboard copy/cut behavior.
packages/muya/src/clipboard/index.ts Routes copy/cut through frozen table selection when present.
packages/muya/src/block/gfm/table/index.ts Adds cellAt + getSubTableState helpers used by selection/clipboard.
packages/muya/src/block/gfm/table/tests/subTableState.spec.ts Adds unit tests for cellAt/getSubTableState.
packages/muya/src/block/base/content.ts Removes the obsolete float-tool guard for mu-table-picker.
packages/muya/src/assets/styles/blockSyntax.css Adds .mu-table-cell-selected highlight styling.
packages/muya/e2e/tests/editing/table-cell-selection.spec.ts Adds Playwright coverage for real drag + OS clipboard copy/cut.
packages/desktop/src/types/muya-core.d.ts Removes TableChessboard declaration from desktop types.
packages/desktop/src/renderer/src/components/editorWithTabs/editor.vue Removes TableChessboard import and Muya.use(TableChessboard) registration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +88 to +128
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();
};

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@Jocs
Jocs force-pushed the fix/muya-table-chessboard-multicell branch from 2580737 to 27a8c16 Compare June 10, 2026 05:05
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>
@Jocs
Jocs force-pushed the fix/muya-table-chessboard-multicell branch from 27a8c16 to 55b24a1 Compare June 10, 2026 05:07
@github-actions

Copy link
Copy Markdown
Contributor

Build artifacts for PR #4435:

Run: https://github.com/marktext/marktext/actions/runs/27254612809

Artifact Size Link
marktext-macos-x64 256.9 MB Download
marktext-windows-arm64 256.3 MB Download
marktext-linux 556.5 MB Download
marktext-windows-x64 257.6 MB Download
marktext-macos-arm64 246.7 MB Download

@Jocs
Jocs merged commit df98a72 into develop Jun 10, 2026
16 checks passed
Jocs added a commit that referenced this pull request Jun 10, 2026
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>
Jocs added a commit that referenced this pull request Jun 10, 2026
…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>
Jocs added a commit that referenced this pull request Jun 10, 2026
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>
@Jocs
Jocs deleted the fix/muya-table-chessboard-multicell branch June 10, 2026 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants