Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix table selector and resizer #1919

Merged
merged 11 commits into from
Jun 30, 2023
Merged

Fix table selector and resizer #1919

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
85f7c7d
css fix
Andres-CT98 Jun 27, 2023
9391f35
added plugin events
Andres-CT98 Jun 29, 2023
ef52556
Merge branch 'master' of https://github.com/microsoft/roosterjs into …
Andres-CT98 Jun 29, 2023
6f7ea4a
Merge branch 'master' into u/acampostams/fix-table-selector-and-resizer
Andres-CT98 Jun 29, 2023
7bc81f4
remove from editor if getContent()
Andres-CT98 Jun 29, 2023
d7ea4ee
Merge branch 'master' of https://github.com/microsoft/roosterjs into …
Andres-CT98 Jun 29, 2023
ea200aa
Merge branch 'u/acampostams/fix-table-selector-and-resizer' of https:…
Andres-CT98 Jun 29, 2023
01ca812
fix
Andres-CT98 Jun 29, 2023
1e5bd1a
made constants
Andres-CT98 Jun 29, 2023
7f14ae4
Merge branch 'master' of https://github.com/microsoft/roosterjs into …
Andres-CT98 Jun 29, 2023
52e4c34
added comment
Andres-CT98 Jun 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion packages/roosterjs-editor-plugins/lib/plugins/TableResize/TableResize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import TableEditor from './editors/TableEditor';
import TableEditor, { TEF_CLASS_NAME } from './editors/TableEditor';
import { normalizeRect, safeInstanceOf } from 'roosterjs-editor-dom';
import {
CreateElementData,
Expand Down Expand Up @@ -85,9 +85,16 @@ export default class TableResize implements EditorPlugin {
case PluginEventType.ContentChanged:
case PluginEventType.Scroll:
case PluginEventType.ZoomChanged:
case PluginEventType.SelectionChanged:
this.setTableEditor(null);
this.invalidateTableRects();
break;
case PluginEventType.ExtractContentWithDom:
const divClass = 'div.' + TEF_CLASS_NAME;
e.clonedRoot.querySelectorAll(divClass).forEach(div => {
div.parentNode?.removeChild(div);
});
break;
}
}

Expand Down
4 changes: 4 additions & 0 deletions packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ const enum TOP_OR_SIDE {
top = 0,
side = 1,
}

// Class name for Table edit feature divs
export const TEF_CLASS_NAME = '_roosterjsTableEditFeature';
Andres-CT98 marked this conversation as resolved.
Show resolved Hide resolved

/**
* @internal
*
Expand Down
15 changes: 8 additions & 7 deletions packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableResizer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import DragAndDropHelper from '../../../pluginUtils/DragAndDropHelper';
import TableEditFeature from './TableEditorFeature';
import { createElement, normalizeRect, VTable } from 'roosterjs-editor-dom';
import { CreateElementData } from 'roosterjs-editor-types';
import { TEF_CLASS_NAME } from './TableEditor';

const TABLE_RESIZER_LENGTH = 12;
const MIN_CELL_WIDTH = 30;
Expand All @@ -24,7 +25,8 @@ export default function createTableResizer(
const document = table.ownerDocument;
const createElementData = {
tag: 'div',
style: `position: fixed; cursor: ${
className: TEF_CLASS_NAME,
style: `position: absolute; cursor: ${
isRTL ? 'ne' : 'nw'
}-resize; user-select: none; border: 1px solid #808080`,
};
Expand All @@ -35,7 +37,7 @@ export default function createTableResizer(

div.style.width = `${TABLE_RESIZER_LENGTH}px`;
div.style.height = `${TABLE_RESIZER_LENGTH}px`;
document.body.appendChild(div);
table.insertAdjacentElement('afterend', div);
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we also need to do the same change for other table resize helper elements? Actually there are 6:

  1. Whole table selector
  2. Whole table resizer (these 2 are visible as a gray square)
  3. horizontal resizer
  4. vertical resizer
  5. new column inserter (a circle with "+" sign when hover above a table)
  6. new row inserter (a circle with "+" sign when hover on left side (or right side when RTL) of a table)

3,4 are invisible, but it will change mouse cursor when hover
5,6 will be visible when hover.

I can see your change fixed 1 and 2, so we also need same fix for 3-6. And maybe we should have a unified function for all of them to reduce duplicated code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that but wanted to do that on a separate PR. I think a better approach would be to do something like Image Edit does, where the helpers are in in a span.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's ok to do it separately, and please open an issue to track it.

I think it should be fine to use a SPAN for all the resizer UI, although this seems like a larger change.


const context: DragAndDropContext = {
isRTL,
Expand Down Expand Up @@ -138,13 +140,12 @@ function onDragging(
}

function setResizeDivPosition(context: DragAndDropContext, trigger: HTMLElement) {
const { table, isRTL } = context;
const { table, isRTL, zoomScale } = context;
const rect = normalizeRect(table.getBoundingClientRect());

if (rect) {
trigger.style.top = `${rect.bottom}px`;
trigger.style.left = isRTL
? `${rect.left - TABLE_RESIZER_LENGTH - 2}px`
: `${rect.right}px`;
isRTL
? (trigger.style.marginRight = `${(rect.right - rect.left) / zoomScale}px`)
: (trigger.style.marginLeft = `${(rect.right - rect.left) / zoomScale}px`);
}
}
24 changes: 18 additions & 6 deletions packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableSelector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import DragAndDropHandler from '../../../pluginUtils/DragAndDropHandler';
import DragAndDropHelper from '../../../pluginUtils/DragAndDropHelper';
import TableEditorFeature from './TableEditorFeature';
import { createElement, normalizeRect, safeInstanceOf } from 'roosterjs-editor-dom';
import { CreateElementData, IEditor, Rect } from 'roosterjs-editor-types';
import { TEF_CLASS_NAME } from './TableEditor';
import {
createElement,
getComputedStyle,
normalizeRect,
safeInstanceOf,
} from 'roosterjs-editor-dom';

const TABLE_SELECTOR_LENGTH = 12;
const TABLE_SELECTOR_ID = '_Table_Selector';
Expand Down Expand Up @@ -31,7 +37,9 @@ export default function createTableSelector(
const document = table.ownerDocument;
const createElementData = {
tag: 'div',
style: 'position: fixed; cursor: all-scroll; user-select: none; border: 1px solid #808080',
className: TEF_CLASS_NAME,
style:
'position: absolute; cursor: all-scroll; user-select: none; border: 1px solid #808080;',
};

onShowHelperElement?.(createElementData, 'TableSelector');
Expand All @@ -41,12 +49,13 @@ export default function createTableSelector(
div.id = TABLE_SELECTOR_ID;
div.style.width = `${TABLE_SELECTOR_LENGTH}px`;
div.style.height = `${TABLE_SELECTOR_LENGTH}px`;
document.body.appendChild(div);
table.insertAdjacentElement('beforebegin', div);
Copy link
Collaborator

@JiuqingSong JiuqingSong Jun 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

So this is to insert the handle DIV into editor content, is that right? If an auto save happens while the resize handle is showing, it will be saved as part of the content

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it is ok to insert here. But we need to remove it when getContent() is called, by handling ExtractContentWithDomEvent event


const context: TableSelectorContext = {
table,
zoomScale,
rect,
isRTL: getComputedStyle(table, 'direction') == 'rtl',
};

setSelectorDivPosition(context, div);
Expand Down Expand Up @@ -76,6 +85,7 @@ interface TableSelectorContext {
table: HTMLTableElement;
zoomScale: number;
rect: Rect | null;
isRTL: boolean;
}

interface TableSelectorInitValue {
Expand Down Expand Up @@ -109,10 +119,12 @@ class TableSelectorFeature extends DragAndDropHelper<TableSelectorContext, Table
}

function setSelectorDivPosition(context: TableSelectorContext, trigger: HTMLElement) {
const { rect } = context;
const { rect, zoomScale } = context;
if (rect) {
trigger.style.top = `${rect.top - TABLE_SELECTOR_LENGTH}px`;
trigger.style.left = `${rect.left - TABLE_SELECTOR_LENGTH - 2}px`;
trigger.style.marginTop = `${-TABLE_SELECTOR_LENGTH - 1}px`;
context.isRTL
? (trigger.style.marginRight = `${(rect.right - rect.left) / zoomScale}px`)
: (trigger.style.marginLeft = `${-TABLE_SELECTOR_LENGTH - 1}px`);
}
}

Expand Down