refactor: editor table performance optimization #8574
Conversation
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
Contributor
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughReplaces React-based table row/column drag handles with vanilla-JS factories, adds dropdown content and icon utilities, updates plugins to manage DragHandleInstance elements, refactors table discovery to traverse ProseMirror nodes, and debounces table-update scheduling. (47 words) Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes table editor performance by replacing React-based drag handles with vanilla JavaScript implementations and improving the efficiency of table-finding algorithms.
Changes:
- Refactored drag handles from ReactRenderer to vanilla JS, eliminating React rendering overhead
- Implemented requestAnimationFrame batching for table insert plugin updates to reduce unnecessary rerenders
- Optimized
findAllTablesutility from O(table × nodes) to O(nodes) complexity by iterating through the document once instead of matching DOM elements for each table
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/editor/src/core/extensions/table/plugins/insert-handlers/utils.ts | Optimized findAllTables algorithm to O(n) complexity with single document traversal |
| packages/editor/src/core/extensions/table/plugins/insert-handlers/plugin.ts | Added requestAnimationFrame batching to debounce table updates |
| packages/editor/src/core/extensions/table/plugins/drag-handles/row/plugin.ts | Updated to use vanilla JS drag handles instead of ReactRenderer |
| packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx | Removed React component implementation |
| packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.ts | New vanilla JS implementation of row drag handle with dropdown |
| packages/editor/src/core/extensions/table/plugins/drag-handles/column/plugin.ts | Updated to use vanilla JS drag handles instead of ReactRenderer |
| packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx | Removed React component implementation |
| packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts | New vanilla JS implementation of column drag handle with dropdown |
| packages/editor/src/core/extensions/table/plugins/drag-handles/icons.ts | New shared SVG icon definitions for drag handles |
| packages/editor/src/core/extensions/table/plugins/drag-handles/dropdown-content.ts | New shared dropdown content creation utilities |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/insert-handlers/utils.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.ts
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/editor/src/core/extensions/table/plugins/insert-handlers/plugin.ts (1)
75-105: Guard both setTimeout and RAF callbacks against late execution after plugin destroy.The RAF callback in
scheduleUpdate()can fire afterdestroy()completes, and the initialsetTimeout(updateAllTables, 0)inview()has the same race condition. Add adestroyedflag checked before anyupdateAllTables()call, store and cancel the RAF id on destroy, and store and clear the initial timeout id as well.🐛 Suggested fix
+ let timeoutId: number | null = null; let updateScheduled = false; + let rafId: number | null = null; + let destroyed = false; const scheduleUpdate = () => { - if (updateScheduled) return; + if (updateScheduled || destroyed) return; updateScheduled = true; - requestAnimationFrame(() => { + rafId = requestAnimationFrame(() => { updateScheduled = false; + rafId = null; + if (destroyed) return; updateAllTables(); }); }; return new Plugin({ key: TABLE_INSERT_PLUGIN_KEY, view() { - setTimeout(updateAllTables, 0); + timeoutId = setTimeout(() => { + timeoutId = null; + if (!destroyed) updateAllTables(); + }, 0); return { update(view, prevState) { // Debounce updates using RAF to batch multiple changes if (!prevState.doc.eq(view.state.doc)) { scheduleUpdate(); } }, destroy() { + destroyed = true; + if (timeoutId !== null) { + clearTimeout(timeoutId); + } + if (rafId !== null) { + cancelAnimationFrame(rafId); + } // Clean up all tables tableMap.forEach((_, tableElement) => { cleanupTable(tableElement); }); tableMap.clear(); }, }; }, });
🤖 Fix all issues with AI agents
In
`@packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts`:
- Around line 54-58: The button's vertical padding shifts because the initial
class uses "py-1" while the toggle states use "px-1"; update the class strings
for the toggle/open/close button variants (the element created in drag-handle.ts
referenced as button and any subsequent className assignments around the other
toggle branches at the locations matching lines ~91-94 and ~122-124) so they
consistently use "py-1" (or the same padding utility) across all states,
ensuring no horizontal/vertical padding swap on toggle.
🧹 Nitpick comments (2)
packages/editor/src/core/extensions/table/plugins/insert-handlers/utils.ts (1)
223-251: Skip descending into table children to keep traversal lean.
Returningfalsefromdoc.descendantswhen a table node is found avoids walking every cell, which can be sizable in large tables and aligns with the performance goal.♻️ Proposed refactor
- editor.state.doc.descendants((node, pos) => { - if (node.type.spec.tableRole === "table") { + editor.state.doc.descendants((node, pos) => { + if (node.type.spec.tableRole === "table") { try { const domAtPos = editor.view.domAtPos(pos + 1); let domTable = domAtPos.node; @@ if (domTable instanceof HTMLElement && domTable.tagName === "TABLE") { tables.push({ tableElement: domTable, tableNode: node, tablePos: pos, }); } } catch (error) { // Skip tables that fail to resolve console.error("Error finding table:", error); } + return false; // don't descend into table children } + return true; });packages/editor/src/core/extensions/table/plugins/drag-handles/icons.ts (1)
35-48: ConstraincreateSvgElementto internal icon keys to reduce XSS surface.
SinceiconPathis assigned viainnerHTML, restricting inputs tokeyof typeof DRAG_HANDLE_ICONSprevents accidental use of untrusted strings.♻️ Proposed refactor
-export function createSvgElement(iconPath: string, className = "size-3"): SVGSVGElement { +export type DragHandleIconKey = keyof typeof DRAG_HANDLE_ICONS; + +export function createSvgElement(icon: DragHandleIconKey, className = "size-3"): SVGSVGElement { + const iconPath = DRAG_HANDLE_ICONS[icon]; const svg = document.createElementNS("http://www.w3.org/2000/svg", "svg"); svg.setAttribute("class", className); svg.setAttribute("xmlns", "http://www.w3.org/2000/svg");
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.ts`:
- Around line 331-340: The code assumes rows[row] exists but may be
out-of-bounds; before using rows[row] (used for startTop and later for height
calculations) add a defensive check that row is within 0 <= row < rows.length
and bail out of the mousedown/drag initialization if not (clean up any created
markers and do not attach move/up listeners). Update the logic around
getTableRowNodesInfo, dropIndex, startTop, startYPos, tableElement,
getDropMarker and getRowDragMarker to only proceed when the index is valid, or
compute a safe fallback index (e.g., clamp to rows.length-1) and use optional
chaining/defaults for subsequent accesses to avoid runtime errors.
♻️ Duplicate comments (4)
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts (1)
178-183: Limit dropdown keydown handling to Escape only.
Right now any key press closes the dropdown and blocks the key event, which can swallow editor input. Consider filtering to Escape (or only when focus is within the dropdown).🔧 Suggested fix
- const handleKeyDown = (event: KeyboardEvent) => { - closeDropdown(); - event.preventDefault(); - event.stopPropagation(); - }; + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key !== "Escape" && event.key !== "Esc") return; + closeDropdown(); + event.preventDefault(); + event.stopPropagation(); + };packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.ts (3)
59-60: Icon missingrotate-90class for row handle orientation.The ellipsis icon needs rotation to display horizontally for row handles, matching the original React implementation.
Suggested fix
- const icon = createSvgElement(DRAG_HANDLE_ICONS.ellipsis, "size-4 text-primary"); + const icon = createSvgElement(DRAG_HANDLE_ICONS.ellipsis, "size-4 text-primary rotate-90");
179-184: Keydown handler closes dropdown on any key press.The current implementation closes the dropdown and prevents default for any key pressed anywhere on the page. This can interfere with user interactions outside the dropdown context (e.g., typing in the editor). Consider restricting to Escape key only, or checking if the event target is within the dropdown.
Suggested fix
const handleKeyDown = (event: KeyboardEvent) => { + if (event.key !== "Escape") return; closeDropdown(); - event.preventDefault(); - event.stopPropagation(); };
233-300: Dropdown button event listeners not explicitly cleaned up.Event listeners attached to buttons via
attachDropdownEventListenersare not explicitly removed when the dropdown closes. While removing the parent element and dereferencing it should allow garbage collection, explicit cleanup would be more robust.Consider using event delegation on the dropdown element itself, which is properly cleaned up on removal:
Alternative: Event delegation approach
const attachDropdownEventListeners = (dropdown: HTMLElement) => { - const buttons = dropdown.querySelectorAll("button[data-action]"); const colorPanel = dropdown.querySelector(".color-panel"); const colorChevron = dropdown.querySelector(".color-chevron"); - buttons.forEach((btn) => { - const action = btn.getAttribute("data-action"); - if (!action) return; - - btn.addEventListener("click", (e) => { + dropdown.addEventListener("click", (e) => { + const btn = (e.target as HTMLElement).closest("button[data-action]"); + if (!btn) return; + const action = btn.getAttribute("data-action"); + if (!action) return; + e.preventDefault(); e.stopPropagation(); - // Handle common actions handleDropdownAction(action, editor, closeDropdown, colorPanel, colorChevron); - // Handle row-specific actions switch (action) { // ... rest of switch cases } - }); }); };
Comment on lines
+331
to
+340
| const rows = getTableRowNodesInfo(table, editor); | ||
|
|
||
| let dropIndex = row; | ||
| const startTop = rows[row].top ?? 0; | ||
| const startYPos = e.clientY; | ||
| const tableElement = editor.view.nodeDOM(table.pos); | ||
|
|
||
| const dropMarker = tableElement instanceof HTMLElement ? getDropMarker(tableElement) : null; | ||
| const dragMarker = tableElement instanceof HTMLElement ? getRowDragMarker(tableElement) : null; | ||
|
|
Contributor
There was a problem hiding this comment.
Potential out-of-bounds access on rows[row].
If row index is out of bounds (e.g., row was deleted between handle creation and mousedown), accessing rows[row].top at line 334 and rows[row].height at line 400 will throw or return undefined.
Suggested defensive check
const rows = getTableRowNodesInfo(table, editor);
+ // Validate row index is within bounds
+ if (row < 0 || row >= rows.length) {
+ console.warn("Row index out of bounds:", row);
+ return;
+ }
+
let dropIndex = row;
const startTop = rows[row].top ?? 0;🤖 Prompt for AI Agents
In
`@packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.ts`
around lines 331 - 340, The code assumes rows[row] exists but may be
out-of-bounds; before using rows[row] (used for startTop and later for height
calculations) add a defensive check that row is within 0 <= row < rows.length
and bail out of the mousedown/drag initialization if not (clean up any created
markers and do not attach move/up listeners). Update the logic around
getTableRowNodesInfo, dropIndex, startTop, startYPos, tableElement,
getDropMarker and getRowDragMarker to only proceed when the index is valid, or
compute a safe fallback index (e.g., clamp to rows.length-1) and use optional
chaining/defaults for subsequent accesses to avoid runtime errors.
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts
Show resolved
Hide resolved
Palanikannan1437
previously approved these changes
Jan 22, 2026
Palanikannan1437
previously approved these changes
Jan 22, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts`:
- Around line 54-128: The icon-only column drag handle button lacks ARIA
attributes; update the button element created in this module (button variable)
to include an accessible label (e.g., setAttribute("aria-label", "Column
options") or similar) and manage its expanded state by setting aria-expanded to
the boolean isDropdownOpen when toggling: update toggleDropdown/openDropdown to
set button.setAttribute("aria-expanded", "true") and closeDropdown to set
button.setAttribute("aria-expanded", "false"); ensure the label is present
before appending the icon so screen readers can identify the control and keep
backdropClickHandler/cleanupFloating behavior unchanged.
- Around line 330-335: Guard against reading columns[col] when col is outside
the current columns array: before accessing columns[col] in the drag-handle
logic (where getTableWidthPx and getTableColumnNodesInfo are called and
variables dropIndex, startLeft, startXPos are set), validate col against
columns.length; if columns.length === 0 set dropIndex = 0 and startLeft = 0 (and
avoid indexing), otherwise clamp dropIndex = Math.max(0, Math.min(col,
columns.length - 1)) and then read startLeft from columns[dropIndex].left; this
prevents throws if the table changed between handle creation and mousedown while
preserving expected dropIndex/startLeft behavior.
In
`@packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.ts`:
- Around line 54-128: The icon-only row handle button (variable "button" created
near top) lacks ARIA attributes; add an accessible name and state by setting
button.setAttribute("aria-label", "Row actions") and
button.setAttribute("aria-haspopup", "menu") when creating the element, ensure
the SVG icon created by createSvgElement(DRAG_HANDLE_ICONS.ellipsis, ...) is
aria-hidden (or removed from accessibility tree), and update
button.setAttribute("aria-expanded", String(isDropdownOpen)) as part of the
toggle flow—specifically set aria-expanded="true" in openDropdown() and
aria-expanded="false" in closeDropdown() (and initialize it to "false" on
creation); keep toggling via toggleDropdown() so screen readers receive the
open/closed state.
In `@packages/editor/src/styles/table.css`:
- Around line 60-68: The .open-state rule currently sets only
border-accent-strong which may be a color-only utility and causes border width
to disappear; update the .open-state selector (alongside .default-state) to
include the border utility (e.g., add the plain border utility) so the border
width matches .default-state and the open-state retains consistent border
thickness when toggling.
♻️ Duplicate comments (1)
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.ts (1)
330-335: Validate row index before usingrows[row].
Line 334 usesrows[row]directly; if the row was deleted or indices shifted, this can throw. A defensive bounds check prevents crashes.🛡️ Proposed fix
const rows = getTableRowNodesInfo(table, editor); + if (row < 0 || row >= rows.length) { + console.warn("Row index out of bounds:", row); + return; + } + let dropIndex = row; const startTop = rows[row].top ?? 0;
Comment on lines
+54
to
+128
| const button = document.createElement("button"); | ||
| button.type = "button"; | ||
| button.className = "default-state"; | ||
|
|
||
| // Create icon (Ellipsis lucide icon as SVG) | ||
| const icon = createSvgElement(DRAG_HANDLE_ICONS.ellipsis, "size-4 text-primary"); | ||
|
|
||
| button.appendChild(icon); | ||
| container.appendChild(button); | ||
|
|
||
| // State for dropdown | ||
| let isDropdownOpen = false; | ||
| let dropdownElement: HTMLElement | null = null; | ||
| let backdropElement: HTMLElement | null = null; | ||
| let cleanupFloating: (() => void) | null = null; | ||
| let backdropClickHandler: (() => void) | null = null; | ||
|
|
||
| // Track drag event listeners for cleanup | ||
| let dragListeners: { | ||
| mouseup?: (e: MouseEvent) => void; | ||
| mousemove?: (e: MouseEvent) => void; | ||
| } = {}; | ||
|
|
||
| // Dropdown toggle function | ||
| const toggleDropdown = () => { | ||
| if (isDropdownOpen) { | ||
| closeDropdown(); | ||
| } else { | ||
| openDropdown(); | ||
| } | ||
| }; | ||
|
|
||
| const closeDropdown = () => { | ||
| if (!isDropdownOpen) return; | ||
|
|
||
| isDropdownOpen = false; | ||
|
|
||
| // Reset button to default state | ||
| button.className = "default-state"; | ||
|
|
||
| // Remove dropdown and backdrop | ||
| if (dropdownElement) { | ||
| dropdownElement.remove(); | ||
| dropdownElement = null; | ||
| } | ||
| if (backdropElement) { | ||
| // Remove backdrop listener before removing element | ||
| if (backdropClickHandler) { | ||
| backdropElement.removeEventListener("click", backdropClickHandler); | ||
| backdropClickHandler = null; | ||
| } | ||
| backdropElement.remove(); | ||
| backdropElement = null; | ||
| } | ||
|
|
||
| // Cleanup floating UI (this also removes keydown listener) | ||
| if (cleanupFloating) { | ||
| cleanupFloating(); | ||
| cleanupFloating = null; | ||
| } | ||
|
|
||
| // Remove active dropdown extension | ||
| setTimeout(() => { | ||
| editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.TABLE); | ||
| }, 0); | ||
| }; | ||
|
|
||
| const openDropdown = () => { | ||
| if (isDropdownOpen) return; | ||
|
|
||
| isDropdownOpen = true; | ||
|
|
||
| // Update button to open state | ||
| button.className = "open-state"; | ||
|
|
Contributor
There was a problem hiding this comment.
Add ARIA metadata for the icon-only column handle.
Lines 54–56 create an icon-only button without aria-label/aria-expanded, which makes it inaccessible to screen readers. Add label + state toggles in open/close.
♿ Proposed fix
const button = document.createElement("button");
button.type = "button";
button.className = "default-state";
+ button.setAttribute("aria-label", "Column actions");
+ button.setAttribute("aria-haspopup", "menu");
+ button.setAttribute("aria-expanded", "false");
const closeDropdown = () => {
if (!isDropdownOpen) return;
isDropdownOpen = false;
// Reset button to default state
button.className = "default-state";
+ button.setAttribute("aria-expanded", "false");
...
};
const openDropdown = () => {
if (isDropdownOpen) return;
isDropdownOpen = true;
// Update button to open state
button.className = "open-state";
+ button.setAttribute("aria-expanded", "true");
...
};🤖 Prompt for AI Agents
In
`@packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts`
around lines 54 - 128, The icon-only column drag handle button lacks ARIA
attributes; update the button element created in this module (button variable)
to include an accessible label (e.g., setAttribute("aria-label", "Column
options") or similar) and manage its expanded state by setting aria-expanded to
the boolean isDropdownOpen when toggling: update toggleDropdown/openDropdown to
set button.setAttribute("aria-expanded", "true") and closeDropdown to set
button.setAttribute("aria-expanded", "false"); ensure the label is present
before appending the icon so screen readers can identify the control and keep
backdropClickHandler/cleanupFloating behavior unchanged.
Comment on lines
+330
to
+335
| const tableWidthPx = getTableWidthPx(table, editor); | ||
| const columns = getTableColumnNodesInfo(table, editor); | ||
|
|
||
| let dropIndex = col; | ||
| const startLeft = columns[col].left ?? 0; | ||
| const startXPos = e.clientX; |
Contributor
There was a problem hiding this comment.
Guard against out-of-range column index before reading columns[col].
Line 334 reads columns[col] without bounds checks; if the table changed between handle creation and mousedown, this throws.
🛡️ Proposed fix
const columns = getTableColumnNodesInfo(table, editor);
+ if (col < 0 || col >= columns.length) {
+ console.warn("Column index out of bounds:", col);
+ return;
+ }
+
let dropIndex = col;
const startLeft = columns[col].left ?? 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const tableWidthPx = getTableWidthPx(table, editor); | |
| const columns = getTableColumnNodesInfo(table, editor); | |
| let dropIndex = col; | |
| const startLeft = columns[col].left ?? 0; | |
| const startXPos = e.clientX; | |
| const tableWidthPx = getTableWidthPx(table, editor); | |
| const columns = getTableColumnNodesInfo(table, editor); | |
| if (col < 0 || col >= columns.length) { | |
| console.warn("Column index out of bounds:", col); | |
| return; | |
| } | |
| let dropIndex = col; | |
| const startLeft = columns[col].left ?? 0; | |
| const startXPos = e.clientX; |
🤖 Prompt for AI Agents
In
`@packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.ts`
around lines 330 - 335, Guard against reading columns[col] when col is outside
the current columns array: before accessing columns[col] in the drag-handle
logic (where getTableWidthPx and getTableColumnNodesInfo are called and
variables dropIndex, startLeft, startXPos are set), validate col against
columns.length; if columns.length === 0 set dropIndex = 0 and startLeft = 0 (and
avoid indexing), otherwise clamp dropIndex = Math.max(0, Math.min(col,
columns.length - 1)) and then read startLeft from columns[dropIndex].left; this
prevents throws if the table changed between handle creation and mousedown while
preserving expected dropIndex/startLeft behavior.
Comment on lines
+54
to
+128
| const button = document.createElement("button"); | ||
| button.type = "button"; | ||
| button.className = "default-state"; | ||
|
|
||
| // Create icon (Ellipsis lucide icon as SVG) | ||
| const icon = createSvgElement(DRAG_HANDLE_ICONS.ellipsis, "size-4 text-primary"); | ||
|
|
||
| button.appendChild(icon); | ||
| container.appendChild(button); | ||
|
|
||
| // State for dropdown | ||
| let isDropdownOpen = false; | ||
| let dropdownElement: HTMLElement | null = null; | ||
| let backdropElement: HTMLElement | null = null; | ||
| let cleanupFloating: (() => void) | null = null; | ||
| let backdropClickHandler: (() => void) | null = null; | ||
|
|
||
| // Track drag event listeners for cleanup | ||
| let dragListeners: { | ||
| mouseup?: (e: MouseEvent) => void; | ||
| mousemove?: (e: MouseEvent) => void; | ||
| } = {}; | ||
|
|
||
| // Dropdown toggle function | ||
| const toggleDropdown = () => { | ||
| if (isDropdownOpen) { | ||
| closeDropdown(); | ||
| } else { | ||
| openDropdown(); | ||
| } | ||
| }; | ||
|
|
||
| const closeDropdown = () => { | ||
| if (!isDropdownOpen) return; | ||
|
|
||
| isDropdownOpen = false; | ||
|
|
||
| // Reset button to default state | ||
| button.className = "default-state"; | ||
|
|
||
| // Remove dropdown and backdrop | ||
| if (dropdownElement) { | ||
| dropdownElement.remove(); | ||
| dropdownElement = null; | ||
| } | ||
| if (backdropElement) { | ||
| // Remove backdrop listener before removing element | ||
| if (backdropClickHandler) { | ||
| backdropElement.removeEventListener("click", backdropClickHandler); | ||
| backdropClickHandler = null; | ||
| } | ||
| backdropElement.remove(); | ||
| backdropElement = null; | ||
| } | ||
|
|
||
| // Cleanup floating UI (this also removes keydown listener) | ||
| if (cleanupFloating) { | ||
| cleanupFloating(); | ||
| cleanupFloating = null; | ||
| } | ||
|
|
||
| // Remove active dropdown extension | ||
| setTimeout(() => { | ||
| editor.commands.removeActiveDropbarExtension(CORE_EXTENSIONS.TABLE); | ||
| }, 0); | ||
| }; | ||
|
|
||
| const openDropdown = () => { | ||
| if (isDropdownOpen) return; | ||
|
|
||
| isDropdownOpen = true; | ||
|
|
||
| // Update button to open state | ||
| button.className = "open-state"; | ||
|
|
Contributor
There was a problem hiding this comment.
Add ARIA metadata for the icon-only row handle.
Lines 54–56 create an icon-only button without aria-label/aria-expanded, which makes it inaccessible to screen readers. Add label + state toggles in open/close.
♿ Proposed fix
const button = document.createElement("button");
button.type = "button";
button.className = "default-state";
+ button.setAttribute("aria-label", "Row actions");
+ button.setAttribute("aria-haspopup", "menu");
+ button.setAttribute("aria-expanded", "false");
const closeDropdown = () => {
if (!isDropdownOpen) return;
isDropdownOpen = false;
// Reset button to default state
button.className = "default-state";
+ button.setAttribute("aria-expanded", "false");
...
};
const openDropdown = () => {
if (isDropdownOpen) return;
isDropdownOpen = true;
// Update button to open state
button.className = "open-state";
+ button.setAttribute("aria-expanded", "true");
...
};🤖 Prompt for AI Agents
In
`@packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.ts`
around lines 54 - 128, The icon-only row handle button (variable "button"
created near top) lacks ARIA attributes; add an accessible name and state by
setting button.setAttribute("aria-label", "Row actions") and
button.setAttribute("aria-haspopup", "menu") when creating the element, ensure
the SVG icon created by createSvgElement(DRAG_HANDLE_ICONS.ellipsis, ...) is
aria-hidden (or removed from accessibility tree), and update
button.setAttribute("aria-expanded", String(isDropdownOpen)) as part of the
toggle flow—specifically set aria-expanded="true" in openDropdown() and
aria-expanded="false" in closeDropdown() (and initialize it to "false" on
creation); keep toggling via toggleDropdown() so screen readers receive the
open/closed state.
packages/editor/src/styles/table.css
Outdated
Comment on lines
60
to
68
| @apply py-1 opacity-0 rounded-sm outline-none; | ||
|
|
||
| &.default-state { | ||
| @apply bg-layer-1 hover:bg-layer-1-hover border border-strong-1; | ||
| } | ||
|
|
||
| &.open-state { | ||
| @apply opacity-100! bg-accent-primary border-accent-strong; | ||
| } |
Contributor
There was a problem hiding this comment.
Keep the open-state border width consistent.
Line 67 applies only border-accent-strong; if that utility is color-only, the border width drops when toggling to open state. Consider adding border to keep parity with the default state.
🔧 Proposed fix
&.open-state {
- `@apply` opacity-100! bg-accent-primary border-accent-strong;
+ `@apply` opacity-100! bg-accent-primary border border-accent-strong;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @apply py-1 opacity-0 rounded-sm outline-none; | |
| &.default-state { | |
| @apply bg-layer-1 hover:bg-layer-1-hover border border-strong-1; | |
| } | |
| &.open-state { | |
| @apply opacity-100! bg-accent-primary border-accent-strong; | |
| } | |
| `@apply` py-1 opacity-0 rounded-sm outline-none; | |
| &.default-state { | |
| `@apply` bg-layer-1 hover:bg-layer-1-hover border border-strong-1; | |
| } | |
| &.open-state { | |
| `@apply` opacity-100! bg-accent-primary border border-accent-strong; | |
| } |
🤖 Prompt for AI Agents
In `@packages/editor/src/styles/table.css` around lines 60 - 68, The .open-state
rule currently sets only border-accent-strong which may be a color-only utility
and causes border width to disappear; update the .open-state selector (alongside
.default-state) to include the border utility (e.g., add the plain border
utility) so the border width matches .default-state and the open-state retains
consistent border thickness when toggling.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This PR optimizes editor performance when tables are a part of the content.
ReactRendererwhich introduced a lot of overheads.findAllTablesutility now has a 10x improvement with more efficient logic to look for tables instead of the earlier logic, the complexity of which wasO(table x nodes). The updated logic has the time complexity ofO(nodes).Type of Change
Summary by CodeRabbit
New Features
Performance
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.