Skip to content

[WIKI-497] feat: table insert column and row handles #7286

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

Open
wants to merge 14 commits into
base: preview
Choose a base branch
from

Conversation

aaryan610
Copy link
Member

@aaryan610 aaryan610 commented Jun 30, 2025

Description

  • Smoother flow for adding new columns and rows to a table.
  • When hovered over any table, a button to add new columns and one to add new rows will appear
  • Clicking on the button will add one column/row.
  • Dragging the button will add as many columns/rows you want.

Type of Change

  • Feature (non-breaking change which adds functionality)

Media

Screen.Recording.2025-06-30.at.17.22.17.mov

Summary by CodeRabbit

  • New Features

    • Introduced interactive insert buttons for adding or removing table columns and rows directly within the editor.
    • Added visual outlines for selected table cells, enhancing selection clarity.
    • Tables now default to a consistent column width for improved layout.
  • Style

    • Updated table and cell styles, including refined borders, improved selected cell/table highlighting, and repositioned resize handles.
    • Table controls and insert buttons are now visually integrated and appear on table hover.
    • Editor layout now better supports full-width table blocks.
  • Bug Fixes

    • Improved detection and interaction with table elements for drag-and-drop and selection features.
    • Enhanced compatibility and appearance of selected nodes and tables in various editor states.

Copy link
Contributor

coderabbitai bot commented Jun 30, 2025

Walkthrough

This update introduces interactive table insert buttons and enhanced cell selection outlines in the editor. It adds new ProseMirror plugins, utilities for dynamic button creation, and updates CSS for table visuals and controls. Table creation, default column widths, and related command signatures are refactored for consistency and improved user interaction.

Changes

File(s) Change Summary
.../side-menu.ts, .../drag-handle.ts Changed selectors from .table-wrapper to table for logic related to node hovering and drag handle identification.
.../table-cell.ts, .../table-header.ts, .../table/index.ts, .../table.ts, .../create-table.ts Refactored table creation logic to use a default column width constant; updated command signatures and attribute defaults.
.../editor-commands.ts Removed explicit column width from table insertion commands.
.../table-view.tsx Updated root container CSS classes for tables, adding full-width block and adjusting scrollbar size.
.../plugins/insert-handlers/plugin.ts, .../plugins/insert-handlers/utils.ts Introduced new plugin and utilities for interactive table column/row insert buttons with drag/click support.
.../plugins/selection-outline/plugin.ts, .../plugins/selection-outline/utils.ts Added plugin and utilities for outlining selected table cells with dynamic border classes.
.../drag-drop.css, .../table.css, .../variables.css Updated CSS for table wrappers, selection highlights, insert buttons, cell borders, and editor container layout.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Editor
    participant TableInsertPlugin
    participant TableCellSelectionOutlinePlugin

    User->>Editor: Hovers over table
    Editor->>TableInsertPlugin: Detects table, adds insert buttons
    User->>Editor: Clicks or drags insert button
    Editor->>TableInsertPlugin: Handles insert/remove column/row
    User->>Editor: Selects table cells
    Editor->>TableCellSelectionOutlinePlugin: Applies selection outline decorations
Loading

Suggested labels

✨feature, 🛠️refactor, ready_to_merge

Suggested reviewers

  • Palanikannan1437
  • sriramveeraghanta

Poem

A rabbit hopped through tables bright,
Adding buttons left and right—
Click or drag, new rows appear,
Columns grow as you draw near.
Outlines shimmer, cells aglow,
With every change, the features grow!
🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@aaryan610 aaryan610 marked this pull request as draft June 30, 2025 12:19
Copy link

makeplane bot commented Jun 30, 2025

Pull Request Linked with Plane Work Items

Comment Automatically Generated by Plane

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (6)
packages/editor/src/core/extensions/table/plugins/table/insert-handlers/plugin.ts (1)

31-36: Add error handling for DOM operations.

