[WIKI-277] fix: unable to delete last table column/row in editor#7389
[WIKI-277] fix: unable to delete last table column/row in editor#7389sriramveeraghanta merged 5 commits intopreviewfrom
Conversation
aaryan610
added
🐛bug
WalkthroughThe changes replace the original Changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧬 Code Graph Analysis (1)packages/editor/src/core/extensions/table/table/utilities/helpers.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/editor/src/core/extensions/table/table/utilities/delete-column.ts (1)
15-18: Consider improving table node retrieval robustness.The current logic uses
selection.$anchorCell.start(-1)andstate.doc.nodeAt(tableStart - 1)to get the table node. While this works, it relies on specific document structure assumptions that could be fragile.Consider using a more robust approach:
- // Get the ProseMirrorTable and calculate total columns - const tableStart = selection.$anchorCell.start(-1); - const selectedTable = state.doc.nodeAt(tableStart - 1); + // Get the ProseMirrorTable and calculate total columns + const tableStart = selection.$anchorCell.start(-1); + const selectedTable = state.doc.nodeAt(tableStart - 1); + + // Alternative: More robust table node retrieval + // const tablePos = selection.$anchorCell.pos; + // const selectedTable = state.doc.resolve(tablePos).node(-1);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/editor/src/core/extensions/table/table/table.ts(2 hunks)packages/editor/src/core/extensions/table/table/utilities/delete-column.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/editor/src/core/extensions/table/table/table.ts (1)
packages/editor/src/core/extensions/table/table/utilities/delete-column.ts (1)
deleteColumnOrTable(4-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
packages/editor/src/core/extensions/table/table/utilities/delete-column.ts (4)
1-3: LGTM: Imports are correct and necessary.The imports include all required types and functions from TipTap for table operations.
4-12: LGTM: Function signature and selection validation are correct.The function correctly implements the TipTap Command pattern and properly validates that the current selection is a CellSelection before proceeding.
31-36: LGTM: Decision logic correctly implements the PR objective.The logic correctly deletes the entire table when only one column remains, and falls back to normal column deletion otherwise. This successfully addresses the PR objective of handling the "unable to delete last column" issue.
20-28: Validate totalColumns logic for all table configurationsThe current implementation in packages/editor/src/core/extensions/table/table/utilities/delete-column.ts (lines 20–28) only sums
colspanvalues in the first row to determine the total number of columns. This can undercount or miscount tables that use rowspans or have differing column structures in later rows.Please verify and handle edge cases such as:
- First-row cells using
rowspan, masking columns that only appear beneath- Later rows introducing extra columns not represented in the first row
- Empty or missing cells in the first row
- Complex combinations of
rowspanandcolspanTo ensure accuracy, consider using ProseMirror’s
TableMapto compute the full table matrix and derive the true column count before deciding between deleting the entire table or a single column.packages/editor/src/core/extensions/table/table/table.ts (2)
29-29: LGTM: Import change is correct.The import correctly references the new
deleteColumnOrTableutility function from the local utilities directory.
143-143: LGTM: Command mapping correctly implements the new behavior.The
deleteColumncommand now usesdeleteColumnOrTabledirectly, which maintains the same interface while providing the enhanced functionality to delete the entire table when only one column remains.
aaryan610
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/editor/src/core/extensions/table/table/table.ts(3 hunks)packages/editor/src/core/extensions/table/table/utilities/delete-row.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/editor/src/core/extensions/table/table/table.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
packages/editor/src/core/extensions/table/table/utilities/delete-row.ts (5)
1-2: LGTM: Clean imports for table operations.The imports are appropriate and include all necessary components for the table manipulation functionality.
4-6: LGTM: Proper TipTap command structure.The function signature follows the standard TipTap command pattern with proper typing and destructuring.
9-12: LGTM: Appropriate selection validation.The validation correctly checks for CellSelection and returns false when not applicable, following TipTap command conventions.
14-18: LGTM: Standard table node retrieval with proper null checking.The table retrieval logic correctly uses ProseMirror's node access patterns and includes appropriate null checking.
28-34: LGTM: Correct conditional deletion logic.The implementation properly handles the core requirement - deleting the entire table when only one row remains, otherwise falling back to normal row deletion behavior.
packages/editor/src/core/extensions/table/table/utilities/delete-row.ts
Outdated
Show resolved
Hide resolved
* fix: delete column not working * refactor: delete column commad * fix: delete last row logic * refactor: row count logic * refactor: isCellSelection utility function
3 participants
Description
This PR fixes the bug where users are unable to delete the last column/row of a table.
Cause: By default, ProseMirror doesn't allow a table to exist without any column/row.
Solution: If the user tries to delete the last column/row, we delete the entire table, bypassing the default logic.
Type of Change
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes