[Table Improvements] Add preview for table cell selection #3274
Conversation
| parsedTable, | ||
| firstCell, | ||
| lastCell, | ||
| isHiding ? '' : `background-color:${tableSelectionColor}!important;` |
There was a problem hiding this comment.
If we are hiding, do we need to handle all the table parsing or we can just remove the style?
juliaroldi
requested review from
BryanValverdeU,
JiuqingSong,
Copilot,
flyingbee2012,
ianeli1 and
vinguyen12
There was a problem hiding this comment.
Pull request overview
This PR adds table cell selection preview support when entering shadow edit mode. When shadow edit starts, the background color is removed from selected table cells to make style changes visible during editing. The implementation extracts table cell styling logic from setDOMSelection.ts into reusable functions and adds a new toggleTableSelection function to manage table selection visibility.
Changes:
- Added
toggleTableSelectionfunction to hide/show table cell selection background when entering/leaving shadow edit mode - Refactored table cell styling logic from
setDOMSelection.tsintosetTableCellsStyle.tsfor reusability - Updated
switchShadowEditto calltoggleTableSelectionalongside existingtoggleCaretcalls
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
switchShadowEdit.ts |
Added calls to toggleTableSelection when entering/leaving shadow edit mode |
toggleTableSelection.ts |
New file implementing table selection visibility toggle logic |
setTableCellsStyle.ts |
Extracted table cell styling functions (setTableCellsStyle, removeTableCellsStyle) from setDOMSelection.ts |
setDOMSelection.ts |
Refactored to use extracted setTableCellsStyle function, removed duplicate code |
switchShadowEditTest.ts |
Added test coverage for new toggleTableSelection calls |
toggleTableSelectionTest.ts |
Comprehensive tests for table selection toggle functionality |
setTableCellsStyleTest.ts |
Comprehensive tests for table cell styling functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * @internal | ||
| * Set style for table cells in the selection | ||
| * @param core The EditorCore object | ||
| * @param selection The table selection | ||
| * @param style The CSS style to apply, or empty string to remove style | ||
| * @param parsedTable Optional pre-parsed table to avoid parsing twice | ||
| */ | ||
| export function removeTableCellsStyle(core: EditorCore) { | ||
| core.api.setEditorStyle(core, DOM_SELECTION_CSS_KEY, ''); | ||
| } | ||
|
|
||
| /** | ||
| * @internal | ||
| * Set style for table cells in the selection | ||
| * @param core The EditorCore object | ||
| * @param selection The table selection | ||
| * @param style The CSS style to apply, or empty string to remove style | ||
| * @param parsedTable Optional pre-parsed table to avoid parsing twice | ||
| */ |
There was a problem hiding this comment.
The JSDoc comments for both removeTableCellsStyle and setTableCellsStyle functions are outdated and incorrect. They still reference parameters that don't exist in the actual function signatures. For removeTableCellsStyle, the function only takes core as a parameter, but the JSDoc mentions selection, style, and parsedTable. For setTableCellsStyle, the function takes core, table, parsedTable, firstCell, and lastCell, but the JSDoc still references the old parameters selection, style, and describes parsedTable as "Optional" when it's actually required.
When start shadow edit, check if table cells are selected, if they are selected, remove the background color to make the styles changes visible in the table.