The cleanup operations could fail if DOM elements are removed externally or if there are timing issues.

Add try-catch blocks around DOM operations:

 const cleanupTable = (tableElement: HTMLElement) => {
   const tableInfo = tableMap.get(tableElement);
-  tableInfo?.columnButtonElement?.remove();
-  tableInfo?.rowButtonElement?.remove();
+  try {
+    tableInfo?.columnButtonElement?.remove();
+    tableInfo?.rowButtonElement?.remove();
+  } catch (error) {
+    console.warn('Failed to cleanup table buttons:', error);
+  }
   tableMap.delete(tableElement);
 };
packages/editor/src/styles/table.css (1)

108-122: Consider making positioning values configurable.

The hardcoded positioning values (20px, 50%) work well but could be made configurable for different table sizes or layouts.

Consider using CSS custom properties for key positioning values:

 .table-column-insert-button {
   top: 0;
-  right: -20px;
-  width: 20px;
+  right: calc(-1 * var(--table-insert-button-size, 20px));
+  width: var(--table-insert-button-size, 20px);
   height: 100%;
   transform: translateX(50%);
 }
packages/editor/src/core/extensions/table/plugins/table/insert-handlers/utils.ts (4)

60-63: Add cursor style for better drag feedback.

The code sets userSelect but doesn't set a cursor style during drag. Consider adding a cursor to indicate the drag state.

 // Visual feedback
 button.classList.add("dragging");
 document.body.style.userSelect = "none";
+document.body.style.cursor = "ew-resize";

156-159: Add cursor style for vertical drag feedback.

Similar to the column button, add a cursor style to indicate vertical dragging.

 // Visual feedback
 button.classList.add("dragging");
 document.body.style.userSelect = "none";
+document.body.style.cursor = "ns-resize";

20-210: Consider refactoring common drag logic.

The createColumnInsertButton and createRowInsertButton functions share significant code duplication. Consider extracting the common drag handling logic into a reusable function to improve maintainability.


228-234: Simplify navigation logic with optional chaining.

The nested conditions can be simplified using optional chaining for better readability.

 // Navigate to find the table element
