-
Notifications
You must be signed in to change notification settings - Fork 995
Fix table unmapped click selection #4967
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@platejs/table": patch | ||
| --- | ||
|
|
||
| Fix table-area clicks on unmapped editor regions losing selection and breaking typing or paste. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| /// <reference types="next" /> | ||
| /// <reference types="next/image-types/global" /> | ||
| /// <reference types="next/navigation-types/compat/navigation" /> | ||
| import "./.next/types/routes.d.ts"; | ||
| import "./.next/dev/types/routes.d.ts"; | ||
|
|
||
| // NOTE: This file should not be edited | ||
| // see https://nextjs.org/docs/app/api-reference/config/typescript for more information. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| # 4956 Table Unmapped DOM Selection | ||
|
|
||
| ## Goal | ||
|
|
||
| Fix table/editor clicks on unmapped contentEditable regions so typing or paste cannot desync DOM from Slate or crash `toSlateRange`. | ||
|
|
||
| ## Source | ||
|
|
||
| - GitHub issue: https://github.com/udecode/plate/issues/4956 | ||
| - Discussion: https://github.com/udecode/plate/discussions/4952 | ||
| - Type: bug | ||
| - Browser surface: yes, `/docs/table` | ||
|
|
||
| ## Acceptance | ||
|
|
||
| - Clicking editor padding/unmapped regions with table content does not leave a stale invalid interaction path. | ||
| - Targeted regression coverage proves the selection recovery contract. | ||
| - Browser check proves `/docs/table` no longer throws in the reported flow. | ||
|
|
||
| ## Plan | ||
|
|
||
| - [x] Fetch issue, discussion, comments, and prior local learnings. | ||
| - [x] Classify scope and release requirement. | ||
| - [x] Add failing regression coverage for unmapped table click handling. | ||
| - [x] Implement the highest-leverage fix. | ||
| - [x] Run package verification and lint. | ||
| - [x] Run browser verification on `/docs/table`. | ||
| - [x] Create changeset. | ||
| - [x] Run PR gate. | ||
| - [ ] Create PR and issue comment. | ||
|
|
||
| ## Findings | ||
|
|
||
| - The issue is real table/browser behavior: root/padding/gap clicks can leave `editor.selection` stale, then typing or rich paste can hit an unmapped DOM point. | ||
| - Discussion says suppressing `toSlateRange` alone is insufficient because native text can still render outside the Slate value. | ||
| - Prior learning: DOM selection stack traces can hide ownership bugs; here the linked discussion and current table normalizer point to missing mapped targets around tables plus a root-click guard. | ||
| - `docs/solutions/patterns/critical-patterns.md` does not exist in this checkout. | ||
| - Published package code is likely changing under `packages/`; a changeset is required. | ||
| - Forcing paragraphs around tables is too broad: it shifts path-targeted table operations. The fix guards unmapped root clicks by selecting the closest editable child point. | ||
|
|
||
| ## Progress | ||
|
|
||
| - Loaded `task`, `planning-with-files`, `tdd`, `learnings-researcher`, and `dev-browser`. | ||
| - Created branch `codex/4956-table-unmapped-dom-selection` from current `main`. | ||
| - Red: `bun test packages/table/src/lib/withNormalizeTable.spec.tsx packages/table/src/react/onMouseDownTable.spec.tsx` failed on missing `onMouseDownTable`. | ||
| - Full table suite exposed adjacent-table normalization as path-breaking; removed that approach. | ||
| - Green: `bun test packages/table/src` passed. | ||
| - Package gate passed: `pnpm install`, `pnpm turbo build --filter=./packages/table`, `pnpm turbo typecheck --filter=./packages/table`, `pnpm lint:fix`. | ||
| - Browser gate passed on `http://localhost:3000/docs/table`: root click target was `data-slate-node="value"`, selection moved to a Slate text node, typed text stayed in the table, and no console/page errors fired. Screenshot: `/Users/zbeyens/.dev-browser/tmp/plate-4956-table-unmapped-click-fixed.png`. | ||
| - PR gate passed: `pnpm check`. | ||
| - Compounded the reusable learning in `docs/solutions/ui-bugs/2026-04-25-table-root-clicks-must-recover-selection.md`. |
69 changes: 69 additions & 0 deletions
69
docs/solutions/ui-bugs/2026-04-25-table-root-clicks-must-recover-selection.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| --- | ||
| title: Table root clicks must recover selection | ||
| date: 2026-04-25 | ||
| category: ui-bugs | ||
| module: packages/table | ||
| problem_type: ui_bug | ||
| component: documentation | ||
| symptoms: | ||
| - Clicking an unmapped editor root region near a table can leave browser selection outside Slate text. | ||
| - Typing or pasting after the click can throw `Cannot resolve a Slate point from DOM point`. | ||
| - Text can render outside the Plate value when the native browser edit is not intercepted. | ||
| root_cause: logic_error | ||
| resolution_type: code_fix | ||
| severity: high | ||
| tags: | ||
| - plate | ||
| - slate | ||
| - table | ||
| - selection | ||
| - dom-selection | ||
| - root-click | ||
| - toslaterange | ||
| --- | ||
|
|
||
| # Table root clicks must recover selection | ||
|
|
||
| ## Problem | ||
|
|
||
| On `/docs/table`, clicking editor padding or another unmapped root region near a table could move the browser selection to the contentEditable root instead of a Slate text node. The next typed character or paste could then run through Slate React with a DOM point that has no Slate point. | ||
|
|
||
| ## Symptoms | ||
|
|
||
| - Browser console error: `Cannot resolve a Slate point from DOM point`. | ||
| - The clicked target can be the editor root with `data-slate-node="value"`, not a mapped descendant. | ||
| - Typed text can appear outside the editor value if the native browser edit is allowed to proceed. | ||
|
|
||
| ## What Didn't Work | ||
|
|
||
| - Adding paragraph normalization around adjacent tables was too broad. It shifted path-targeted table transforms, including insert and margin operations that intentionally address a specific top-level table path. | ||
| - Treating any ancestor with `data-slate-node` as mapped was wrong. `data-slate-node="value"` identifies the editor root, not a selectable Slate descendant. | ||
|
|
||
| ## Solution | ||
|
|
||
| Handle table editor `mousedown` events on unmapped root regions. If the click is inside the editable, not on a mapped Slate descendant, and the editor contains a table, prevent the native edit path and select the closest valid Slate point. | ||
|
|
||
| ```ts | ||
| const hasMappedSlateTarget = (target: EventTarget | null) => { | ||
| const slateNode = getTargetElement(target)?.closest('[data-slate-node]'); | ||
|
|
||
| return !!slateNode && slateNode.getAttribute('data-slate-node') !== 'value'; | ||
| }; | ||
| ``` | ||
|
|
||
| The handler then chooses the nearest top-level child by vertical click position and selects either that child start or end point. | ||
|
|
||
| ## Why This Works | ||
|
|
||
| Slate React can only translate DOM points that belong to mapped Slate nodes. Root-level contentEditable clicks can still produce a DOM target inside the editor, but the root is not enough information to resolve a text point. Intercepting those clicks before the browser edits the DOM keeps selection inside Slate and makes typing or paste use the editor transforms. | ||
|
|
||
| ## Prevention | ||
|
|
||
| - Do not count `data-slate-node="value"` as a mapped Slate target in DOM event guards. | ||
| - Prefer explicit selection recovery for root/unmapped clicks over structural normalization that changes document paths. | ||
| - Add browser proof for DOM selection bugs. Unit tests catch the guard, but only the browser proves native typing stays inside the editor value. | ||
|
|
||
| ## Related Issues | ||
|
|
||
| - GitHub issue: https://github.com/udecode/plate/issues/4956 | ||
| - Related learning: `docs/solutions/ui-bugs/2026-03-30-docs-demos-must-clone-reusable-values-per-editor.md` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| /** @jsx jsxt */ | ||
|
|
||
| import { type SlateEditor, createSlateEditor } from 'platejs'; | ||
|
|
||
| import { jsxt } from '@platejs/test-utils'; | ||
|
|
||
| import { getTestTablePlugins } from '../lib/__tests__/getTestTablePlugins'; | ||
| import { onMouseDownTable } from './onMouseDownTable'; | ||
|
|
||
| jsxt; | ||
|
|
||
| const createTableEditor = (input: SlateEditor) => | ||
| createSlateEditor({ | ||
| nodeId: true, | ||
| plugins: getTestTablePlugins(), | ||
| selection: input.selection, | ||
| value: input.children, | ||
| }); | ||
|
|
||
| const setRect = (element: HTMLElement, rect: Partial<DOMRect>) => { | ||
| element.getBoundingClientRect = () => | ||
| ({ | ||
| bottom: 0, | ||
| height: 0, | ||
| left: 0, | ||
| right: 0, | ||
| top: 0, | ||
| width: 0, | ||
| x: 0, | ||
| y: 0, | ||
| toJSON: () => rect, | ||
| ...rect, | ||
| }) as DOMRect; | ||
| }; | ||
|
|
||
| describe('onMouseDownTable', () => { | ||
| it('selects a Slate point when a click lands on the editable root beside a table', () => { | ||
| const input = ( | ||
| <editor> | ||
| <htable> | ||
| <htr> | ||
| <htd> | ||
| <hp>11</hp> | ||
| </htd> | ||
| </htr> | ||
| </htable> | ||
| <hp>after</hp> | ||
| </editor> | ||
| ) as any as SlateEditor; | ||
|
|
||
| const editor = createTableEditor(input); | ||
| const editable = document.createElement('div'); | ||
| const tableDom = document.createElement('table'); | ||
| const paragraphDom = document.createElement('p'); | ||
| const preventDefault = mock(); | ||
| const stopPropagation = mock(); | ||
|
|
||
| editable.setAttribute('data-slate-editor', 'true'); | ||
| editable.setAttribute('data-slate-node', 'value'); | ||
| editable.append(tableDom, paragraphDom); | ||
| setRect(tableDom, { bottom: 40, top: 0 }); | ||
| setRect(paragraphDom, { bottom: 80, top: 41 }); | ||
|
|
||
| spyOn(editor.api, 'toDOMNode').mockImplementation((node) => | ||
| node === editor.children[0] ? tableDom : paragraphDom | ||
| ); | ||
|
|
||
| onMouseDownTable({ | ||
| editor, | ||
| event: { | ||
| button: 0, | ||
| clientY: 30, | ||
| currentTarget: editable, | ||
| defaultPrevented: false, | ||
| preventDefault, | ||
| stopPropagation, | ||
| target: editable, | ||
| }, | ||
| } as any); | ||
|
|
||
| expect(preventDefault).toHaveBeenCalledTimes(1); | ||
| expect(stopPropagation).toHaveBeenCalledTimes(1); | ||
| expect(editor.selection).toEqual({ | ||
| anchor: { offset: 2, path: [0, 0, 0, 0, 0] }, | ||
| focus: { offset: 2, path: [0, 0, 0, 0, 0] }, | ||
| }); | ||
| }); | ||
|
|
||
| it('ignores clicks that already target a mapped Slate node', () => { | ||
| const input = ( | ||
| <editor> | ||
| <htable> | ||
| <htr> | ||
| <htd> | ||
| <hp>11</hp> | ||
| </htd> | ||
| </htr> | ||
| </htable> | ||
| </editor> | ||
| ) as any as SlateEditor; | ||
|
|
||
| const editor = createTableEditor(input); | ||
| const editable = document.createElement('div'); | ||
| const mappedNode = document.createElement('div'); | ||
| const preventDefault = mock(); | ||
|
|
||
| editable.setAttribute('data-slate-editor', 'true'); | ||
| mappedNode.setAttribute('data-slate-node', 'element'); | ||
| editable.append(mappedNode); | ||
|
|
||
| onMouseDownTable({ | ||
| editor, | ||
| event: { | ||
| button: 0, | ||
| clientY: 20, | ||
| currentTarget: editable, | ||
| defaultPrevented: false, | ||
| preventDefault, | ||
| stopPropagation: mock(), | ||
| target: mappedNode, | ||
| }, | ||
| } as any); | ||
|
|
||
| expect(preventDefault).not.toHaveBeenCalled(); | ||
| expect(editor.selection).toEqual(input.selection); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| import type React from 'react'; | ||
|
|
||
| import type { DOMHandler } from 'platejs/react'; | ||
|
|
||
| import { KEYS } from 'platejs'; | ||
|
|
||
| import type { TableConfig } from '../lib'; | ||
|
|
||
| const DATA_SLATE_NODE_SELECTOR = '[data-slate-node]'; | ||
|
|
||
| const getTargetElement = (target: EventTarget | null) => { | ||
| if (target instanceof Element) return target; | ||
| if (target instanceof Node) return target.parentElement; | ||
| }; | ||
|
|
||
| const hasMappedSlateTarget = (target: EventTarget | null) => { | ||
| const slateNode = getTargetElement(target)?.closest(DATA_SLATE_NODE_SELECTOR); | ||
|
|
||
| return !!slateNode && slateNode.getAttribute('data-slate-node') !== 'value'; | ||
| }; | ||
|
|
||
| const getClosestEditableChildPath = ( | ||
| editor: Parameters<DOMHandler<TableConfig>>[0]['editor'], | ||
| clientY: number | ||
| ) => { | ||
| let previousPath: number[] | undefined; | ||
|
|
||
| for (const [index, node] of editor.children.entries()) { | ||
| const path = [index]; | ||
| const domNode = editor.api.toDOMNode(node); | ||
|
|
||
| if (!(domNode instanceof Element)) continue; | ||
|
|
||
| const rect = domNode.getBoundingClientRect(); | ||
|
|
||
| if (clientY < rect.top) { | ||
| return previousPath | ||
| ? { edge: 'end' as const, path: previousPath } | ||
| : { edge: 'start' as const, path }; | ||
| } | ||
|
|
||
| if (clientY <= rect.bottom) { | ||
| const height = rect.height || rect.bottom - rect.top; | ||
|
|
||
| return { | ||
| edge: clientY <= rect.top + height / 2 ? 'start' : 'end', | ||
| path, | ||
| } as const; | ||
| } | ||
|
|
||
| previousPath = path; | ||
| } | ||
|
|
||
| if (previousPath) { | ||
| return { edge: 'end' as const, path: previousPath }; | ||
| } | ||
| }; | ||
|
|
||
| export const onMouseDownTable: DOMHandler<TableConfig, React.MouseEvent> = ({ | ||
| editor, | ||
| event, | ||
| }) => { | ||
| if (event.defaultPrevented || event.button !== 0) return; | ||
| if (hasMappedSlateTarget(event.target)) return; | ||
|
|
||
| const editable = event.currentTarget; | ||
| const target = event.target; | ||
|
|
||
| if (!(editable instanceof HTMLElement)) return; | ||
| if (!(target instanceof Node) || !editable.contains(target)) return; | ||
| if ( | ||
| !editor.api.some({ at: [], match: { type: editor.getType(KEYS.table) } }) | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| const closestPath = getClosestEditableChildPath(editor, event.clientY); | ||
|
|
||
| if (!closestPath) return; | ||
|
|
||
| const point = | ||
| closestPath.edge === 'start' | ||
| ? editor.api.start(closestPath.path) | ||
| : editor.api.end(closestPath.path); | ||
|
|
||
| if (!point) return; | ||
|
|
||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| editor.tf.select(point); | ||
|
|
||
| return true; | ||
| }; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import was changed to
./.next/dev/types/routes.d.ts, which is a dev-mode output path. In build/typegen flows Next writes route types under./.next/types/routes.d.ts, so this committed value becomes mode-dependent and gets rewritten in non-dev workflows, creating deterministic churn and potentially pointing at a missing route-types file when only build artifacts are present.Useful? React with 👍 / 👎.