-while (domTable && domTable.parentNode && domTable.nodeType !== Node.ELEMENT_NODE) {
+while (domTable?.parentNode && domTable.nodeType !== Node.ELEMENT_NODE) {
   domTable = domTable.parentNode;
 }

-while (domTable && domTable.parentNode && (domTable as HTMLElement).tagName !== "TABLE") {
+while (domTable?.parentNode && (domTable as HTMLElement).tagName !== "TABLE") {
   domTable = domTable.parentNode;
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2f6923f and 369be93.

📒 Files selected for processing (16)
  • packages/editor/src/core/extensions/side-menu.ts (1 hunks)
  • packages/editor/src/core/extensions/table/plugins/table/insert-handlers/plugin.ts (1 hunks)
  • packages/editor/src/core/extensions/table/plugins/table/insert-handlers/utils.ts (1 hunks)
  • packages/editor/src/core/extensions/table/plugins/table/selection-outline/plugin.ts (1 hunks)
  • packages/editor/src/core/extensions/table/plugins/table/selection-outline/utils.ts (1 hunks)
  • packages/editor/src/core/extensions/table/table-cell.ts (3 hunks)
  • packages/editor/src/core/extensions/table/table-header.ts (2 hunks)
  • packages/editor/src/core/extensions/table/table/index.ts (1 hunks)
  • packages/editor/src/core/extensions/table/table/table-view.tsx (1 hunks)
  • packages/editor/src/core/extensions/table/table/table.ts (5 hunks)
  • packages/editor/src/core/extensions/table/table/utilities/create-table.ts (2 hunks)
  • packages/editor/src/core/helpers/editor-commands.ts (1 hunks)
  • packages/editor/src/core/plugins/drag-handle.ts (3 hunks)
  • packages/editor/src/styles/drag-drop.css (2 hunks)
  • packages/editor/src/styles/table.css (1 hunks)
  • packages/editor/src/styles/variables.css (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
packages/editor/src/core/extensions/table/table/table-view.tsx (1)
Learnt from: lifeiscontent
PR: makeplane/plane#7164
File: packages/ui/.storybook/main.ts:24-47
Timestamp: 2025-06-04T16:22:44.344Z
Learning: In packages/ui/.storybook/main.ts, the webpackFinal function intentionally overrides the CSS loader strategy by finding and replacing existing CSS rules. This is a temporary workaround for a known upstream issue in Storybook's CSS handling that has been communicated to the Storybook maintainers. The current implementation should not be changed until the upstream issue is resolved.
packages/editor/src/styles/drag-drop.css (1)
Learnt from: vineetk13
PR: makeplane/plane#6391
File: web/styles/react-day-picker.css:249-282
Timestamp: 2025-01-17T05:17:51.953Z
Learning: In the date range picker's CSS, left/right positioning for cell background effects (like in `.rdp-range_start::before`, `.rdp-range_middle::before`, `.rdp-range_end::before`) should use physical properties instead of logical properties, as these create visual effects that should remain consistent regardless of text direction.
packages/editor/src/styles/variables.css (1)
Learnt from: lifeiscontent
PR: makeplane/plane#7164
File: packages/ui/.storybook/main.ts:24-47
Timestamp: 2025-06-04T16:22:44.344Z
Learning: In packages/ui/.storybook/main.ts, the webpackFinal function intentionally overrides the CSS loader strategy by finding and replacing existing CSS rules. This is a temporary workaround for a known upstream issue in Storybook's CSS handling that has been communicated to the Storybook maintainers. The current implementation should not be changed until the upstream issue is resolved.
packages/editor/src/styles/table.css (1)
Learnt from: vineetk13
PR: makeplane/plane#6391
File: web/styles/react-day-picker.css:249-282
Timestamp: 2025-01-17T05:17:51.953Z
Learning: In the date range picker's CSS, left/right positioning for cell background effects (like in `.rdp-range_start::before`, `.rdp-range_middle::before`, `.rdp-range_end::before`) should use physical properties instead of logical properties, as these create visual effects that should remain consistent regardless of text direction.
🧬 Code Graph Analysis (5)
packages/editor/src/core/extensions/table/table-header.ts (1)
packages/editor/src/core/extensions/table/table/index.ts (1)
  • DEFAULT_COLUMN_WIDTH (3-3)
packages/editor/src/core/extensions/table/plugins/table/insert-handlers/plugin.ts (1)
packages/editor/src/core/extensions/table/plugins/table/insert-handlers/utils.ts (4)
  • TableInfo (12-18)
  • createColumnInsertButton (20-114)
  • createRowInsertButton (116-210)
  • findAllTables (212-254)
packages/editor/src/core/extensions/table/table-cell.ts (2)
packages/editor/src/core/extensions/table/table/index.ts (1)
  • DEFAULT_COLUMN_WIDTH (3-3)
packages/editor/src/core/extensions/table/plugins/table/selection-outline/plugin.ts (1)
  • TableCellSelectionOutlinePlugin (14-58)
packages/editor/src/core/extensions/table/plugins/table/selection-outline/plugin.ts (1)
packages/editor/src/core/extensions/table/plugins/table/selection-outline/utils.ts (1)
  • getCellBorderClasses (50-75)
packages/editor/src/core/extensions/table/table/table.ts (3)
packages/editor/src/core/extensions/table/table/index.ts (2)
  • Table (1-1)
  • DEFAULT_COLUMN_WIDTH (3-3)
packages/editor/src/core/extensions/table/table/utilities/create-table.ts (1)
  • createTable (15-45)
packages/editor/src/core/extensions/table/plugins/table/insert-handlers/plugin.ts (1)
  • TableInsertPlugin (8-87)
🪛 Biome (1.9.4)
packages/editor/src/core/extensions/table/plugins/table/insert-handlers/utils.ts

[error] 228-228: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


[error] 232-232: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (35)
packages/editor/src/styles/variables.css (1)

182-193: CSS refactoring for full-width block support looks well-implemented.

The conditional styling approach cleanly separates regular constrained content from full-width blocks like tables. The use of CSS custom properties and calculated padding ensures responsive behavior across different layout widths.

packages/editor/src/core/extensions/table/plugins/table/selection-outline/utils.ts (2)

9-48: Well-designed utility function with clear logic.

The coordinate conversion logic is correct and the boundary checks properly handle table edges. The extensive comments make the math operations easy to follow.


50-75: Border class logic correctly implements selection outline behavior.

The function properly determines which borders to show by checking if adjacent cells are selected. This creates a clean outline around the selection area without internal borders.

packages/editor/src/core/extensions/table/table/index.ts (1)

3-3: Good addition of a centralized default column width constant.

The value of 150px is reasonable for default table column width and centralizing this constant promotes consistency across the table extension.

packages/editor/src/core/extensions/side-menu.ts (1)

134-137: Consistent selector update aligns with table refactoring.

The change from .table-wrapper to table selector is consistent with the broader refactoring mentioned in the AI summary and maintains the same positioning logic.

packages/editor/src/core/extensions/table/table/table-view.tsx (1)

390-390: Class updates properly integrate with the new layout system.

Adding editor-full-width-block enables the CSS styling defined in variables.css for full-width table layout, and the scrollbar size change is a reasonable UI refinement.

packages/editor/src/core/helpers/editor-commands.ts (1)

112-113: LGTM! Good refactoring to use centralized default column width.

The removal of the hardcoded columnWidth: 150 is a positive change that aligns with the introduction of the DEFAULT_COLUMN_WIDTH constant. This improves maintainability by centralizing the default value.

packages/editor/src/core/extensions/table/table-header.ts (2)

4-6: LGTM! Import properly added for centralized constant.

The import of DEFAULT_COLUMN_WIDTH is correctly structured and aligns with the consistent column width standardization across table nodes.


31-31: LGTM! Consistent default column width implementation.

Changing the default from null to [DEFAULT_COLUMN_WIDTH] ensures consistent column sizing across table header and cell nodes, which is essential for proper table rendering.

packages/editor/src/styles/drag-drop.css (2)

38-38: LGTM! Proper exclusion of table wrapper from general selected node styles.

Adding :not(.table-wrapper) prevents potential conflicts between general ProseMirror selected node styles and specific table wrapper styling, which is the correct approach for specialized table UI components.


64-65: LGTM! Consistent styling treatment for table wrappers.

Including .table-wrapper alongside image components ensures consistent visual treatment for block-level elements that require zero horizontal offset and specialized background colors.

packages/editor/src/core/plugins/drag-handle.ts (2)

19-19: LGTM! Updated table selector for consistency.

The change from .table-wrapper to table aligns with the broader table handling standardization. Using the HTML table element directly is more semantic than relying on CSS classes.


93-93: LGTM! Consistent table element detection.

The update to use table element matching is consistent with the selector changes and provides more reliable table detection.

packages/editor/src/core/extensions/table/table-cell.ts (3)

4-7: LGTM! Proper imports for new functionality.

The imports for both TableCellSelectionOutlinePlugin and DEFAULT_COLUMN_WIDTH are correctly structured and support the new table selection features and consistent column width handling.


32-32: LGTM! Consistent default column width across table nodes.

Using [DEFAULT_COLUMN_WIDTH] as the default ensures consistent column sizing between TableCell and TableHeader nodes, which is essential for proper table layout.


53-55: LGTM! Proper integration of table cell selection outline plugin.

The addProseMirrorPlugins method correctly integrates the TableCellSelectionOutlinePlugin, which will provide visual feedback for selected table cells. The plugin initialization with this.editor follows the standard ProseMirror extension pattern.

packages/editor/src/core/extensions/table/plugins/table/insert-handlers/plugin.ts (1)

11-29: LGTM! Well-structured table setup logic.

The setup logic correctly checks for existing buttons before creating new ones and properly updates the table map.

packages/editor/src/core/extensions/table/plugins/table/selection-outline/plugin.ts (3)

20-25: LGTM! Efficient early returns for performance.

The early returns for non-editable editor, missing table, and unchanged document/selection are well-implemented optimizations.


34-46: LGTM! Well-structured cell selection processing.

The two-pass approach (first collecting selected cells, then adding decorations) is efficient and ensures all adjacency information is available when determining border classes.


53-57: LGTM! Clean plugin props structure.

The decorations prop correctly retrieves and returns the plugin state decorations.

packages/editor/src/core/extensions/table/table/table.ts (4)

26-26: LGTM! Proper plugin import.

The TableInsertPlugin import is correctly added to the local imports section.


47-47: LGTM! Clean API simplification.

Removing the columnWidth option from the public API simplifies the interface while using a sensible default internally.


118-124: LGTM! Improved function call syntax.

The refactored createTable call using an options object is more readable and maintainable than positional arguments.


270-270: LGTM! Proper plugin registration.

The TableInsertPlugin is correctly added to the ProseMirror plugins list.

packages/editor/src/core/extensions/table/table/utilities/create-table.ts (2)

6-13: LGTM! Well-defined Props interface.

The Props type clearly defines all required parameters with appropriate types, improving type safety and maintainability.


15-16: LGTM! Clean function signature refactoring.

The refactoring from multiple parameters to a single props object improves maintainability and makes the function easier to call with named parameters.

packages/editor/src/styles/table.css (4)

3-3: LGTM! Added padding for insert buttons.

The padding-bottom: 30px provides necessary space for the row insert button positioned below the table.


27-55: LGTM! Well-implemented selection outline styles.

The selectedCell styling with conditional borders using pseudo-elements provides clean visual feedback for cell selection without affecting layout.


77-106: LGTM! Comprehensive insert button styling.

The insert button styles include all necessary states (default, hover, dragging) with smooth transitions and proper visual feedback.


137-144: LGTM! Improved column resize handle positioning.

The updated positioning with right: -1px and top: -1px ensures the resize handle aligns properly with table borders.

packages/editor/src/core/extensions/table/plugins/table/insert-handlers/utils.ts (5)

12-18: Well-structured type definition.

The TableInfo type appropriately captures the essential table data with optional UI elements.


256-261: Good defensive programming.

The function properly refreshes table info and includes a fallback to handle edge cases.


264-354: Well-implemented column operations.

The column manipulation functions properly handle edge cases, check for empty content, and prevent removing the last column. The empty checking logic is thorough.


319-337: Thorough empty cell detection.

The function correctly handles various content types and edge cases when determining if a cell is empty.


357-426: Consistent row operations implementation.

The row manipulation functions mirror the column operations appropriately, with proper bounds checking and empty validation.

@aaryan610 aaryan610 marked this pull request as ready for review July 1, 2025 11:46
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (5)
packages/editor/src/core/extensions/table/plugins/selection-outline/plugin.ts (1)

19-25: Consider performance optimization for large tables.

The plugin recalculates decorations on every selection change, which could impact performance with large tables. The early return logic is good, but consider adding debouncing or memoization for complex table interactions.

 apply(tr, prev, oldState, newState) {
   if (!editor.isEditable) return {};
   const table = findParentNode((node) => node.type.spec.tableRole === "table")(newState.selection);
   const hasDocChanged = tr.docChanged || !newState.selection.eq(oldState.selection);
   if (!table || !hasDocChanged) {
     return table === undefined ? {} : prev;
   }
+  
+  // Early return if selection type hasn't changed and is not CellSelection
+  if (!(newState.selection instanceof CellSelection) && !(oldState.selection instanceof CellSelection)) {
+    return {};
+  }
packages/editor/src/core/extensions/table/plugins/selection-outline/utils.ts (1)

18-23: Consider optimizing the indexOf operation for large tables.

The indexOf operation has O(n) complexity which could impact performance with large tables. Consider using a Map for faster lookups if this becomes a bottleneck.

 const getAdjacentCellPositions = (
   cellStart: number,
-  tableMap: TableMap
+  tableMap: TableMap,
+  cellIndexMap?: Map<number, number>
 ): { top?: number; bottom?: number; left?: number; right?: number } => {
   const { width, height } = tableMap;
   
-  const cellIndex = tableMap.map.indexOf(cellStart);
+  const cellIndex = cellIndexMap ? 
+    cellIndexMap.get(cellStart) ?? -1 : 
+    tableMap.map.indexOf(cellStart);
packages/editor/src/core/extensions/table/plugins/insert-handlers/utils.ts (3)

232-238: Use optional chaining for cleaner code.

The null checks can be simplified using optional chaining.

Apply this diff to use optional chaining:

-        while (domTable && domTable.parentNode && domTable.nodeType !== Node.ELEMENT_NODE) {
+        while (domTable?.parentNode && domTable.nodeType !== Node.ELEMENT_NODE) {
           domTable = domTable.parentNode;
         }

-        while (domTable && domTable.parentNode && (domTable as HTMLElement).tagName !== "TABLE") {
+        while (domTable?.parentNode && (domTable as HTMLElement).tagName !== "TABLE") {
           domTable = domTable.parentNode;
         }

323-341: Consider enhancing the empty cell detection logic.

The current implementation might consider cells with only formatting marks as non-empty, which could prevent deletion of visually empty columns/rows.

Consider checking for actual text content:

 const isCellEmpty = (cell: ProseMirrorNode | null | undefined): boolean => {
   if (!cell || cell.content.size === 0) {
     return true;
   }

   // Check if cell has any non-empty content
   let hasContent = false;
   cell.content.forEach((node) => {
-    if (node.type.name === "paragraph") {
-      if (node.content.size > 0) {
-        hasContent = true;
-      }
-    } else if (node.content.size > 0 || node.isText) {
+    if (node.isText && node.text?.trim()) {
+      hasContent = true;
+    } else if (node.type.name !== "paragraph" && !node.isInline && node.content.size > 0) {
+      // Check for non-paragraph block content
       hasContent = true;
+    } else if (node.content.size > 0) {
+      // Recursively check nested content
+      node.content.forEach((child) => {
+        if (child.isText && child.text?.trim()) {
+          hasContent = true;
+        }
+      });
     }
   });

   return !hasContent;
 };

216-258: Consider caching or optimizing table lookups for large documents.

The current implementation walks the entire document for each table element, which could be slow for documents with many tables. While likely acceptable for typical use cases, you might want to consider optimization strategies for larger documents.

Potential optimizations:

  • Cache the table positions and update incrementally on document changes
  • Use ProseMirror's view decorations or node views to maintain table-to-position mapping
  • Consider using editor.view.nodeDOM if available to directly map nodes to DOM elements
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 369be93 and ace33f6.

📒 Files selected for processing (7)
  • packages/editor/src/core/extensions/table/plugins/insert-handlers/plugin.ts (1 hunks)
  • packages/editor/src/core/extensions/table/plugins/insert-handlers/utils.ts (1 hunks)
  • packages/editor/src/core/extensions/table/plugins/selection-outline/plugin.ts (1 hunks)
  • packages/editor/src/core/extensions/table/plugins/selection-outline/utils.ts (1 hunks)
  • packages/editor/src/core/extensions/table/table-cell.ts (3 hunks)
  • packages/editor/src/core/extensions/table/table/table.ts (5 hunks)
  • packages/editor/src/styles/table.css (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/editor/src/core/extensions/table/table/table.ts
  • packages/editor/src/core/extensions/table/table-cell.ts
  • packages/editor/src/styles/table.css
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/editor/src/core/extensions/table/plugins/insert-handlers/plugin.ts (2)
packages/editor/src/core/extensions/table/plugins/insert-handlers/utils.ts (4)
  • TableInfo (12-18)
  • createColumnInsertButton (20-116)
  • createRowInsertButton (118-214)
  • findAllTables (216-258)
packages/i18n/src/store/index.ts (1)
  • t (211-232)
packages/editor/src/core/extensions/table/plugins/selection-outline/plugin.ts (1)
packages/editor/src/core/extensions/table/plugins/selection-outline/utils.ts (1)
  • getCellBorderClasses (50-75)
🪛 Biome (1.9.4)
packages/editor/src/core/extensions/table/plugins/insert-handlers/utils.ts

[error] 232-232: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


[error] 236-236: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🔇 Additional comments (13)
packages/editor/src/core/extensions/table/plugins/selection-outline/plugin.ts (6)

1-6: LGTM: Clean imports and dependencies.

The imports are well-organized and include all necessary dependencies from TipTap and ProseMirror. The local utility import follows a logical structure.


8-12: LGTM: Well-defined plugin state and key.

The plugin state type is properly defined with optional decorations, and the plugin key follows a clear naming convention.


27-28: LGTM: Proper type checking for CellSelection.

The instanceof check ensures the plugin only processes cell selections, which is the correct approach.


34-38: LGTM: Efficient two-pass approach for decoration calculation.

The two-pass approach (first collecting selected cells, then calculating decorations) is efficient and necessary for the border logic to work correctly.


41-46: LGTM: Proper decoration creation with appropriate classes.

The decoration creation logic correctly calculates cell positions relative to the table and applies the appropriate CSS classes for border visualization.


53-57: LGTM: Standard ProseMirror plugin props implementation.

The props implementation correctly exposes the decorations from the plugin state for rendering.

packages/editor/src/core/extensions/table/plugins/selection-outline/utils.ts (5)

1-1: LGTM: Appropriate import for table utilities.

The import correctly brings in the TableMap type from ProseMirror tables.


3-12: LGTM: Excellent documentation and function signature.

The JSDoc comment provides clear documentation of the function's purpose, parameters, and return value. The function signature with optional properties is well-designed.


25-48: LGTM: Correct coordinate calculation and boundary checks.

The coordinate calculation logic is mathematically sound:

  • Row calculation using integer division is correct
  • Column calculation using modulo is correct
  • Boundary checks prevent out-of-bounds access
  • The conditional logic properly handles table edges

50-75: LGTM: Solid border class determination logic.

The function correctly determines which borders to show based on adjacent cell selection state. The logic ensures that borders are only shown on edges where adjacent cells are either non-existent or not selected, which is the correct behavior for selection outlines.


9-48: I need more context on how getAdjacentCellPositions is used in the selection-outline plugin (particularly where and with what “cellStart” values it’s called) to be sure merged cells won’t mis-compute neighbours. Could you show the surrounding code in plugin.ts where we call getAdjacentCellPositions?

packages/editor/src/core/extensions/table/plugins/insert-handlers/plugin.ts (1)

65-87: Well-structured ProseMirror plugin implementation!

The plugin correctly manages the lifecycle of table insert buttons with proper cleanup on destroy and efficient updates only when the document changes. The use of setTimeout ensures the DOM is ready before the initial update.

packages/editor/src/core/extensions/table/plugins/insert-handlers/utils.ts (1)

20-116: Excellent drag interaction implementation!

The drag handling is well-implemented with:

  • Proper threshold-based activation
  • Visual feedback during drag
  • Bidirectional operations (add/remove)
  • Correct event cleanup
  • Different thresholds for horizontal (150px) vs vertical (40px) actions make sense ergonomically

Also applies to: 118-214

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant