From 0aacf072e02e028795cfb3eb6a85995897c7702d Mon Sep 17 00:00:00 2001 From: Jiuqing Song Date: Mon, 31 Jul 2023 08:44:00 -0700 Subject: [PATCH 01/17] Content Model: Graduate some features (#1975) --- .../ContentModelEditorOptionsPlugin.ts | 2 - .../ContentModelExperimentalFeatures.tsx | 3 - .../lib/editor/ContentModelEditor.ts | 2 +- .../lib/editor/coreApi/createContentModel.ts | 6 +- .../editor/createContentModelEditorCore.ts | 22 +---- .../editor/plugins/ContentModelEditPlugin.ts | 9 +-- .../lib/publicTypes/ContentModelEditorCore.ts | 5 -- .../test/editor/ContentModelEditorTest.ts | 20 +---- .../editor/coreApi/createContentModelTest.ts | 80 ------------------- .../createContentModelEditorCoreTest.ts | 35 ++------ .../plugins/ContentModelEditPluginTest.ts | 45 ----------- .../plugins/paste/e2e/cmPasteFromExcelTest.ts | 4 + .../lib/enum/ExperimentalFeatures.ts | 22 ++--- 13 files changed, 33 insertions(+), 222 deletions(-) diff --git a/demo/scripts/controls/sidePane/editorOptions/ContentModelEditorOptionsPlugin.ts b/demo/scripts/controls/sidePane/editorOptions/ContentModelEditorOptionsPlugin.ts index ee4fd928d19..bdc1a6ce315 100644 --- a/demo/scripts/controls/sidePane/editorOptions/ContentModelEditorOptionsPlugin.ts +++ b/demo/scripts/controls/sidePane/editorOptions/ContentModelEditorOptionsPlugin.ts @@ -30,8 +30,6 @@ const initialState: BuildInPluginState = { forcePreserveRatio: false, experimentalFeatures: [ ExperimentalFeatures.AutoFormatList, - ExperimentalFeatures.ReusableContentModel, - ExperimentalFeatures.EditWithContentModel, ExperimentalFeatures.InlineEntityReadOnlyDelimiters, ExperimentalFeatures.ContentModelPaste, ], diff --git a/demo/scripts/controls/sidePane/editorOptions/ContentModelExperimentalFeatures.tsx b/demo/scripts/controls/sidePane/editorOptions/ContentModelExperimentalFeatures.tsx index acc2e3088a6..bcb2cabe1b6 100644 --- a/demo/scripts/controls/sidePane/editorOptions/ContentModelExperimentalFeatures.tsx +++ b/demo/scripts/controls/sidePane/editorOptions/ContentModelExperimentalFeatures.tsx @@ -14,9 +14,6 @@ const FeatureNames: Partial> = { 'Trigger formatting by a especial characters. Ex: (A), 1. i).', [ExperimentalFeatures.ReuseAllAncestorListElements]: "Reuse ancestor list elements even if they don't match the types from the list item.", - [ExperimentalFeatures.ReusableContentModel]: - 'Reuse existing DOM structure if possible when convert Content Model back to DOM tree', - [ExperimentalFeatures.EditWithContentModel]: 'Handle keyboard edit event with Content Model', [ExperimentalFeatures.DeleteTableWithBackspace]: 'Delete a table selected with the table selector pressing Backspace key', [ExperimentalFeatures.InlineEntityReadOnlyDelimiters]: diff --git a/packages-content-model/roosterjs-content-model-editor/lib/editor/ContentModelEditor.ts b/packages-content-model/roosterjs-content-model-editor/lib/editor/ContentModelEditor.ts index 18e4d95ff31..630a3a7cf4e 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/editor/ContentModelEditor.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/editor/ContentModelEditor.ts @@ -53,7 +53,7 @@ export default class ContentModelEditor cacheContentModel(model: ContentModelDocument | null) { const core = this.getCore(); - if (core.reuseModel && !core.lifecycle.shadowEditFragment) { + if (!core.lifecycle.shadowEditFragment) { core.cachedModel = model || undefined; core.cachedRangeEx = undefined; } diff --git a/packages-content-model/roosterjs-content-model-editor/lib/editor/coreApi/createContentModel.ts b/packages-content-model/roosterjs-content-model-editor/lib/editor/coreApi/createContentModel.ts index dfb23225ab9..e21d011cca7 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/editor/coreApi/createContentModel.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/editor/coreApi/createContentModel.ts @@ -13,7 +13,7 @@ import { * @param option The option to customize the behavior of DOM to Content Model conversion */ export const createContentModel: CreateContentModel = (core, option) => { - let cachedModel = core.reuseModel ? core.cachedModel : null; + let cachedModel = core.cachedModel; if (cachedModel && core.lifecycle.shadowEditFragment) { // When in shadow edit, use a cloned model so we won't pollute the cached one @@ -38,10 +38,6 @@ function internalCreateContentModel( ...option?.processorOverride, }; - if (!core.reuseModel) { - context.disableCacheElement = true; - } - return domToContentModel( core.contentDiv, context, diff --git a/packages-content-model/roosterjs-content-model-editor/lib/editor/createContentModelEditorCore.ts b/packages-content-model/roosterjs-content-model-editor/lib/editor/createContentModelEditorCore.ts index f03f2eda532..c7cd4287690 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/editor/createContentModelEditorCore.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/editor/createContentModelEditorCore.ts @@ -28,12 +28,7 @@ export const createContentModelEditorCore: CoreCreator< new ContentModelEditPlugin(), ], corePluginOverride: { - typeInContainer: isFeatureEnabled( - options.experimentalFeatures, - ExperimentalFeatures.EditWithContentModel - ) - ? new ContentModelTypeInContainerPlugin() - : undefined, + typeInContainer: new ContentModelTypeInContainerPlugin(), copyPaste: isFeatureEnabled( options.experimentalFeatures, ExperimentalFeatures.ContentModelPaste @@ -82,10 +77,6 @@ function promoteContentModelInfo( cmCore.defaultDomToModelOptions = options.defaultDomToModelOptions || {}; cmCore.defaultModelToDomOptions = options.defaultModelToDomOptions || {}; - cmCore.reuseModel = isFeatureEnabled( - experimentalFeatures, - ExperimentalFeatures.ReusableContentModel - ); cmCore.addDelimiterForEntity = isFeatureEnabled( experimentalFeatures, ExperimentalFeatures.InlineEntityReadOnlyDelimiters @@ -96,17 +87,8 @@ function promoteCoreApi(cmCore: ContentModelEditorCore) { cmCore.api.createEditorContext = createEditorContext; cmCore.api.createContentModel = createContentModel; cmCore.api.setContentModel = setContentModel; + cmCore.api.switchShadowEdit = switchShadowEdit; cmCore.api.getSelectionRangeEx = getSelectionRangeEx; - - if ( - isFeatureEnabled( - cmCore.lifecycle.experimentalFeatures, - ExperimentalFeatures.ReusableContentModel - ) - ) { - // Only use Content Model shadow edit when reuse model is enabled because it relies on cached model for the original model - cmCore.api.switchShadowEdit = switchShadowEdit; - } cmCore.originalApi.createEditorContext = createEditorContext; cmCore.originalApi.createContentModel = createContentModel; cmCore.originalApi.setContentModel = setContentModel; diff --git a/packages-content-model/roosterjs-content-model-editor/lib/editor/plugins/ContentModelEditPlugin.ts b/packages-content-model/roosterjs-content-model-editor/lib/editor/plugins/ContentModelEditPlugin.ts index bdfd6bbcae8..11c4c051776 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/editor/plugins/ContentModelEditPlugin.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/editor/plugins/ContentModelEditPlugin.ts @@ -10,7 +10,6 @@ import { isNodeOfType, normalizeContentModel } from 'roosterjs-content-model-dom import { EditorPlugin, EntityOperationEvent, - ExperimentalFeatures, IEditor, Keys, NodePosition, @@ -40,7 +39,6 @@ const ProcessKey = 'Process'; export default class ContentModelEditPlugin implements EditorPlugin { private editor: IContentModelEditor | null = null; private triggeredEntityEvents: EntityOperationEvent[] = []; - private editWithContentModel = false; private hasDefaultFormat = false; /** @@ -59,9 +57,6 @@ export default class ContentModelEditPlugin implements EditorPlugin { initialize(editor: IEditor) { // TODO: Later we may need a different interface for Content Model editor plugin this.editor = editor as IContentModelEditor; - this.editWithContentModel = this.editor.isFeatureEnabled( - ExperimentalFeatures.EditWithContentModel - ); const defaultFormat = this.editor.getContentModelDefaultFormat(); this.hasDefaultFormat = @@ -117,10 +112,10 @@ export default class ContentModelEditPlugin implements EditorPlugin { const rawEvent = event.rawEvent; const which = rawEvent.which; - if (!this.editWithContentModel || rawEvent.defaultPrevented) { + if (rawEvent.defaultPrevented || event.handledByEditFeature) { // Other plugins already handled this event, so it is most likely content is already changed, we need to clear cached content model editor.cacheContentModel(null /*model*/); - } else if (!rawEvent.defaultPrevented && !event.handledByEditFeature) { + } else { // TODO: Consider use ContentEditFeature and need to hide other conflict features that are not based on Content Model switch (which) { case Keys.BACKSPACE: diff --git a/packages-content-model/roosterjs-content-model-editor/lib/publicTypes/ContentModelEditorCore.ts b/packages-content-model/roosterjs-content-model-editor/lib/publicTypes/ContentModelEditorCore.ts index ba886f462af..a0e48c67474 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/publicTypes/ContentModelEditorCore.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/publicTypes/ContentModelEditorCore.ts @@ -101,11 +101,6 @@ export interface ContentModelEditorCore extends EditorCore { */ defaultModelToDomOptions: ModelToDomOption; - /** - * Whether reuse Content Model is allowed - */ - reuseModel: boolean; - /** * Whether adding delimiter for entity is allowed */ diff --git a/packages-content-model/roosterjs-content-model-editor/test/editor/ContentModelEditorTest.ts b/packages-content-model/roosterjs-content-model-editor/test/editor/ContentModelEditorTest.ts index e3e5b8a24cc..6f7b95debcc 100644 --- a/packages-content-model/roosterjs-content-model-editor/test/editor/ContentModelEditorTest.ts +++ b/packages-content-model/roosterjs-content-model-editor/test/editor/ContentModelEditorTest.ts @@ -2,13 +2,8 @@ import * as contentModelToDom from 'roosterjs-content-model-dom/lib/modelToDom/c import * as domToContentModel from 'roosterjs-content-model-dom/lib/domToModel/domToContentModel'; import ContentModelEditor from '../../lib/editor/ContentModelEditor'; import { ContentModelDocument, EditorContext } from 'roosterjs-content-model-types'; +import { EditorPlugin, PluginEventType, SelectionRangeTypes } from 'roosterjs-editor-types'; import { tablePreProcessor } from '../../lib/domToModel/processors/tablePreProcessor'; -import { - EditorPlugin, - ExperimentalFeatures, - PluginEventType, - SelectionRangeTypes, -} from 'roosterjs-editor-types'; const editorContext: EditorContext = { isDarkMode: false, @@ -35,7 +30,6 @@ describe('ContentModelEditor', () => { processorOverride: { table: tablePreProcessor, }, - disableCacheElement: true, }, editorContext, { @@ -48,9 +42,7 @@ describe('ContentModelEditor', () => { it('domToContentModel, with Reuse Content Model dont add disableCacheElement option', () => { const div = document.createElement('div'); - const editor = new ContentModelEditor(div, { - experimentalFeatures: [ExperimentalFeatures.ReusableContentModel], - }); + const editor = new ContentModelEditor(div); const mockedResult = 'Result' as any; @@ -174,9 +166,7 @@ describe('ContentModelEditor', () => { it('get model with cache', () => { const div = document.createElement('div'); - const editor = new ContentModelEditor(div, { - experimentalFeatures: [ExperimentalFeatures.ReusableContentModel], - }); + const editor = new ContentModelEditor(div); const cachedModel = 'MODEL' as any; (editor as any).core.cachedModel = cachedModel; @@ -191,9 +181,7 @@ describe('ContentModelEditor', () => { it('cache model', () => { const div = document.createElement('div'); - const editor = new ContentModelEditor(div, { - experimentalFeatures: [ExperimentalFeatures.ReusableContentModel], - }); + const editor = new ContentModelEditor(div); const cachedModel = 'MODEL' as any; editor.cacheContentModel(cachedModel); diff --git a/packages-content-model/roosterjs-content-model-editor/test/editor/coreApi/createContentModelTest.ts b/packages-content-model/roosterjs-content-model-editor/test/editor/coreApi/createContentModelTest.ts index 6697e67e044..407cd547685 100644 --- a/packages-content-model/roosterjs-content-model-editor/test/editor/coreApi/createContentModelTest.ts +++ b/packages-content-model/roosterjs-content-model-editor/test/editor/coreApi/createContentModelTest.ts @@ -41,80 +41,9 @@ describe('createContentModel', () => { } as any) as ContentModelEditorCore; }); - it('Not reuse model, no shadow edit', () => { - const option: DomToModelOption = { disableCacheElement: true }; - - const model = createContentModel(core, option); - - expect(createEditorContext).toHaveBeenCalledWith(core); - expect(getSelectionRangeEx).toHaveBeenCalledWith(core); - expect(domToContentModelSpy).toHaveBeenCalledWith( - mockedDiv, - { - ...option, - processorOverride: { - table: tablePreProcessor, - }, - }, - mockedEditorContext, - null - ); - expect(model).toBe(mockedModel); - }); - - it('Not reuse model, no shadow edit, with default options', () => { - const defaultOption = { o: 'OPTION', disableCacheElement: true } as any; - const option: DomToModelOption = {}; - - core.defaultDomToModelOptions = defaultOption; - - const model = createContentModel(core, option); - - expect(createEditorContext).toHaveBeenCalledWith(core); - expect(getSelectionRangeEx).toHaveBeenCalledWith(core); - expect(domToContentModelSpy).toHaveBeenCalledWith( - mockedDiv, - { - processorOverride: { - table: tablePreProcessor, - }, - ...defaultOption, - }, - mockedEditorContext, - null - ); - expect(model).toBe(mockedModel); - }); - - it('Not reuse model, no shadow edit, with default options and additional option', () => { - const defaultOption = { o: 'OPTION', disableCacheElement: true } as any; - const additionalOption = { o: 'OPTION1', o2: 'OPTION2' } as any; - - core.defaultDomToModelOptions = defaultOption; - - const model = createContentModel(core, additionalOption); - - expect(createEditorContext).toHaveBeenCalledWith(core); - expect(getSelectionRangeEx).toHaveBeenCalledWith(core); - expect(domToContentModelSpy).toHaveBeenCalledWith( - mockedDiv, - { - processorOverride: { - table: tablePreProcessor, - }, - ...defaultOption, - ...additionalOption, - }, - mockedEditorContext, - null - ); - expect(model).toBe(mockedModel); - }); - it('Reuse model, no cache, no shadow edit', () => { const option: DomToModelOption = { disableCacheElement: false }; - core.reuseModel = true; core.cachedModel = undefined; const model = createContentModel(core, option); @@ -137,9 +66,6 @@ describe('createContentModel', () => { it('Reuse model, no shadow edit', () => { const option: DomToModelOption = {}; - - core.reuseModel = true; - const model = createContentModel(core, option); expect(createEditorContext).not.toHaveBeenCalled(); @@ -151,7 +77,6 @@ describe('createContentModel', () => { it('Reuse model, with cache, with shadow edit', () => { const option: DomToModelOption = {}; - core.reuseModel = true; core.lifecycle.shadowEditFragment = {} as any; const model = createContentModel(core, option); @@ -206,7 +131,6 @@ describe('createContentModel with selection', () => { processorOverride: { table: tablePreProcessor, }, - disableCacheElement: true, }, undefined, { @@ -239,7 +163,6 @@ describe('createContentModel with selection', () => { processorOverride: { table: tablePreProcessor, }, - disableCacheElement: true, }, undefined, { @@ -270,7 +193,6 @@ describe('createContentModel with selection', () => { processorOverride: { table: tablePreProcessor, }, - disableCacheElement: true, }, undefined, { @@ -295,7 +217,6 @@ describe('createContentModel with selection', () => { processorOverride: { table: tablePreProcessor, }, - disableCacheElement: true, }, undefined, { @@ -319,7 +240,6 @@ describe('createContentModel with selection', () => { processorOverride: { table: tablePreProcessor, }, - disableCacheElement: true, }, undefined, { diff --git a/packages-content-model/roosterjs-content-model-editor/test/editor/createContentModelEditorCoreTest.ts b/packages-content-model/roosterjs-content-model-editor/test/editor/createContentModelEditorCoreTest.ts index 7d58599ec63..635b67a9f2d 100644 --- a/packages-content-model/roosterjs-content-model-editor/test/editor/createContentModelEditorCoreTest.ts +++ b/packages-content-model/roosterjs-content-model-editor/test/editor/createContentModelEditorCoreTest.ts @@ -54,7 +54,7 @@ describe('createContentModelEditorCore', () => { expect(createEditorCoreSpy).toHaveBeenCalledWith(contentDiv, { plugins: [new ContentModelFormatPlugin(), new ContentModelEditPlugin()], corePluginOverride: { - typeInContainer: undefined, + typeInContainer: new ContentModelTypeInContainerPlugin(), copyPaste: copyPastePlugin, }, }); @@ -64,7 +64,7 @@ describe('createContentModelEditorCore', () => { defaultFormat: {}, }, api: { - switchShadowEdit: mockedSwitchShadowEdit, + switchShadowEdit, createEditorContext, createContentModel, setContentModel, @@ -87,7 +87,6 @@ describe('createContentModelEditorCore', () => { textColor: undefined, backgroundColor: undefined, }, - reuseModel: false, addDelimiterForEntity: false, contentDiv: { style: {}, @@ -105,7 +104,6 @@ describe('createContentModelEditorCore', () => { corePluginOverride: { copyPaste: copyPastePlugin, }, - experimentalFeatures: [ExperimentalFeatures.EditWithContentModel], }; const core = createContentModelEditorCore(contentDiv, options); @@ -117,7 +115,6 @@ describe('createContentModelEditorCore', () => { typeInContainer: new ContentModelTypeInContainerPlugin(), copyPaste: copyPastePlugin, }, - experimentalFeatures: [ExperimentalFeatures.EditWithContentModel], }); expect(core).toEqual({ @@ -126,7 +123,7 @@ describe('createContentModelEditorCore', () => { defaultFormat: {}, }, api: { - switchShadowEdit: mockedSwitchShadowEdit, + switchShadowEdit, createEditorContext, createContentModel, setContentModel, @@ -149,7 +146,6 @@ describe('createContentModelEditorCore', () => { textColor: undefined, backgroundColor: undefined, }, - reuseModel: false, addDelimiterForEntity: false, contentDiv: { style: {}, @@ -174,10 +170,6 @@ describe('createContentModelEditorCore', () => { }, }; - spyOn(isFeatureEnabled, 'isFeatureEnabled').and.callFake( - (features, feature) => feature == ExperimentalFeatures.EditWithContentModel - ); - const core = createContentModelEditorCore(contentDiv, options); expect(createEditorCoreSpy).toHaveBeenCalledWith(contentDiv, { @@ -201,7 +193,7 @@ describe('createContentModelEditorCore', () => { }, }, api: { - switchShadowEdit: mockedSwitchShadowEdit, + switchShadowEdit, createEditorContext, createContentModel, setContentModel, @@ -224,7 +216,6 @@ describe('createContentModelEditorCore', () => { textColor: 'red', backgroundColor: 'blue', }, - reuseModel: false, addDelimiterForEntity: false, contentDiv: { style: {}, @@ -233,20 +224,12 @@ describe('createContentModelEditorCore', () => { }); it('Reuse model', () => { - mockedCore.lifecycle.experimentalFeatures.push(ExperimentalFeatures.ReusableContentModel); - const options = { corePluginOverride: { copyPaste: copyPastePlugin, }, }; - spyOn(isFeatureEnabled, 'isFeatureEnabled').and.callFake( - (features, feature) => - feature == ExperimentalFeatures.EditWithContentModel || - feature == ExperimentalFeatures.ReusableContentModel - ); - const core = createContentModelEditorCore(contentDiv, options); expect(createEditorCoreSpy).toHaveBeenCalledWith(contentDiv, { @@ -258,7 +241,7 @@ describe('createContentModelEditorCore', () => { }); expect(core).toEqual({ lifecycle: { - experimentalFeatures: [ExperimentalFeatures.ReusableContentModel], + experimentalFeatures: [], defaultFormat: {}, }, api: { @@ -285,7 +268,6 @@ describe('createContentModelEditorCore', () => { textColor: undefined, backgroundColor: undefined, }, - reuseModel: true, addDelimiterForEntity: false, contentDiv: { style: {}, @@ -305,9 +287,7 @@ describe('createContentModelEditorCore', () => { }; spyOn(isFeatureEnabled, 'isFeatureEnabled').and.callFake( - (features, feature) => - feature == ExperimentalFeatures.EditWithContentModel || - feature == ExperimentalFeatures.InlineEntityReadOnlyDelimiters + (features, feature) => feature == ExperimentalFeatures.InlineEntityReadOnlyDelimiters ); const core = createContentModelEditorCore(contentDiv, options); @@ -325,7 +305,7 @@ describe('createContentModelEditorCore', () => { defaultFormat: {}, }, api: { - switchShadowEdit: mockedSwitchShadowEdit, + switchShadowEdit, createEditorContext, createContentModel, setContentModel, @@ -348,7 +328,6 @@ describe('createContentModelEditorCore', () => { textColor: undefined, backgroundColor: undefined, }, - reuseModel: false, addDelimiterForEntity: true, contentDiv: { style: {}, diff --git a/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/ContentModelEditPluginTest.ts b/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/ContentModelEditPluginTest.ts index 329be288b9f..758a81aefa7 100644 --- a/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/ContentModelEditPluginTest.ts +++ b/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/ContentModelEditPluginTest.ts @@ -6,7 +6,6 @@ import { IContentModelEditor } from '../../../lib/publicTypes/IContentModelEdito import { Position } from 'roosterjs-editor-dom'; import { EntityOperation, - ExperimentalFeatures, Keys, PluginEventType, SelectionRangeTypes, @@ -15,21 +14,16 @@ import { describe('ContentModelEditPlugin', () => { let editor: IContentModelEditor; let cacheContentModel: jasmine.Spy; - let isFeatureEnabled: jasmine.Spy; let getContentModelDefaultFormat: jasmine.Spy; beforeEach(() => { cacheContentModel = jasmine.createSpy('cacheContentModel'); - isFeatureEnabled = jasmine - .createSpy('isFeatureEnabled') - .and.callFake(f => f == ExperimentalFeatures.EditWithContentModel); getContentModelDefaultFormat = jasmine .createSpy('getContentModelDefaultFormat') .and.returnValue({}); editor = ({ cacheContentModel, - isFeatureEnabled, getContentModelDefaultFormat, getSelectionRangeEx: () => ({ @@ -75,40 +69,6 @@ describe('ContentModelEditPlugin', () => { expect(cacheContentModel).not.toHaveBeenCalled(); }); - it('Backspace, feature is not enabled', () => { - const plugin = new ContentModelEditPlugin(); - const rawEvent = { which: Keys.BACKSPACE } as any; - - isFeatureEnabled.and.returnValue(false); - - plugin.initialize(editor); - - plugin.onPluginEvent({ - eventType: PluginEventType.KeyDown, - rawEvent, - }); - - expect(handleKeyDownEventSpy).not.toHaveBeenCalled(); - expect(cacheContentModel).toHaveBeenCalledWith(null); - }); - - it('Delete, feature is not enabled', () => { - const plugin = new ContentModelEditPlugin(); - const rawEvent = { which: Keys.DELETE } as any; - - isFeatureEnabled.and.returnValue(false); - - plugin.initialize(editor); - - plugin.onPluginEvent({ - eventType: PluginEventType.KeyDown, - rawEvent, - }); - - expect(handleKeyDownEventSpy).not.toHaveBeenCalled(); - expect(cacheContentModel).toHaveBeenCalledWith(null); - }); - it('Other key', () => { const plugin = new ContentModelEditPlugin(); const rawEvent = { which: 41 } as any; @@ -303,7 +263,6 @@ describe('ContentModelEditPlugin for default format', () => { let getSelectionRangeEx: jasmine.Spy; let getPendingFormatSpy: jasmine.Spy; let setPendingFormatSpy: jasmine.Spy; - let isFeatureEnabledSpy: jasmine.Spy; let cacheContentModelSpy: jasmine.Spy; let addUndoSnapshotSpy: jasmine.Spy; @@ -311,9 +270,6 @@ describe('ContentModelEditPlugin for default format', () => { setPendingFormatSpy = spyOn(pendingFormat, 'setPendingFormat'); getPendingFormatSpy = spyOn(pendingFormat, 'getPendingFormat'); getSelectionRangeEx = jasmine.createSpy('getSelectionRangeEx'); - isFeatureEnabledSpy = jasmine - .createSpy('isFeatureEnabled') - .and.callFake(f => f == ExperimentalFeatures.EditWithContentModel); cacheContentModelSpy = jasmine.createSpy('cacheContentModel'); addUndoSnapshotSpy = jasmine.createSpy('addUndoSnapshot'); @@ -325,7 +281,6 @@ describe('ContentModelEditPlugin for default format', () => { getContentModelDefaultFormat: () => ({ fontFamily: 'Arial', }), - isFeatureEnabled: isFeatureEnabledSpy, cacheContentModel: cacheContentModelSpy, addUndoSnapshot: addUndoSnapshotSpy, } as any) as IContentModelEditor; diff --git a/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/paste/e2e/cmPasteFromExcelTest.ts b/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/paste/e2e/cmPasteFromExcelTest.ts index be8028bc2e5..f449f977639 100644 --- a/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/paste/e2e/cmPasteFromExcelTest.ts +++ b/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/paste/e2e/cmPasteFromExcelTest.ts @@ -73,10 +73,14 @@ describe(ID, () => { spyOn(processPastedContentFromExcel, 'processPastedContentFromExcel').and.callThrough(); paste(editor, clipboardData, false, false, true); + + editor.cacheContentModel(null); + const model = editor.createContentModel({ processorOverride: { table: tableProcessor, }, + disableCacheElement: true, }); expect(model).toEqual({ diff --git a/packages/roosterjs-editor-types/lib/enum/ExperimentalFeatures.ts b/packages/roosterjs-editor-types/lib/enum/ExperimentalFeatures.ts index 508fca93808..410fa620c0a 100644 --- a/packages/roosterjs-editor-types/lib/enum/ExperimentalFeatures.ts +++ b/packages/roosterjs-editor-types/lib/enum/ExperimentalFeatures.ts @@ -124,6 +124,18 @@ export const enum ExperimentalFeatures { */ DefaultFormatOnContainer = 'DefaultFormatOnContainer', + /** + * @deprecated This feature is always enabled + * Reuse existing DOM structure if possible when convert Content Model back to DOM tree + */ + ReusableContentModel = 'ReusableContentModel', + + /** + * @deprecated This feature is always enabled + * Handle keyboard editing event with Content Model + */ + EditWithContentModel = 'EditWithContentModel', + //#endregion /** @@ -144,16 +156,6 @@ export const enum ExperimentalFeatures { */ ReuseAllAncestorListElements = 'ReuseAllAncestorListElements', - /** - * Reuse existing DOM structure if possible when convert Content Model back to DOM tree - */ - ReusableContentModel = 'ReusableContentModel', - - /** - * Handle keyboard editing event with Content Model - */ - EditWithContentModel = 'EditWithContentModel', - /** * Delete table with Backspace key with the whole was selected with table selector */ From 648f9ef7e22257067f9781b4b401e0346e971dd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Roldi?= Date: Mon, 31 Jul 2023 13:30:02 -0300 Subject: [PATCH 02/17] customized merge --- .../PastePlugin/ContentModelPastePlugin.ts | 1 + .../lib/index.ts | 1 + .../modelApi/selection/collectSelections.ts | 10 - .../lib/publicApi/link/adjustLinkSelection.ts | 2 +- .../lib/publicApi/link/insertLink.ts | 2 +- .../lib/publicApi/link/removeLink.ts | 2 +- .../selection/getSelectedSegments.ts | 12 ++ .../lib/publicApi/utils/paste.ts | 63 ++++--- .../event/ContentModelBeforePasteEvent.ts | 6 +- .../selection/collectSelectionsTest.ts | 157 --------------- .../selection/getSelectedSegmentsTest.ts | 178 ++++++++++++++++++ .../test/publicApi/utils/pasteTest.ts | 26 ++- 12 files changed, 263 insertions(+), 197 deletions(-) create mode 100644 packages-content-model/roosterjs-content-model-editor/lib/publicApi/selection/getSelectedSegments.ts create mode 100644 packages-content-model/roosterjs-content-model-editor/test/publicApi/selection/getSelectedSegmentsTest.ts diff --git a/packages-content-model/roosterjs-content-model-editor/lib/editor/plugins/PastePlugin/ContentModelPastePlugin.ts b/packages-content-model/roosterjs-content-model-editor/lib/editor/plugins/PastePlugin/ContentModelPastePlugin.ts index 7360aed2410..5b60805a5d4 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/editor/plugins/PastePlugin/ContentModelPastePlugin.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/editor/plugins/PastePlugin/ContentModelPastePlugin.ts @@ -78,6 +78,7 @@ export default class ContentModelPastePlugin implements EditorPlugin { if (!ev.domToModelOption) { return; } + const pasteSource = getPasteSource(event, false); switch (pasteSource) { case KnownPasteSourceType.WordDesktop: diff --git a/packages-content-model/roosterjs-content-model-editor/lib/index.ts b/packages-content-model/roosterjs-content-model-editor/lib/index.ts index babe9d24c2c..b125c62979b 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/index.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/index.ts @@ -42,6 +42,7 @@ export { default as setListStartNumber } from './publicApi/list/setListStartNumb export { default as hasSelectionInBlock } from './publicApi/selection/hasSelectionInBlock'; export { default as hasSelectionInSegment } from './publicApi/selection/hasSelectionInSegment'; export { default as hasSelectionInBlockGroup } from './publicApi/selection/hasSelectionInBlockGroup'; +export { default as getSelectedSegments } from './publicApi/selection/getSelectedSegments'; export { default as setIndentation } from './publicApi/block/setIndentation'; export { default as setAlignment } from './publicApi/block/setAlignment'; export { default as setDirection } from './publicApi/block/setDirection'; diff --git a/packages-content-model/roosterjs-content-model-editor/lib/modelApi/selection/collectSelections.ts b/packages-content-model/roosterjs-content-model-editor/lib/modelApi/selection/collectSelections.ts index 4381d23bfca..2cbab5bd76e 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/modelApi/selection/collectSelections.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/modelApi/selection/collectSelections.ts @@ -49,16 +49,6 @@ export function getSelectedSegmentsAndParagraphs( return result; } -/** - * @internal - */ -export function getSelectedSegments( - model: ContentModelDocument, - includingFormatHolder: boolean -): ContentModelSegment[] { - return getSelectedSegmentsAndParagraphs(model, includingFormatHolder).map(x => x[0]); -} - /** * @internal */ diff --git a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/link/adjustLinkSelection.ts b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/link/adjustLinkSelection.ts index f6672103ef1..7cbcfc0bd06 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/link/adjustLinkSelection.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/link/adjustLinkSelection.ts @@ -1,7 +1,7 @@ +import getSelectedSegments from '../selection/getSelectedSegments'; import { adjustSegmentSelection } from '../../modelApi/selection/adjustSegmentSelection'; import { adjustWordSelection } from '../../modelApi/selection/adjustWordSelection'; import { formatWithContentModel } from '../utils/formatWithContentModel'; -import { getSelectedSegments } from '../../modelApi/selection/collectSelections'; import { IContentModelEditor } from '../../publicTypes/IContentModelEditor'; import { setSelection } from '../../modelApi/selection/setSelection'; diff --git a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/link/insertLink.ts b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/link/insertLink.ts index c08b1fb6fc2..d2dc3b872b3 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/link/insertLink.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/link/insertLink.ts @@ -1,9 +1,9 @@ +import getSelectedSegments from '../selection/getSelectedSegments'; import { ChangeSource } from 'roosterjs-editor-types'; import { ContentModelLink } from 'roosterjs-content-model-types'; import { formatWithContentModel } from '../utils/formatWithContentModel'; import { getOnDeleteEntityCallback } from '../../editor/utils/handleKeyboardEventCommon'; import { getPendingFormat } from '../../modelApi/format/pendingFormat'; -import { getSelectedSegments } from '../../modelApi/selection/collectSelections'; import { HtmlSanitizer, matchLink } from 'roosterjs-editor-dom'; import { IContentModelEditor } from '../../publicTypes/IContentModelEditor'; import { mergeModel } from '../../modelApi/common/mergeModel'; diff --git a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/link/removeLink.ts b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/link/removeLink.ts index 3e04ba0ab2c..a874b404731 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/link/removeLink.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/link/removeLink.ts @@ -1,6 +1,6 @@ +import getSelectedSegments from '../selection/getSelectedSegments'; import { adjustSegmentSelection } from '../../modelApi/selection/adjustSegmentSelection'; import { formatWithContentModel } from '../utils/formatWithContentModel'; -import { getSelectedSegments } from '../../modelApi/selection/collectSelections'; import { IContentModelEditor } from '../../publicTypes/IContentModelEditor'; /** diff --git a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/selection/getSelectedSegments.ts b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/selection/getSelectedSegments.ts new file mode 100644 index 00000000000..988c843092e --- /dev/null +++ b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/selection/getSelectedSegments.ts @@ -0,0 +1,12 @@ +import { ContentModelDocument, ContentModelSegment } from 'roosterjs-content-model-types'; +import { getSelectedSegmentsAndParagraphs } from '../../modelApi/selection/collectSelections'; + +/** + * Get selected segments from a content model + */ +export default function getSelectedSegments( + model: ContentModelDocument, + includingFormatHolder: boolean +): ContentModelSegment[] { + return getSelectedSegmentsAndParagraphs(model, includingFormatHolder).map(x => x[0]); +} diff --git a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts index 6709c56117f..dca7089fb00 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts @@ -1,11 +1,13 @@ -import ContentModelBeforePasteEvent from '../../publicTypes/event/ContentModelBeforePasteEvent'; -import { BeforePasteEvent, NodePosition } from 'roosterjs-editor-types'; import { ContentModelBlockFormat, FormatParser } from 'roosterjs-content-model-types'; import { domToContentModel } from 'roosterjs-content-model-dom'; import { formatWithContentModel } from './formatWithContentModel'; import { getOnDeleteEntityCallback } from '../../editor/utils/handleKeyboardEventCommon'; import { IContentModelEditor } from '../../publicTypes/IContentModelEditor'; import { mergeModel } from '../../modelApi/common/mergeModel'; +import { NodePosition } from 'roosterjs-editor-types'; +import ContentModelBeforePasteEvent, { + ContentModelBeforePasteEventData, +} from '../../publicTypes/event/ContentModelBeforePasteEvent'; import { createDefaultHtmlSanitizerOptions, getPasteType, @@ -45,32 +47,32 @@ export default function paste( clipboardData.snapshotBeforePaste = editor.getContent(GetContentMode.RawHTMLWithSelection); } - const event = createBeforePasteEvent( + const eventData = createBeforePasteEventData( editor, clipboardData, getPasteType(pasteAsText, applyCurrentFormat, pasteAsImage) ); - const fragment = createFragmentFromClipboardData( + const { pluginEvent, fragment } = createBeforePasteEventAndFragment( editor, clipboardData, null /* position */, pasteAsText, pasteAsImage, - event + eventData ); const pasteModel = domToContentModel(fragment, { - ...event.domToModelOption, + ...pluginEvent.domToModelOption, disableCacheElement: true, additionalFormatParsers: { - ...event.domToModelOption.additionalFormatParsers, + ...pluginEvent.domToModelOption.additionalFormatParsers, block: [ - ...(event.domToModelOption.additionalFormatParsers?.block || []), + ...(pluginEvent.domToModelOption.additionalFormatParsers?.block || []), ...(applyCurrentFormat ? [blockElementParser] : []), ], listLevel: [ - ...(event.domToModelOption.additionalFormatParsers?.listLevel || []), + ...(pluginEvent.domToModelOption.additionalFormatParsers?.listLevel || []), ...(applyCurrentFormat ? [blockElementParser] : []), ], }, @@ -81,12 +83,16 @@ export default function paste( editor, 'Paste', model => { - mergeModel(model, pasteModel, getOnDeleteEntityCallback(editor), { - mergeFormat: applyCurrentFormat ? 'keepSourceEmphasisFormat' : 'none', - mergeTable: - pasteModel.blocks.length === 1 && - pasteModel.blocks[0].blockType === 'Table', - }); + if (pluginEvent.customizedMerge) { + pluginEvent.customizedMerge(model, pasteModel); + } else { + mergeModel(model, pasteModel, getOnDeleteEntityCallback(editor), { + mergeFormat: applyCurrentFormat ? 'keepSourceEmphasisFormat' : 'none', + mergeTable: + pasteModel.blocks.length === 1 && + pasteModel.blocks[0].blockType === 'Table', + }); + } return true; }, { @@ -97,18 +103,17 @@ export default function paste( } } -function createBeforePasteEvent( +function createBeforePasteEventData( editor: IContentModelEditor, clipboardData: ClipboardData, pasteType: PasteType -): ContentModelBeforePasteEvent { +): ContentModelBeforePasteEventData { const options = createDefaultHtmlSanitizerOptions(); // Remove "caret-color" style generated by Safari to make sure caret shows in right color after paste options.cssStyleCallbacks['caret-color'] = () => false; return { - eventType: PluginEventType.BeforePaste, clipboardData, fragment: editor.getDocument().createDocumentFragment(), sanitizingOption: options, @@ -120,14 +125,21 @@ function createBeforePasteEvent( }; } -function createFragmentFromClipboardData( +function createBeforePasteEventAndFragment( editor: IContentModelEditor, clipboardData: ClipboardData, position: NodePosition | null, pasteAsText: boolean, pasteAsImage: boolean, - event: BeforePasteEvent + eventData: ContentModelBeforePasteEventData ) { + // Step 1: Trigger BeforePasteEvent so that plugins can do proper change before paste + // and create the proper fragment for paste + const event = { + eventType: PluginEventType.BeforePaste, + ...eventData, + } as ContentModelBeforePasteEvent; + const { fragment } = event; const { rawHtml, text, imageDataUri } = clipboardData; const trustedHTMLHandler = editor.getTrustedHTMLHandler(); @@ -150,13 +162,16 @@ function createFragmentFromClipboardData( handleTextPaste(text, position, fragment); } - // Step 4: Trigger BeforePasteEvent so that plugins can do proper change before paste - editor.triggerPluginEvent(PluginEventType.BeforePaste, event, true /* broadcast */); + const pluginEvent = editor.triggerPluginEvent( + PluginEventType.BeforePaste, + eventData, + true /* broadcast */ + ) as ContentModelBeforePasteEvent; - // Step 5. Sanitize the fragment before paste to make sure the content is safe + // Step 4. Sanitize the fragment before paste to make sure the content is safe sanitizePasteContent(event, position); - return fragment; + return { fragment, pluginEvent }; } /** diff --git a/packages-content-model/roosterjs-content-model-editor/lib/publicTypes/event/ContentModelBeforePasteEvent.ts b/packages-content-model/roosterjs-content-model-editor/lib/publicTypes/event/ContentModelBeforePasteEvent.ts index 5e2266dde9c..fdb3aee34c6 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/publicTypes/event/ContentModelBeforePasteEvent.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/publicTypes/event/ContentModelBeforePasteEvent.ts @@ -1,4 +1,4 @@ -import { DomToModelOption } from 'roosterjs-content-model-types'; +import { ContentModelDocument, DomToModelOption } from 'roosterjs-content-model-types'; import { BeforePasteEvent, BeforePasteEventData, @@ -13,6 +13,10 @@ export interface ContentModelBeforePasteEventData extends BeforePasteEventData { * domToModel Options to use when creating the content model from the paste fragment */ domToModelOption: Partial; + /** + * customizedMerge Customized merge function to use when merging the paste fragment into the editor + */ + customizedMerge?: (target: ContentModelDocument, source: ContentModelDocument) => void; } /** diff --git a/packages-content-model/roosterjs-content-model-editor/test/modelApi/selection/collectSelectionsTest.ts b/packages-content-model/roosterjs-content-model-editor/test/modelApi/selection/collectSelectionsTest.ts index 0402cceede7..1e88ac3f0f4 100644 --- a/packages-content-model/roosterjs-content-model-editor/test/modelApi/selection/collectSelectionsTest.ts +++ b/packages-content-model/roosterjs-content-model-editor/test/modelApi/selection/collectSelectionsTest.ts @@ -22,7 +22,6 @@ import { ContentModelTable, } from 'roosterjs-content-model-types'; import { - getSelectedSegments, getSelectedParagraphs, getFirstSelectedListItem, getFirstSelectedTable, @@ -207,162 +206,6 @@ describe('getSelectedSegmentsAndParagraphs', () => { }); }); -describe('getSelectedSegments', () => { - function runTest( - selections: SelectionInfo[], - includingFormatHolder: boolean, - expectedResult: ContentModelSegment[] - ) { - spyOn(iterateSelections, 'iterateSelections').and.callFake((_, callback) => { - selections.forEach(({ path, tableContext, block, segments }) => { - callback(path, tableContext, block, segments); - }); - - return false; - }); - - const result = getSelectedSegments(null!, includingFormatHolder); - - expect(result).toEqual(expectedResult); - } - - it('Empty result', () => { - runTest([], false, []); - }); - - it('Add segments', () => { - const s1 = createText('test1'); - const s2 = createText('test2'); - const s3 = createText('test3'); - const s4 = createText('test4'); - const p1 = createParagraph(); - const p2 = createParagraph(); - - runTest( - [ - { - path: [], - block: p1, - segments: [s1, s2], - }, - { - path: [], - block: p2, - segments: [s3, s4], - }, - ], - false, - [s1, s2, s3, s4] - ); - }); - - it('Block with incompatible types', () => { - const s1 = createText('test1'); - const s2 = createText('test2'); - const s3 = createText('test3'); - const s4 = createText('test4'); - const b1 = createDivider('div'); - - runTest( - [ - { - path: [], - block: b1, - segments: [s1, s2], - }, - { - path: [], - segments: [s3, s4], - }, - ], - false, - [] - ); - }); - - it('Block with incompatible types, include format holder', () => { - const s1 = createText('test1'); - const s2 = createText('test2'); - const s3 = createText('test3'); - const s4 = createText('test4'); - const b1 = createDivider('div'); - - runTest( - [ - { - path: [], - block: b1, - segments: [s1, s2], - }, - { - path: [], - segments: [s3, s4], - }, - ], - true, - [s3, s4] - ); - }); - - it('Unmeaningful segments should be included', () => { - const s1 = createText('test1'); - const s2 = createText('test2'); - const s3 = createText('test3'); - const s4 = createText('test4'); - const m1 = createSelectionMarker({ fontSize: '10px' }); - const m2 = createSelectionMarker({ fontSize: '20px' }); - const p1 = createParagraph(); - const p2 = createParagraph(); - const p3 = createParagraph(); - - p1.segments.push(s1, m1); - p2.segments.push(s2, s3); - p3.segments.push(m2, s4); - - runTest( - [ - { - path: [], - block: p1, - segments: [m1], - }, - { - path: [], - block: p2, - segments: [s2, s3], - }, - { - path: [], - block: p3, - segments: [m2], - }, - ], - true, - [m1, s2, s3, m2] - ); - }); - - it('Include editable entity, but filter out readonly entity', () => { - const e1 = createEntity(null!, true); - const e2 = createEntity(null!, false); - const p1 = createParagraph(); - - p1.segments.push(e1, e2); - - runTest( - [ - { - path: [], - block: p1, - segments: [e1, e2], - }, - ], - false, - [e2] - ); - }); -}); - describe('getSelectedParagraphs', () => { function runTest(selections: SelectionInfo[], expectedResult: ContentModelParagraph[]) { spyOn(iterateSelections, 'iterateSelections').and.callFake((_, callback) => { diff --git a/packages-content-model/roosterjs-content-model-editor/test/publicApi/selection/getSelectedSegmentsTest.ts b/packages-content-model/roosterjs-content-model-editor/test/publicApi/selection/getSelectedSegmentsTest.ts new file mode 100644 index 00000000000..b29fbda0d93 --- /dev/null +++ b/packages-content-model/roosterjs-content-model-editor/test/publicApi/selection/getSelectedSegmentsTest.ts @@ -0,0 +1,178 @@ +import * as iterateSelections from '../../../lib/modelApi/selection/iterateSelections'; +import getSelectedSegments from '../../../lib/publicApi/selection/getSelectedSegments'; +import { TableSelectionContext } from '../../../lib/publicTypes/selection/TableSelectionContext'; +import { + createDivider, + createEntity, + createParagraph, + createSelectionMarker, + createText, +} from 'roosterjs-content-model-dom'; +import { + ContentModelBlock, + ContentModelBlockGroup, + ContentModelSegment, +} from 'roosterjs-content-model-types'; + +interface SelectionInfo { + path: ContentModelBlockGroup[]; + segments?: ContentModelSegment[]; + block?: ContentModelBlock; + tableContext?: TableSelectionContext; +} + +describe('getSelectedSegments', () => { + function runTest( + selections: SelectionInfo[], + includingFormatHolder: boolean, + expectedResult: ContentModelSegment[] + ) { + spyOn(iterateSelections, 'iterateSelections').and.callFake((_, callback) => { + selections.forEach(({ path, tableContext, block, segments }) => { + callback(path, tableContext, block, segments); + }); + + return false; + }); + + const result = getSelectedSegments(null!, includingFormatHolder); + + expect(result).toEqual(expectedResult); + } + + it('Empty result', () => { + runTest([], false, []); + }); + + it('Add segments', () => { + const s1 = createText('test1'); + const s2 = createText('test2'); + const s3 = createText('test3'); + const s4 = createText('test4'); + const p1 = createParagraph(); + const p2 = createParagraph(); + + runTest( + [ + { + path: [], + block: p1, + segments: [s1, s2], + }, + { + path: [], + block: p2, + segments: [s3, s4], + }, + ], + false, + [s1, s2, s3, s4] + ); + }); + + it('Block with incompatible types', () => { + const s1 = createText('test1'); + const s2 = createText('test2'); + const s3 = createText('test3'); + const s4 = createText('test4'); + const b1 = createDivider('div'); + + runTest( + [ + { + path: [], + block: b1, + segments: [s1, s2], + }, + { + path: [], + segments: [s3, s4], + }, + ], + false, + [] + ); + }); + + it('Block with incompatible types, include format holder', () => { + const s1 = createText('test1'); + const s2 = createText('test2'); + const s3 = createText('test3'); + const s4 = createText('test4'); + const b1 = createDivider('div'); + + runTest( + [ + { + path: [], + block: b1, + segments: [s1, s2], + }, + { + path: [], + segments: [s3, s4], + }, + ], + true, + [s3, s4] + ); + }); + + it('Unmeaningful segments should be included', () => { + const s1 = createText('test1'); + const s2 = createText('test2'); + const s3 = createText('test3'); + const s4 = createText('test4'); + const m1 = createSelectionMarker({ fontSize: '10px' }); + const m2 = createSelectionMarker({ fontSize: '20px' }); + const p1 = createParagraph(); + const p2 = createParagraph(); + const p3 = createParagraph(); + + p1.segments.push(s1, m1); + p2.segments.push(s2, s3); + p3.segments.push(m2, s4); + + runTest( + [ + { + path: [], + block: p1, + segments: [m1], + }, + { + path: [], + block: p2, + segments: [s2, s3], + }, + { + path: [], + block: p3, + segments: [m2], + }, + ], + true, + [m1, s2, s3, m2] + ); + }); + + it('Include editable entity, but filter out readonly entity', () => { + const e1 = createEntity(null!, true); + const e2 = createEntity(null!, false); + const p1 = createParagraph(); + + p1.segments.push(e1, e2); + + runTest( + [ + { + path: [], + block: p1, + segments: [e1, e2], + }, + ], + false, + [e2] + ); + }); +}); diff --git a/packages-content-model/roosterjs-content-model-editor/test/publicApi/utils/pasteTest.ts b/packages-content-model/roosterjs-content-model-editor/test/publicApi/utils/pasteTest.ts index 9d6daaafbf7..da35cb4e03c 100644 --- a/packages-content-model/roosterjs-content-model-editor/test/publicApi/utils/pasteTest.ts +++ b/packages-content-model/roosterjs-content-model-editor/test/publicApi/utils/pasteTest.ts @@ -1,7 +1,7 @@ import * as domToContentModel from 'roosterjs-content-model-dom/lib/domToModel/domToContentModel'; import * as mergeModelFile from '../../../lib/modelApi/common/mergeModel'; import paste from '../../../lib/publicApi/utils/paste'; -import { ClipboardData } from 'roosterjs-editor-types'; +import { ClipboardData, PasteType } from 'roosterjs-editor-types'; import { ContentModelDocument } from 'roosterjs-content-model-types'; import { IContentModelEditor } from '../../../lib/publicTypes/IContentModelEditor'; @@ -50,11 +50,33 @@ describe('Paste ', () => { focus = jasmine.createSpy('focus'); getFocusedPosition = jasmine.createSpy('getFocusedPosition').and.returnValue(mockedPos); getContent = jasmine.createSpy('getContent'); - triggerPluginEvent = jasmine.createSpy('triggerPluginEvent'); getDocument = jasmine.createSpy('getDocument').and.returnValue(document); + triggerPluginEvent = jasmine.createSpy('triggerPluginEvent').and.returnValue({ + clipboardData, + fragment: document.createDocumentFragment(), + sanitizingOption: { + elementCallbacks: {}, + attributeCallbacks: {}, + cssStyleCallbacks: {}, + additionalTagReplacements: {}, + additionalAllowedAttributes: [], + additionalAllowedCssClasses: [], + additionalDefaultStyleValues: {}, + additionalGlobalStyleNodes: [], + additionalPredefinedCssForElement: {}, + preserveHtmlComments: false, + unknownTagReplacement: null, + }, + htmlBefore: '', + htmlAfter: '', + htmlAttributes: {}, + domToModelOption: {}, + pasteType: PasteType.Normal, + }); getTrustedHTMLHandler = jasmine .createSpy('getTrustedHTMLHandler') .and.returnValue((html: string) => html); + spyOn(mergeModelFile, 'mergeModel').and.callFake(() => (mockedModel = mockedMergeModel)); editor = ({ From 4c74440e7f372e5c0af6abbda586c71141d72c2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Roldi?= Date: Mon, 31 Jul 2023 13:33:15 -0300 Subject: [PATCH 03/17] remove empty space --- .../lib/editor/plugins/PastePlugin/ContentModelPastePlugin.ts | 1 - .../test/publicApi/utils/pasteTest.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/packages-content-model/roosterjs-content-model-editor/lib/editor/plugins/PastePlugin/ContentModelPastePlugin.ts b/packages-content-model/roosterjs-content-model-editor/lib/editor/plugins/PastePlugin/ContentModelPastePlugin.ts index 5b60805a5d4..7360aed2410 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/editor/plugins/PastePlugin/ContentModelPastePlugin.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/editor/plugins/PastePlugin/ContentModelPastePlugin.ts @@ -78,7 +78,6 @@ export default class ContentModelPastePlugin implements EditorPlugin { if (!ev.domToModelOption) { return; } - const pasteSource = getPasteSource(event, false); switch (pasteSource) { case KnownPasteSourceType.WordDesktop: diff --git a/packages-content-model/roosterjs-content-model-editor/test/publicApi/utils/pasteTest.ts b/packages-content-model/roosterjs-content-model-editor/test/publicApi/utils/pasteTest.ts index da35cb4e03c..885815ae29f 100644 --- a/packages-content-model/roosterjs-content-model-editor/test/publicApi/utils/pasteTest.ts +++ b/packages-content-model/roosterjs-content-model-editor/test/publicApi/utils/pasteTest.ts @@ -76,7 +76,6 @@ describe('Paste ', () => { getTrustedHTMLHandler = jasmine .createSpy('getTrustedHTMLHandler') .and.returnValue((html: string) => html); - spyOn(mergeModelFile, 'mergeModel').and.callFake(() => (mockedModel = mockedMergeModel)); editor = ({ From c82803870482da91406e22f5df1c7d747e1a5013 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Roldi?= Date: Mon, 31 Jul 2023 13:45:43 -0300 Subject: [PATCH 04/17] fix comment --- .../lib/publicApi/utils/paste.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts index dca7089fb00..81d0f7e9e5f 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts @@ -133,8 +133,6 @@ function createBeforePasteEventAndFragment( pasteAsImage: boolean, eventData: ContentModelBeforePasteEventData ) { - // Step 1: Trigger BeforePasteEvent so that plugins can do proper change before paste - // and create the proper fragment for paste const event = { eventType: PluginEventType.BeforePaste, ...eventData, @@ -162,13 +160,14 @@ function createBeforePasteEventAndFragment( handleTextPaste(text, position, fragment); } + // Step 5: Trigger BeforePasteEvent so that plugins can do proper change before paste const pluginEvent = editor.triggerPluginEvent( PluginEventType.BeforePaste, eventData, true /* broadcast */ ) as ContentModelBeforePasteEvent; - // Step 4. Sanitize the fragment before paste to make sure the content is safe + // Step 5. Sanitize the fragment before paste to make sure the content is safe sanitizePasteContent(event, position); return { fragment, pluginEvent }; From 6fe8a9149cac9c0e4eba9e9b50e7f9344ed74a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Roldi?= Date: Mon, 31 Jul 2023 13:46:43 -0300 Subject: [PATCH 05/17] fix comment --- .../roosterjs-content-model-editor/lib/publicApi/utils/paste.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts index 81d0f7e9e5f..413ca56a9f0 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts @@ -160,7 +160,7 @@ function createBeforePasteEventAndFragment( handleTextPaste(text, position, fragment); } - // Step 5: Trigger BeforePasteEvent so that plugins can do proper change before paste + // Step 4: Trigger BeforePasteEvent so that plugins can do proper change before paste const pluginEvent = editor.triggerPluginEvent( PluginEventType.BeforePaste, eventData, From 808c91f5f81111629de0b4e82d46f1d6a3eb99ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Roldi?= Date: Mon, 31 Jul 2023 14:09:18 -0300 Subject: [PATCH 06/17] remane function and add comments --- .../lib/publicApi/utils/paste.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts index 413ca56a9f0..e7201027454 100644 --- a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts +++ b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts @@ -53,7 +53,7 @@ export default function paste( getPasteType(pasteAsText, applyCurrentFormat, pasteAsImage) ); - const { pluginEvent, fragment } = createBeforePasteEventAndFragment( + const { pluginEvent, fragment } = triggerPluginEventAndCreatePasteFragment( editor, clipboardData, null /* position */, @@ -125,14 +125,18 @@ function createBeforePasteEventData( }; } -function createBeforePasteEventAndFragment( +/** + * This function is used to create a BeforePasteEvent object after trigger the event, so other plugins can modify the event object + * This function will also create a DocumentFragment for paste. + */ +function triggerPluginEventAndCreatePasteFragment( editor: IContentModelEditor, clipboardData: ClipboardData, position: NodePosition | null, pasteAsText: boolean, pasteAsImage: boolean, eventData: ContentModelBeforePasteEventData -) { +): { pluginEvent: ContentModelBeforePasteEvent; fragment: DocumentFragment } { const event = { eventType: PluginEventType.BeforePaste, ...eventData, From d0a934bcc3ee1ccb646a54b4bd98e7431a4cb2b6 Mon Sep 17 00:00:00 2001 From: Jiuqing Song Date: Tue, 1 Aug 2023 13:22:21 -0700 Subject: [PATCH 07/17] Fix test cases for Firefox 116 (#1996) * Fix test cases for Firefox 116 * remove unnecessary change --- .../test/endToEndTest.ts | 3 +- .../backgroundColorFormatHandlerTest.ts | 7 ++- .../common/borderFormatHandlerTest.ts | 2 +- .../segment/textColorFormatHandlerTest.ts | 7 ++- .../modelToDom/handlers/handleListItemTest.ts | 6 +- .../modelToDom/handlers/handleListTest.ts | 60 ++++++------------- .../paste/processPastedContentFromWacTest.ts | 2 +- ...processPastedContentFromWordDesktopTest.ts | 25 +++++--- .../test/format/clearFormatTest.ts | 29 +++++---- .../test/DomTestHelper.ts | 5 ++ .../test/list/VListChainTest.ts | 45 ++++++++++---- 11 files changed, 106 insertions(+), 85 deletions(-) diff --git a/packages-content-model/roosterjs-content-model-dom/test/endToEndTest.ts b/packages-content-model/roosterjs-content-model-dom/test/endToEndTest.ts index 404f941078b..2e76b12a80a 100644 --- a/packages-content-model/roosterjs-content-model-dom/test/endToEndTest.ts +++ b/packages-content-model/roosterjs-content-model-dom/test/endToEndTest.ts @@ -2,6 +2,7 @@ import * as createGeneralBlock from '../lib/modelApi/creators/createGeneralBlock import DarkColorHandlerImpl from 'roosterjs-editor-core/lib/editor/DarkColorHandlerImpl'; import { contentModelToDom } from '../lib/modelToDom/contentModelToDom'; import { domToContentModel } from '../lib/domToModel/domToContentModel'; +import { expectHtml } from 'roosterjs-editor-api/test/TestHelper'; import { ContentModelBlockFormat, ContentModelDocument, @@ -46,7 +47,7 @@ describe('End to end test for DOM => Model', () => { expectedHTMLFirefox, //firefox ]; - expect(possibleHTML.indexOf(div2.innerHTML)).toBeGreaterThanOrEqual(0, div2.innerHTML); + expectHtml(div2.innerHTML, possibleHTML); } it('List with margin', () => { diff --git a/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/common/backgroundColorFormatHandlerTest.ts b/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/common/backgroundColorFormatHandlerTest.ts index a0b91be6abf..fe411cc63c3 100644 --- a/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/common/backgroundColorFormatHandlerTest.ts +++ b/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/common/backgroundColorFormatHandlerTest.ts @@ -2,6 +2,7 @@ import DarkColorHandlerImpl from 'roosterjs-editor-core/lib/editor/DarkColorHand import { backgroundColorFormatHandler } from '../../../lib/formatHandlers/common/backgroundColorFormatHandler'; import { createDomToModelContext } from '../../../lib/domToModel/context/createDomToModelContext'; import { createModelToDomContext } from '../../../lib/modelToDom/context/createModelToDomContext'; +import { expectHtml } from 'roosterjs-editor-dom/test/DomTestHelper'; import { BackgroundColorFormat, DomToModelContext, @@ -97,11 +98,11 @@ describe('backgroundColorFormatHandler.apply', () => { backgroundColorFormatHandler.apply(format, div, context); - const result = [ + const expectedResult = [ '
', '
', - ].indexOf(div.outerHTML); + ]; - expect(result).toBeGreaterThanOrEqual(0, div.outerHTML); + expectHtml(div.outerHTML, expectedResult); }); }); diff --git a/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/common/borderFormatHandlerTest.ts b/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/common/borderFormatHandlerTest.ts index f5f534cf9b1..c0d0fe7d493 100644 --- a/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/common/borderFormatHandlerTest.ts +++ b/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/common/borderFormatHandlerTest.ts @@ -51,7 +51,7 @@ describe('borderFormatHandler.parse', () => { }); }); - it('Has border width none value', () => { + itChromeOnly('Has border width none value', () => { div.style.borderWidth = '1px 2px 3px 4px'; div.style.borderStyle = 'none'; div.style.borderColor = 'red'; diff --git a/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/segment/textColorFormatHandlerTest.ts b/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/segment/textColorFormatHandlerTest.ts index 5973112beeb..dc60a6702ee 100644 --- a/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/segment/textColorFormatHandlerTest.ts +++ b/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/segment/textColorFormatHandlerTest.ts @@ -1,6 +1,7 @@ import DarkColorHandlerImpl from 'roosterjs-editor-core/lib/editor/DarkColorHandlerImpl'; import { createDomToModelContext } from '../../../lib/domToModel/context/createDomToModelContext'; import { createModelToDomContext } from '../../../lib/modelToDom/context/createModelToDomContext'; +import { expectHtml } from 'roosterjs-editor-dom/test/DomTestHelper'; import { textColorFormatHandler } from '../../../lib/formatHandlers/segment/textColorFormatHandler'; import { DomToModelContext, @@ -121,12 +122,12 @@ describe('textColorFormatHandler.apply', () => { textColorFormatHandler.apply(format, div, context); - const result = [ + const expectedResult = [ '
', '
', - ].indexOf(div.outerHTML); + ]; - expect(result).toBeGreaterThanOrEqual(0, div.outerHTML); + expectHtml(div.outerHTML, expectedResult); }); it('HyperLink without color', () => { diff --git a/packages-content-model/roosterjs-content-model-dom/test/modelToDom/handlers/handleListItemTest.ts b/packages-content-model/roosterjs-content-model-dom/test/modelToDom/handlers/handleListItemTest.ts index 062d6e996af..f7603e99f03 100644 --- a/packages-content-model/roosterjs-content-model-dom/test/modelToDom/handlers/handleListItemTest.ts +++ b/packages-content-model/roosterjs-content-model-dom/test/modelToDom/handlers/handleListItemTest.ts @@ -3,6 +3,7 @@ import { createListItem } from '../../../lib/modelApi/creators/createListItem'; import { createListLevel } from '../../../lib/modelApi/creators/createListLevel'; import { createModelToDomContext } from '../../../lib/modelToDom/context/createModelToDomContext'; import { createParagraph } from '../../../lib/modelApi/creators/createParagraph'; +import { expectHtml } from 'roosterjs-editor-dom/test/DomTestHelper'; import { handleList as originalHandleList } from '../../../lib/modelToDom/handlers/handleList'; import { handleListItem } from '../../../lib/modelToDom/handlers/handleListItem'; import { listItemMetadataFormatHandler } from '../../../lib/formatHandlers/list/listItemMetadataFormatHandler'; @@ -249,10 +250,7 @@ describe('handleListItem', () => { '
', ]; - expect(expectedResult.indexOf(parent.outerHTML)).toBeGreaterThanOrEqual( - 0, - parent.outerHTML - ); + expectHtml(parent.outerHTML, expectedResult); expect(context.listFormat).toEqual({ threadItemCounts: [1], nodeStack: [ diff --git a/packages-content-model/roosterjs-content-model-dom/test/modelToDom/handlers/handleListTest.ts b/packages-content-model/roosterjs-content-model-dom/test/modelToDom/handlers/handleListTest.ts index c7d6f7cdd3e..6757425bace 100644 --- a/packages-content-model/roosterjs-content-model-dom/test/modelToDom/handlers/handleListTest.ts +++ b/packages-content-model/roosterjs-content-model-dom/test/modelToDom/handlers/handleListTest.ts @@ -3,8 +3,8 @@ import { ContentModelListItem, ModelToDomContext } from 'roosterjs-content-model import { createListItem } from '../../../lib/modelApi/creators/createListItem'; import { createListLevel } from '../../../lib/modelApi/creators/createListLevel'; import { createModelToDomContext } from '../../../lib/modelToDom/context/createModelToDomContext'; +import { expectHtml, itChromeOnly } from 'roosterjs-editor-dom/test/DomTestHelper'; import { handleList } from '../../../lib/modelToDom/handlers/handleList'; -import { itChromeOnly } from 'roosterjs-editor-dom/test/DomTestHelper'; describe('handleList', () => { let context: ModelToDomContext; @@ -62,10 +62,7 @@ describe('handleList', () => { '
    ', //Firefox ]; - expect(possibleResults.indexOf(parent.outerHTML)).toBeGreaterThanOrEqual( - 0, - parent.outerHTML - ); + expectHtml(parent.outerHTML, possibleResults); expect(context.listFormat).toEqual({ threadItemCounts: [0], nodeStack: [ @@ -277,10 +274,8 @@ describe('handleList', () => { '
        ', //Firefox ]; - expect(possibleResults.indexOf(parent.outerHTML)).toBeGreaterThanOrEqual( - 0, - parent.outerHTML - ); + expectHtml(parent.outerHTML, possibleResults); + expect(context.listFormat).toEqual({ threadItemCounts: [1], nodeStack: [ @@ -322,10 +317,7 @@ describe('handleList', () => { '
            ', //Firefox ]; - expect(possibleResults.indexOf(parent.outerHTML)).toBeGreaterThanOrEqual( - 0, - parent.outerHTML - ); + expectHtml(parent.outerHTML, possibleResults); expect(context.listFormat).toEqual({ threadItemCounts: [1, 2], @@ -432,10 +424,8 @@ describe('handleList without format handlers', () => { '
              ', //Firefox ]; - expect(possibleResults.indexOf(parent.outerHTML)).toBeGreaterThanOrEqual( - 0, - parent.outerHTML - ); + expectHtml(parent.outerHTML, possibleResults); + expect(context.listFormat).toEqual({ threadItemCounts: [], nodeStack: [ @@ -634,10 +624,8 @@ describe('handleList without format handlers', () => { '
                  ', //Firefox ]; - expect(possibleResults.indexOf(parent.outerHTML)).toBeGreaterThanOrEqual( - 0, - parent.outerHTML - ); + expectHtml(parent.outerHTML, possibleResults); + expect(context.listFormat).toEqual({ threadItemCounts: [1], nodeStack: [ @@ -726,11 +714,10 @@ describe('handleList handles metadata', () => { const possibleResults = [ '
                    ', // Chrome '
                      ', // Firefox + '
                        ', // Firefox 116+ ]; - expect(possibleResults.indexOf(parent.innerHTML)).toBeGreaterThanOrEqual( - 0, - parent.innerHTML - ); + + expectHtml(parent.innerHTML, possibleResults); }); it('OL with metadata with simple value', () => { @@ -752,12 +739,10 @@ describe('handleList handles metadata', () => { const possibleResults = [ '
                          ', // Chrome '
                            ', // Firefox + '
                              ', // Firefox 116+ ]; - expect(possibleResults.indexOf(parent.innerHTML)).toBeGreaterThanOrEqual( - 0, - parent.innerHTML - ); + expectHtml(parent.innerHTML, possibleResults); }); it('UL with metadata with simple value', () => { @@ -780,10 +765,7 @@ describe('handleList handles metadata', () => { '
                                ', // Chrome '
                                  ', // Firefox ]; - expect(possibleResults.indexOf(parent.innerHTML)).toBeGreaterThanOrEqual( - 0, - parent.innerHTML - ); + expectHtml(parent.innerHTML, possibleResults); }); it('OL with refNode', () => { @@ -796,10 +778,8 @@ describe('handleList handles metadata', () => { const possibleResults = ['

                                    ']; - expect(possibleResults.indexOf(parent.outerHTML)).toBeGreaterThanOrEqual( - 0, - parent.outerHTML - ); + expectHtml(parent.outerHTML, possibleResults); + expect(context.listFormat).toEqual({ threadItemCounts: [0], nodeStack: [ @@ -834,10 +814,8 @@ describe('handleList handles metadata', () => { '

                                      ', //Firefox ]; - expect(possibleResults.indexOf(parent.outerHTML)).toBeGreaterThanOrEqual( - 0, - parent.outerHTML - ); + expectHtml(parent.outerHTML, possibleResults); + expect(context.listFormat).toEqual({ threadItemCounts: [1, 0], nodeStack: [ diff --git a/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/paste/processPastedContentFromWacTest.ts b/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/paste/processPastedContentFromWacTest.ts index 524b2a079b5..5053a5dba1a 100644 --- a/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/paste/processPastedContentFromWacTest.ts +++ b/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/paste/processPastedContentFromWacTest.ts @@ -1311,7 +1311,7 @@ describe('wordOnlineHandler', () => { }); describe('Contain Word WAC Image', () => { - it('Contain Single WAC Image', () => { + itChromeOnly('Contain Single WAC Image', () => { runTest( 'Graphical user interface, text, application Description automatically generated', undefined, diff --git a/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/paste/processPastedContentFromWordDesktopTest.ts b/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/paste/processPastedContentFromWordDesktopTest.ts index 008ce2dcdf0..b8f53cbf6b3 100644 --- a/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/paste/processPastedContentFromWordDesktopTest.ts +++ b/packages-content-model/roosterjs-content-model-editor/test/editor/plugins/paste/processPastedContentFromWordDesktopTest.ts @@ -1,15 +1,20 @@ import ContentModelBeforePasteEvent from '../../../../lib/publicTypes/event/ContentModelBeforePasteEvent'; -import { Browser, moveChildNodes } from 'roosterjs-editor-dom'; import { ClipboardData, PluginEventType } from 'roosterjs-editor-types'; import { ContentModelDocument } from 'roosterjs-content-model-types'; import { contentModelToDom, domToContentModel } from 'roosterjs-content-model-dom'; +import { expectHtml } from 'roosterjs-editor-api/test/TestHelper'; +import { moveChildNodes } from 'roosterjs-editor-dom'; import { processPastedContentFromWordDesktop } from '../../../../lib/editor/plugins/PastePlugin/WordDesktop/processPastedContentFromWordDesktop'; describe('processPastedContentFromWordDesktopTest', () => { let div: HTMLElement; let fragment: DocumentFragment; - function runTest(source?: string, expected?: string, expectedModel?: ContentModelDocument) { + function runTest( + source?: string, + expected?: string | string[], + expectedModel?: ContentModelDocument + ) { //Act if (source) { div = document.createElement('div'); @@ -40,7 +45,7 @@ describe('processPastedContentFromWordDesktopTest', () => { //Assert if (expected) { - expect(div.innerHTML).toBe(expected); + expectHtml(div.innerHTML, expected); } div.parentElement?.removeChild(div); } @@ -853,9 +858,10 @@ describe('processPastedContentFromWordDesktopTest', () => { '' + '' + '', - Browser.isFirefox - ? '
                                      1. 123123
                                        1. 123123
                                          1. 123123
                                            1. 123123123
                                      ' - : '
                                      1. 123123
                                        1. 123123
                                          1. 123123
                                            1. 123123123
                                      ', + [ + '
                                      1. 123123
                                        1. 123123
                                          1. 123123
                                            1. 123123123
                                      ', + '
                                      1. 123123
                                        1. 123123
                                          1. 123123
                                            1. 123123123
                                      ', + ], { blockGroupType: 'Document', blocks: [ @@ -1164,9 +1170,10 @@ describe('processPastedContentFromWordDesktopTest', () => { it('Word doc created online but edited and copied from Desktop', () => { runTest( '

                                      it went:

                                      1.     Test

                                      2.     Test2

                                      ', - Browser.isFirefox - ? '

                                      it went:

                                      1. Test
                                      2. Test2
                                      ' - : '

                                      it went:

                                      1. Test
                                      2. Test2
                                      ', + [ + '

                                      it went:

                                      1. Test
                                      2. Test2
                                      ', + '

                                      it went:

                                      1. Test
                                      2. Test2
                                      ', + ], { blockGroupType: 'Document', blocks: [ diff --git a/packages/roosterjs-editor-api/test/format/clearFormatTest.ts b/packages/roosterjs-editor-api/test/format/clearFormatTest.ts index 5f12ea78e23..68cee8b98da 100644 --- a/packages/roosterjs-editor-api/test/format/clearFormatTest.ts +++ b/packages/roosterjs-editor-api/test/format/clearFormatTest.ts @@ -210,8 +210,10 @@ describe('clearAutodetectFormat tests', () => { () => { const originalContent = '

                                      • item 2 with more text
                                      • Item 2 
                                      • Sdasd 
                                      • asdasd 

                                      '; - const expectedFormat = - '

                                      item 2 with more text
                                      • Item 2 
                                      • Sdasd 
                                      • asdasd 

                                      '; + const expectedFormat = [ + '

                                      item 2 with more text
                                      • Item 2 
                                      • Sdasd 
                                      • asdasd 

                                      ', + '

                                      item 2 with more text
                                      • Item 2 
                                      • Sdasd 
                                      • asdasd 

                                      ', + ]; editor.setContent(originalContent); const ul = doc.getElementsByTagName('ul')[0]; @@ -224,7 +226,7 @@ describe('clearAutodetectFormat tests', () => { clearFormat(editor, ClearFormatMode.AutoDetect); - expect(editor.getContent()).toBe(expectedFormat); + TestHelper.expectHtml(editor.getContent(), expectedFormat); } ); @@ -299,8 +301,10 @@ describe('clearAutodetectFormat Partial Tests', () => { TestHelper.itFirefoxOnly('removes format of partial selected element inside a LI', () => { const originalContent = '

                                      • item 1 
                                      • Item 2 
                                      • Sdasd 
                                      • asdasd 

                                      '; - const expectedFormat = - '

                                      • item 1 
                                      • Item 2 
                                      • Sdasd 
                                      • asdasd 

                                      '; + const expectedFormat = [ + '

                                      • item 1 
                                      • Item 2 
                                      • Sdasd 
                                      • asdasd 

                                      ', + '

                                      • item 1 
                                      • Item 2 
                                      • Sdasd 
                                      • asdasd 

                                      ', + ]; editor.setContent(originalContent); const ul = doc.getElementsByTagName('ul')[0]; @@ -313,7 +317,7 @@ describe('clearAutodetectFormat Partial Tests', () => { clearFormat(editor, ClearFormatMode.AutoDetect); - expect(editor.getContent()).toBe(expectedFormat); + TestHelper.expectHtml(editor.getContent(), expectedFormat); }); }); @@ -346,7 +350,7 @@ describe('clearAutodetectFormat tests with defaultFormat | ', () => { function runTest( ogContent: string, - expectedContent: string | undefined, + expectedContent: string | undefined | string[], selectCallback: () => void, additionalExpects?: () => void ) { @@ -355,9 +359,11 @@ describe('clearAutodetectFormat tests with defaultFormat | ', () => { selectCallback(); clearFormat(editor, ClearFormatMode.AutoDetect); + if (expectedContent) { - expect(editor.getContent()).toBe(expectedContent); + TestHelper.expectHtml(editor.getContent(), expectedContent); } + additionalExpects?.(); } @@ -421,8 +427,11 @@ describe('clearAutodetectFormat tests with defaultFormat | ', () => { () => { const originalContent = '

                                      • item 2 with more text
                                      • Item 2 
                                      • Sdasd 
                                      • asdasd 

                                      '; - const expectedFormat = - '

                                      item 2 with more text
                                      • Item 2 
                                      • Sdasd 
                                      • asdasd 

                                      '; + const expectedFormat = [ + '

                                      item 2 with more text
                                      • Item 2 
                                      • Sdasd 
                                      • asdasd 

                                      ', + '

                                      item 2 with more text
                                      • Item 2 
                                      • Sdasd 
                                      • asdasd 

                                      ', + ]; + runTest(originalContent, expectedFormat, () => { const ul = doc.getElementsByTagName('ul')[0]; const li = ul.children[0]; diff --git a/packages/roosterjs-editor-dom/test/DomTestHelper.ts b/packages/roosterjs-editor-dom/test/DomTestHelper.ts index 6ab05327dc0..008a67edb86 100644 --- a/packages/roosterjs-editor-dom/test/DomTestHelper.ts +++ b/packages/roosterjs-editor-dom/test/DomTestHelper.ts @@ -121,3 +121,8 @@ export function itChromeOnly( const func = Browser.isChrome ? it : xit; return func(expectation, assertion, timeout); } + +export function expectHtml(actualHtml: string, expectedHtml: string | string[]) { + expectedHtml = Array.isArray(expectedHtml) ? expectedHtml : [expectedHtml]; + expect(expectedHtml.indexOf(actualHtml)).toBeGreaterThanOrEqual(0, actualHtml); +} diff --git a/packages/roosterjs-editor-dom/test/list/VListChainTest.ts b/packages/roosterjs-editor-dom/test/list/VListChainTest.ts index d164c7dbe61..e2f5b873adf 100644 --- a/packages/roosterjs-editor-dom/test/list/VListChainTest.ts +++ b/packages/roosterjs-editor-dom/test/list/VListChainTest.ts @@ -1,7 +1,7 @@ import getBlockElementAtNode from '../../lib/blockElements/getBlockElementAtNode'; import VListChain from '../../lib/list/VListChain'; import VListItem from '../../lib/list/VListItem'; -import { itFirefoxOnly } from '../DomTestHelper'; +import { expectHtml, itFirefoxOnly } from '../DomTestHelper'; import { ListType, PositionType } from 'roosterjs-editor-types'; import { Position } from 'roosterjs-editor-dom'; @@ -24,7 +24,7 @@ describe('createListChains', () => { expect(chains).toEqual([]); }); - function runTest(html: string, expectedHtml: string) { + function runTest(html: string, expectedHtml: string | string[]) { const div = document.createElement('div'); document.body.appendChild(div); div.innerHTML = html; @@ -39,7 +39,7 @@ describe('createListChains', () => { nameGenerator ); - expect(div.innerHTML).toBe(expectedHtml); + expectHtml(div.innerHTML, expectedHtml); document.body.removeChild(div); } @@ -71,42 +71,60 @@ describe('createListChains', () => { itFirefoxOnly('Two continuously lists', () => { runTest( '
                                      1. item1
                                      2. item2
                                      test
                                      1. item3
                                      ', - `
                                      1. item1
                                      2. item2
                                      test
                                      1. item3
                                      ` + [ + `
                                      1. item1
                                      2. item2
                                      test
                                      1. item3
                                      `, + `
                                      1. item1
                                      2. item2
                                      test
                                      1. item3
                                      `, + ] ); }); itFirefoxOnly('Two list chains', () => { runTest( '
                                      1. item1
                                      2. item2
                                      3. item3
                                      test
                                      1. itemA
                                      2. itemB
                                      1. item4
                                      test
                                      1. itemC
                                      ', - `
                                      1. item1
                                      2. item2
                                      3. item3
                                      test
                                      1. itemA
                                      2. itemB
                                      1. item4
                                      test
                                      1. itemC
                                      ` + [ + `
                                      1. item1
                                      2. item2
                                      3. item3
                                      test
                                      1. itemA
                                      2. itemB
                                      1. item4
                                      test
                                      1. itemC
                                      `, + `
                                      1. item1
                                      2. item2
                                      3. item3
                                      test
                                      1. itemA
                                      2. itemB
                                      1. item4
                                      test
                                      1. itemC
                                      `, + ] ); }); itFirefoxOnly('Unordered list in a chain', () => { runTest( '
                                      1. item1
                                      2. item2
                                      test
                                      • test
                                      test
                                      1. item3
                                      ', - `
                                      1. item1
                                      2. item2
                                      test
                                      • test
                                      test
                                      1. item3
                                      ` + [ + `
                                      1. item1
                                      2. item2
                                      test
                                      • test
                                      test
                                      1. item3
                                      `, + `
                                      1. item1
                                      2. item2
                                      test
                                      • test
                                      test
                                      1. item3
                                      `, + ] ); }); itFirefoxOnly('Nested list', () => { runTest( '
                                      1. item1
                                        1. item1.1
                                        2. item1.2
                                      test
                                      1. item2
                                      ', - `
                                      1. item1
                                        1. item1.1
                                        2. item1.2
                                      test
                                      1. item2
                                      ` + [ + `
                                      1. item1
                                        1. item1.1
                                        2. item1.2
                                      test
                                      1. item2
                                      `, + `
                                      1. item1
                                        1. item1.1
                                        2. item1.2
                                      test
                                      1. item2
                                      `, + ] ); }); itFirefoxOnly('Nested list for separated lists', () => { runTest( '
                                      1. item1
                                        1. item1.1
                                        2. item1.2
                                      test
                                      1. item2
                                      ', - `
                                      1. item1
                                        1. item1.1
                                        2. item1.2
                                      test
                                      1. item2
                                      ` + [ + `
                                      1. item1
                                        1. item1.1
                                        2. item1.2
                                      test
                                      1. item2
                                      `, + `
                                      1. item1
                                        1. item1.1
                                        2. item1.2
                                      test
                                      1. item2
                                      `, + ] ); }); itFirefoxOnly('Current node', () => { runTest( `
                                      1. item1
                                        1. item1.1
                                        2. item1.2
                                      test
                                      1. item2
                                      `, - `
                                      1. item1
                                        1. item1.1
                                        2. item1.2
                                      test
                                      1. item2
                                      ` + [ + `
                                      1. item1
                                        1. item1.1
                                        2. item1.2
                                      test
                                      1. item2
                                      `, + `
                                      1. item1
                                        1. item1.1
                                        2. item1.2
                                      test
                                      1. item2
                                      `, + ] ); }); }); @@ -207,7 +225,7 @@ describe('VListChain.createVListAtNode', () => { function runTest( html: string, startNumber: number, - expectedHtml: string, + expectedHtml: string | string[], expectedItems: { listTypes: ListType[]; outerHTML: string }[] ) { const div = document.createElement('div'); @@ -239,7 +257,7 @@ describe('VListChain.createVListAtNode', () => { expect(vList).toBeNull(); } - expect(div.innerHTML).toBe(expectedHtml); + expectHtml(div.innerHTML, expectedHtml); document.body.removeChild(div); } @@ -257,7 +275,10 @@ describe('VListChain.createVListAtNode', () => { runTest( `
                                      1. item
                                      test
                                      `, 2, - `
                                      1. item
                                      1. test
                                      2. `, + [ + `
                                        1. item
                                        1. test
                                        2. `, + `
                                          1. item
                                          1. test
                                          2. `, + ], [ { listTypes: [ListType.None], From 457305f996d9a1246cb29ae5bafd923c94f65358 Mon Sep 17 00:00:00 2001 From: Andres-CT98 <107568016+Andres-CT98@users.noreply.github.com> Date: Tue, 1 Aug 2023 15:08:47 -0600 Subject: [PATCH 08/17] Fix table selector and resizer insertion (#1984) * initial attempt * try add container as parameter * keep old body behavior, add container parameter * fix demo * optimisations * cleanup * naming and comments * test fixed * pos: fixed * revert * refactor, add check for bottom resizer * fix demo site * refactor and fix tests * removed redundant code and renames --- demo/scripts/controls/BuildInPluginState.ts | 1 + demo/scripts/controls/MainPaneBase.tsx | 2 +- demo/scripts/controls/getToggleablePlugins.ts | 4 +- .../ContentModelEditorOptionsPlugin.ts | 1 + .../editorOptions/ContentModelOptionsPane.tsx | 1 + .../editorOptions/EditorOptionsPlugin.ts | 1 + .../sidePane/editorOptions/OptionsPane.tsx | 1 + .../lib/plugins/TableResize/TableResize.ts | 11 +++- .../TableResize/editors/TableEditor.ts | 12 +++-- .../TableResize/editors/TableResizer.ts | 53 +++++++++++++++---- .../TableResize/editors/TableSelector.ts | 34 ++++++++---- .../test/TableResize/tableSelectorTest.ts | 3 +- 12 files changed, 95 insertions(+), 29 deletions(-) diff --git a/demo/scripts/controls/BuildInPluginState.ts b/demo/scripts/controls/BuildInPluginState.ts index 4aa4f850a83..47ac1c80f26 100644 --- a/demo/scripts/controls/BuildInPluginState.ts +++ b/demo/scripts/controls/BuildInPluginState.ts @@ -34,6 +34,7 @@ export default interface BuildInPluginState { experimentalFeatures: ExperimentalFeatures[]; forcePreserveRatio: boolean; isRtl: boolean; + tableFeaturesContainerSelector: string; } export interface BuildInPluginProps extends BuildInPluginState, SidePaneElementProps {} diff --git a/demo/scripts/controls/MainPaneBase.tsx b/demo/scripts/controls/MainPaneBase.tsx index ff2fb6b97f7..ccd28c983a7 100644 --- a/demo/scripts/controls/MainPaneBase.tsx +++ b/demo/scripts/controls/MainPaneBase.tsx @@ -176,7 +176,7 @@ export default abstract class MainPaneBase extends React.Component<{}, MainPaneB this.updateContentPlugin.forceUpdate(); return ( -
                                            +
                                            {this.state.editorCreator && ( void + ) => void, + private anchorContainerSelector?: string ) {} /** @@ -131,11 +135,16 @@ export default class TableResize implements EditorPlugin { } if (!this.tableEditor && table && this.editor && table.rows.length > 0) { + const container = this.anchorContainerSelector + ? this.editor.getDocument().querySelector(this.anchorContainerSelector) + : undefined; + this.tableEditor = new TableEditor( this.editor, table, this.invalidateTableRects, this.onShowHelperElement, + safeInstanceOf(container, 'HTMLElement') ? container : undefined, e?.currentTarget ); } diff --git a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableEditor.ts b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableEditor.ts index e2650028815..d5146c2981e 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableEditor.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableEditor.ts @@ -81,6 +81,7 @@ export default class TableEditor { elementData: CreateElementData, helperType: 'CellResizer' | 'TableInserter' | 'TableResizer' | 'TableSelector' ) => void, + private anchorContainer?: HTMLElement, private contentDiv?: EventTarget | null ) { this.isRTL = getComputedStyle(table, 'direction') == 'rtl'; @@ -211,23 +212,24 @@ export default class TableEditor { if (!this.tableSelector) { this.tableSelector = createTableSelector( this.table, - this.editor.getZoomScale(), this.editor, this.onSelect, this.getOnMouseOut, this.onShowHelperElement, - this.contentDiv + this.contentDiv, + this.anchorContainer ); } if (!this.tableResizer) { this.tableResizer = createTableResizer( this.table, - this.editor.getZoomScale(), - this.isRTL, + this.editor, this.onStartTableResize, this.onFinishEditing, - this.onShowHelperElement + this.onShowHelperElement, + this.contentDiv, + this.anchorContainer ); } } diff --git a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableResizer.ts b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableResizer.ts index f383ffcb5b4..3214ee7f643 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableResizer.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableResizer.ts @@ -1,7 +1,13 @@ import DragAndDropHelper from '../../../pluginUtils/DragAndDropHelper'; import TableEditFeature from './TableEditorFeature'; -import { createElement, normalizeRect, VTable } from 'roosterjs-editor-dom'; -import { CreateElementData } from 'roosterjs-editor-types'; +import { CreateElementData, IEditor, Rect } from 'roosterjs-editor-types'; +import { + createElement, + getComputedStyle, + normalizeRect, + safeInstanceOf, + VTable, +} from 'roosterjs-editor-dom'; const TABLE_RESIZER_LENGTH = 12; const MIN_CELL_WIDTH = 30; @@ -12,16 +18,25 @@ const MIN_CELL_HEIGHT = 20; */ export default function createTableResizer( table: HTMLTableElement, - zoomScale: number, - isRTL: boolean, + editor: IEditor, onStart: () => void, onDragEnd: () => false, onShowHelperElement?: ( elementData: CreateElementData, helperType: 'CellResizer' | 'TableInserter' | 'TableResizer' | 'TableSelector' - ) => void + ) => void, + contentDiv?: EventTarget | null, + anchorContainer?: HTMLElement ): TableEditFeature | null { + const rect = normalizeRect(table.getBoundingClientRect()); + + if (!isTableBottomVisible(editor, rect, contentDiv)) { + return null; + } + const document = table.ownerDocument; + const isRTL = getComputedStyle(table, 'direction') == 'rtl'; + const zoomScale = editor.getZoomScale(); const createElementData = { tag: 'div', style: `position: fixed; cursor: ${ @@ -35,7 +50,8 @@ export default function createTableResizer( div.style.width = `${TABLE_RESIZER_LENGTH}px`; div.style.height = `${TABLE_RESIZER_LENGTH}px`; - document.body.appendChild(div); + + (anchorContainer || document.body).appendChild(div); const context: DragAndDropContext = { isRTL, @@ -44,12 +60,12 @@ export default function createTableResizer( onStart, }; - setResizeDivPosition(context, div); + setDivPosition(context, div); const featureHandler = new DragAndDropHelper( div, context, - setResizeDivPosition, + setDivPosition, { onDragStart, onDragging, @@ -137,7 +153,7 @@ function onDragging( } } -function setResizeDivPosition(context: DragAndDropContext, trigger: HTMLElement) { +function setDivPosition(context: DragAndDropContext, trigger: HTMLElement) { const { table, isRTL } = context; const rect = normalizeRect(table.getBoundingClientRect()); @@ -148,3 +164,22 @@ function setResizeDivPosition(context: DragAndDropContext, trigger: HTMLElement) : `${rect.right}px`; } } + +function isTableBottomVisible( + editor: IEditor, + rect: Rect | null, + contentDiv?: EventTarget | null +): boolean { + const visibleViewport = editor.getVisibleViewport(); + if (contentDiv && safeInstanceOf(contentDiv, 'HTMLElement') && visibleViewport && rect) { + const containerRect = normalizeRect(contentDiv.getBoundingClientRect()); + + return ( + !!containerRect && + containerRect.bottom >= rect.bottom && + visibleViewport.bottom >= rect.bottom + ); + } + + return true; +} diff --git a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableSelector.ts b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableSelector.ts index 35b507828a5..b1536f81847 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableSelector.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableSelector.ts @@ -1,8 +1,13 @@ 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 { + createElement, + normalizeRect, + safeInstanceOf, + getComputedStyle, +} from 'roosterjs-editor-dom'; const TABLE_SELECTOR_LENGTH = 12; const TABLE_SELECTOR_ID = '_Table_Selector'; @@ -12,7 +17,6 @@ const TABLE_SELECTOR_ID = '_Table_Selector'; */ export default function createTableSelector( table: HTMLTableElement, - zoomScale: number, editor: IEditor, onFinishDragging: (table: HTMLTableElement) => void, getOnMouseOut: (feature: HTMLElement) => (ev: MouseEvent) => void, @@ -20,7 +24,8 @@ export default function createTableSelector( elementData: CreateElementData, helperType: 'CellResizer' | 'TableInserter' | 'TableResizer' | 'TableSelector' ) => void, - contentDiv?: EventTarget | null + contentDiv?: EventTarget | null, + anchorContainer?: HTMLElement ): TableEditorFeature | null { const rect = normalizeRect(table.getBoundingClientRect()); @@ -28,6 +33,7 @@ export default function createTableSelector( return null; } + const zoomScale = editor.getZoomScale(); const document = table.ownerDocument; const createElementData = { tag: 'div', @@ -41,15 +47,17 @@ 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); + + (anchorContainer || document.body).appendChild(div); const context: TableSelectorContext = { table, zoomScale, rect, + isRTL: getComputedStyle(table, 'direction') == 'rtl', }; - setSelectorDivPosition(context, div); + setDivPosition(context, div); const onDragEnd = (context: TableSelectorContext, event: MouseEvent): false => { if (event.target == div) { @@ -61,11 +69,11 @@ export default function createTableSelector( const featureHandler = new TableSelectorFeature( div, context, - setSelectorDivPosition, + setDivPosition, { onDragEnd, }, - zoomScale, + context.zoomScale, getOnMouseOut ); @@ -76,6 +84,7 @@ interface TableSelectorContext { table: HTMLTableElement; zoomScale: number; rect: Rect | null; + isRTL: boolean; } interface TableSelectorInitValue { @@ -88,11 +97,16 @@ class TableSelectorFeature extends DragAndDropHelper void, + onSubmit: ( + context: TableSelectorContext, + trigger: HTMLElement, + container?: HTMLElement + ) => void, handler: DragAndDropHandler, zoomScale: number, getOnMouseOut: (feature: HTMLElement) => (ev: MouseEvent) => void, - forceMobile?: boolean + forceMobile?: boolean | undefined, + container?: HTMLElement ) { super(div, context, onSubmit, handler, zoomScale, forceMobile); this.onMouseOut = getOnMouseOut(div); @@ -108,7 +122,7 @@ class TableSelectorFeature extends DragAndDropHelper { //Act const result = createTableSelector( target as HTMLTableElement, - 1, editor, () => {}, () => () => {}, () => {}, - node + node ); //Assert From 1095411a7f6d17238769541ee87771e043305471 Mon Sep 17 00:00:00 2001 From: Jiuqing Song Date: Wed, 2 Aug 2023 13:46:38 -0700 Subject: [PATCH 09/17] Fix #1994: Do not do table or image selection in entity (#1995) --- .../lib/corePlugins/ImageSelection.ts | 1 + .../TableCellSelection/mouseUtils/handleMouseDownEvent.ts | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/roosterjs-editor-core/lib/corePlugins/ImageSelection.ts b/packages/roosterjs-editor-core/lib/corePlugins/ImageSelection.ts index 462df71d862..a0f6a871ac8 100644 --- a/packages/roosterjs-editor-core/lib/corePlugins/ImageSelection.ts +++ b/packages/roosterjs-editor-core/lib/corePlugins/ImageSelection.ts @@ -57,6 +57,7 @@ export default class ImageSelection implements EditorPlugin { const target = event.rawEvent.target; if ( safeInstanceOf(target, 'HTMLImageElement') && + target.isContentEditable && event.rawEvent.button === mouseLeftButton ) { this.editor.select(target); diff --git a/packages/roosterjs-editor-plugins/lib/plugins/TableCellSelection/mouseUtils/handleMouseDownEvent.ts b/packages/roosterjs-editor-plugins/lib/plugins/TableCellSelection/mouseUtils/handleMouseDownEvent.ts index 91e9f6f3b7b..b68a93cfd92 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/TableCellSelection/mouseUtils/handleMouseDownEvent.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/TableCellSelection/mouseUtils/handleMouseDownEvent.ts @@ -24,7 +24,12 @@ export function handleMouseDownEvent( state: TableCellSelectionState, editor: IEditor ) { - const { which, shiftKey } = event.rawEvent; + const { which, shiftKey, target } = event.rawEvent; + const table = editor.getElementAtCursor('table', target as Node, event); + + if (table && !table.isContentEditable) { + return; + } const td = editor.getElementAtCursor(TABLE_CELL_SELECTOR); if (which == RIGHT_CLICK && state.tableSelection && state.vTable && td) { From 818fda09419709aa311aa814d5043ed939f12817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Roldi?= Date: Wed, 2 Aug 2023 17:53:21 -0300 Subject: [PATCH 10/17] fix rotate handles --- .../lib/plugins/ImageEdit/ImageEdit.ts | 42 ++++++++++---- .../plugins/ImageEdit/imageEditors/Resizer.ts | 5 +- .../plugins/ImageEdit/imageEditors/Rotator.ts | 55 +++++++++++-------- .../test/imageEdit/rotatorTest.ts | 4 +- 4 files changed, 67 insertions(+), 39 deletions(-) diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts index 70361182795..62715d4ea96 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts @@ -8,7 +8,7 @@ import ImageEditInfo from './types/ImageEditInfo'; import ImageHtmlOptions from './types/ImageHtmlOptions'; import { Cropper, getCropHTML } from './imageEditors/Cropper'; import { deleteEditInfo, getEditInfoFromImage } from './editInfoUtils/editInfo'; -import { getRotateHTML, Rotator, updateRotateHandlePosition } from './imageEditors/Rotator'; +import { getRotateHTML, Rotator, updateRotateHandleState } from './imageEditors/Rotator'; import { ImageEditElementClass } from './types/ImageEditElementClass'; import { tryToConvertGifToPng } from './editInfoUtils/tryToConvertGifToPng'; import { @@ -87,7 +87,12 @@ const DARK_MODE_BGCOLOR = '#333'; /** * The biggest area of image with 4 handles */ -const MAX_SMALL_SIZE_IMAGE = 10000; +const MAX_SMALL_SIZE_IMAGE = 20000; + +/** + * The smallest value of hight or width to make side handle and rotate handle visible + */ +const MIN_HEIGHT_WIDTH = 100; /** * ImageEdit plugin provides the ability to edit an inline image in editor, including image resizing, rotation and cropping @@ -451,7 +456,7 @@ export default class ImageEdit implements EditorPlugin { rotateHandleBackColor: this.editor.isDarkMode() ? DARK_MODE_BGCOLOR : LIGHT_MODE_BGCOLOR, - isSmallImage: isASmallImage(this.editInfo!), + isSmallImage: isASmallImage(this.editInfo.widthPx, this.editInfo.heightPx), }; const htmlData: CreateElementData[] = [getResizeBordersHTML(options)]; @@ -598,10 +603,13 @@ export default class ImageEdit implements EditorPlugin { } const viewport = this.editor?.getVisibleViewport(); + const isSmall = isASmallImage(targetWidth, targetHeight); if (rotateHandle && rotateCenter && viewport) { - updateRotateHandlePosition(viewport, rotateCenter, rotateHandle); + updateRotateHandleState(viewport, rotateCenter, rotateHandle, isSmall); } + updateSideHandlesVisibility(resizeHandles, isSmall); + updateHandleCursor(resizeHandles, angleRad); } } @@ -714,13 +722,22 @@ function rotateHandles(angleRad: number, y: string = '', x: string = ''): string * @param angleRad The angle that the image was rotated. */ function updateHandleCursor(handles: HTMLElement[], angleRad: number) { - handles.map(handle => { - const y = handle.dataset.y; - const x = handle.dataset.x; + handles.forEach(handle => { + const { y, x } = handle.dataset; handle.style.cursor = `${rotateHandles(angleRad, y, x)}-resize`; }); } +function updateSideHandlesVisibility(handles: HTMLElement[], isSmall: boolean) { + handles.forEach(handle => { + const { y, x } = handle.dataset; + const coordinate = (y ?? '') + (x ?? ''); + const directions = ['n', 's', 'e', 'w']; + const isSideHandle = directions.indexOf(coordinate) > -1; + handle.style.display = isSideHandle && isSmall ? 'none' : ''; + }); +} + /** * Check if the current image was resized by the user * @param image the current image @@ -748,9 +765,14 @@ function isFixedNumberValue(value: string | number) { return !isNaN(numberValue); } -function isASmallImage(editInfo: ImageEditInfo): boolean { - const { widthPx, heightPx } = editInfo; - return widthPx && heightPx && widthPx * widthPx < MAX_SMALL_SIZE_IMAGE ? true : false; +function isASmallImage(widthPx: number, heightPx: number): boolean { + return widthPx && + heightPx && + (widthPx * widthPx < MAX_SMALL_SIZE_IMAGE || + widthPx < MIN_HEIGHT_WIDTH || + heightPx < MIN_HEIGHT_WIDTH) + ? true + : false; } function getColorString(color: string | ModeIndependentColor, isDarkMode: boolean): string { diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts index c9fa0dba628..48db577b5f4 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts @@ -161,12 +161,9 @@ export function getCornerResizeHTML( * Get HTML for resize handles on the sides */ export function getSideResizeHTML( - { borderColor: resizeBorderColor, isSmallImage: isSmallImage }: ImageHtmlOptions, + { borderColor: resizeBorderColor }: ImageHtmlOptions, onShowResizeHandle?: OnShowResizeHandle ): CreateElementData[] | null { - if (isSmallImage) { - return null; - } const result: CreateElementData[] = []; Xs.forEach(x => Ys.forEach(y => { diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Rotator.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Rotator.ts index 772588ecc07..f580175d385 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Rotator.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Rotator.ts @@ -44,34 +44,43 @@ export const Rotator: DragAndDropHandler = { * Move rotate handle. When image is very close to the border of editor, rotate handle may not be visible. * Fix it by reduce the distance from image to rotate handle */ -export function updateRotateHandlePosition( +export function updateRotateHandleState( editorRect: Rect, rotateCenter: HTMLElement, - rotateHandle: HTMLElement + rotateHandle: HTMLElement, + isSmallImage: boolean ) { - const rotateHandleRect = rotateHandle.getBoundingClientRect(); + if (isSmallImage) { + rotateCenter.style.display = 'none'; + rotateHandle.style.display = 'none'; + return; + } else { + rotateCenter.style.display = ''; + rotateHandle.style.display = ''; + const rotateHandleRect = rotateHandle.getBoundingClientRect(); - if (rotateHandleRect) { - const top = rotateHandleRect.top - editorRect.top; - const left = rotateHandleRect.left - editorRect.left; - const right = rotateHandleRect.right - editorRect.right; - const bottom = rotateHandleRect.bottom - editorRect.bottom; - let adjustedDistance = Number.MAX_SAFE_INTEGER; - if (top <= 0) { - adjustedDistance = top; - } else if (left <= 0) { - adjustedDistance = left; - } else if (right >= 0) { - adjustedDistance = right; - } else if (bottom >= 0) { - adjustedDistance = bottom; - } + if (rotateHandleRect) { + const top = rotateHandleRect.top - editorRect.top; + const left = rotateHandleRect.left - editorRect.left; + const right = rotateHandleRect.right - editorRect.right; + const bottom = rotateHandleRect.bottom - editorRect.bottom; + let adjustedDistance = Number.MAX_SAFE_INTEGER; + if (top <= 0) { + adjustedDistance = top; + } else if (left <= 0) { + adjustedDistance = left; + } else if (right >= 0) { + adjustedDistance = right; + } else if (bottom >= 0) { + adjustedDistance = bottom; + } - const rotateGap = Math.max(Math.min(ROTATE_GAP, adjustedDistance), 0); - const rotateTop = Math.max(Math.min(ROTATE_SIZE, adjustedDistance - rotateGap), 0); - rotateCenter.style.top = -rotateGap + 'px'; - rotateCenter.style.height = rotateGap + 'px'; - rotateHandle.style.top = -rotateTop + 'px'; + const rotateGap = Math.max(Math.min(ROTATE_GAP, adjustedDistance), 0); + const rotateTop = Math.max(Math.min(ROTATE_SIZE, adjustedDistance - rotateGap), 0); + rotateCenter.style.top = -rotateGap + 'px'; + rotateCenter.style.height = rotateGap + 'px'; + rotateHandle.style.top = -rotateTop + 'px'; + } } } diff --git a/packages/roosterjs-editor-plugins/test/imageEdit/rotatorTest.ts b/packages/roosterjs-editor-plugins/test/imageEdit/rotatorTest.ts index c058b9709d1..ec1d86492d6 100644 --- a/packages/roosterjs-editor-plugins/test/imageEdit/rotatorTest.ts +++ b/packages/roosterjs-editor-plugins/test/imageEdit/rotatorTest.ts @@ -11,7 +11,7 @@ import DragAndDropContext, { import { getRotateHTML, Rotator, - updateRotateHandlePosition, + updateRotateHandleState, } from '../../lib/plugins/ImageEdit/imageEditors/Rotator'; const ROTATE_SIZE = 32; @@ -140,7 +140,7 @@ describe('updateRotateHandlePosition', () => { }; editorGetVisibleViewport.and.returnValue(viewport); - updateRotateHandlePosition(viewport, rotateCenter, rotateHandle); + updateRotateHandleState(viewport, rotateCenter, rotateHandle, false); expect(rotateCenter.style.top).toBe(rotateCenterTop); expect(rotateCenter.style.height).toBe(rotateCenterHeight); From 38ff20686e71861add0dbe2e84abe88ddb40646a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Roldi?= Date: Thu, 3 Aug 2023 13:54:45 -0300 Subject: [PATCH 11/17] proportions --- .../lib/plugins/ImageEdit/ImageEdit.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts index 62715d4ea96..db559281dbe 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts @@ -84,11 +84,6 @@ const ImageEditHTMLMap = { const LIGHT_MODE_BGCOLOR = 'white'; const DARK_MODE_BGCOLOR = '#333'; -/** - * The biggest area of image with 4 handles - */ -const MAX_SMALL_SIZE_IMAGE = 20000; - /** * The smallest value of hight or width to make side handle and rotate handle visible */ @@ -766,11 +761,7 @@ function isFixedNumberValue(value: string | number) { } function isASmallImage(widthPx: number, heightPx: number): boolean { - return widthPx && - heightPx && - (widthPx * widthPx < MAX_SMALL_SIZE_IMAGE || - widthPx < MIN_HEIGHT_WIDTH || - heightPx < MIN_HEIGHT_WIDTH) + return widthPx && heightPx && (widthPx < MIN_HEIGHT_WIDTH || heightPx < MIN_HEIGHT_WIDTH) ? true : false; } From 93433f99c0784a8bfe66cc7e19249f71e525dc8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Roldi?= Date: Thu, 3 Aug 2023 14:30:53 -0300 Subject: [PATCH 12/17] constants --- .../lib/plugins/ImageEdit/ImageEdit.ts | 6 +---- .../plugins/ImageEdit/constants/constants.ts | 23 +++++++++++++++++++ .../plugins/ImageEdit/imageEditors/Cropper.ts | 12 +--------- .../plugins/ImageEdit/imageEditors/Resizer.ts | 5 +--- .../plugins/ImageEdit/imageEditors/Rotator.ts | 13 ++++++----- 5 files changed, 33 insertions(+), 26 deletions(-) create mode 100644 packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/constants/constants.ts diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts index db559281dbe..7714936ed52 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts @@ -10,6 +10,7 @@ import { Cropper, getCropHTML } from './imageEditors/Cropper'; import { deleteEditInfo, getEditInfoFromImage } from './editInfoUtils/editInfo'; import { getRotateHTML, Rotator, updateRotateHandleState } from './imageEditors/Rotator'; import { ImageEditElementClass } from './types/ImageEditElementClass'; +import { MIN_HEIGHT_WIDTH } from './constants/constants'; import { tryToConvertGifToPng } from './editInfoUtils/tryToConvertGifToPng'; import { arrayPush, @@ -84,11 +85,6 @@ const ImageEditHTMLMap = { const LIGHT_MODE_BGCOLOR = 'white'; const DARK_MODE_BGCOLOR = '#333'; -/** - * The smallest value of hight or width to make side handle and rotate handle visible - */ -const MIN_HEIGHT_WIDTH = 100; - /** * ImageEdit plugin provides the ability to edit an inline image in editor, including image resizing, rotation and cropping */ diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/constants/constants.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/constants/constants.ts new file mode 100644 index 00000000000..86a618a2bc7 --- /dev/null +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/constants/constants.ts @@ -0,0 +1,23 @@ +import { DNDDirectionX, DnDDirectionY } from '../types/DragAndDropContext'; + +export const RESIZE_HANDLE_SIZE = 10; +export const RESIZE_HANDLE_MARGIN = 3; + +export const ROTATE_SIZE = 32; +export const ROTATE_GAP = 15; +export const DEG_PER_RAD = 180 / Math.PI; +export const DEFAULT_ROTATE_HANDLE_HEIGHT = ROTATE_SIZE / 2 + ROTATE_GAP; +export const ROTATE_ICON_MARGIN = 8; + +export const CROP_HANDLE_SIZE = 22; +export const CROP_HANDLE_WIDTH = 7; +export const Xs: DNDDirectionX[] = ['w', 'e']; +export const Ys: DnDDirectionY[] = ['s', 'n']; +export const ROTATION: Record = { + sw: 0, + nw: 90, + ne: 180, + se: 270, +}; + +export const MIN_HEIGHT_WIDTH = 3 * RESIZE_HANDLE_SIZE + 2 * RESIZE_HANDLE_MARGIN; diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Cropper.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Cropper.ts index 0a951570827..fe3e4a786f6 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Cropper.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Cropper.ts @@ -1,21 +1,11 @@ import DragAndDropContext, { DNDDirectionX, DnDDirectionY } from '../types/DragAndDropContext'; import DragAndDropHandler from '../../../pluginUtils/DragAndDropHandler'; import { CreateElementData } from 'roosterjs-editor-types'; +import { CROP_HANDLE_SIZE, CROP_HANDLE_WIDTH, ROTATION, Xs, Ys } from '../constants/constants'; import { CropInfo } from '../types/ImageEditInfo'; import { ImageEditElementClass } from '../types/ImageEditElementClass'; import { rotateCoordinate } from './Resizer'; -const CROP_HANDLE_SIZE = 22; -const CROP_HANDLE_WIDTH = 7; -const Xs: DNDDirectionX[] = ['w', 'e']; -const Ys: DnDDirectionY[] = ['s', 'n']; -const ROTATION: Record = { - sw: 0, - nw: 90, - ne: 180, - se: 270, -}; - /** * @internal * Crop handle for DragAndDropHelper diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts index 48db577b5f4..a15f96ffc8c 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts @@ -4,6 +4,7 @@ import ImageEditInfo, { ResizeInfo } from '../types/ImageEditInfo'; import ImageHtmlOptions from '../types/ImageHtmlOptions'; import { CreateElementData } from 'roosterjs-editor-types'; import { ImageEditElementClass } from '../types/ImageEditElementClass'; +import { RESIZE_HANDLE_MARGIN, RESIZE_HANDLE_SIZE, Xs, Ys } from '../constants/constants'; /** * An optional callback to allow customize resize handle element of image resizing. @@ -18,10 +19,6 @@ const enum HandleTypes { SquareHandles, CircularHandlesCorner, } -const RESIZE_HANDLE_SIZE = 10; -const RESIZE_HANDLE_MARGIN = 3; -const Xs: DNDDirectionX[] = ['w', '', 'e']; -const Ys: DnDDirectionY[] = ['s', '', 'n']; /** * @internal diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Rotator.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Rotator.ts index f580175d385..089a8b52f2f 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Rotator.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Rotator.ts @@ -4,12 +4,13 @@ import ImageHtmlOptions from '../types/ImageHtmlOptions'; import { CreateElementData, Rect } from 'roosterjs-editor-types'; import { ImageEditElementClass } from '../types/ImageEditElementClass'; import { RotateInfo } from '../types/ImageEditInfo'; - -const ROTATE_SIZE = 32; -const ROTATE_GAP = 15; -const DEG_PER_RAD = 180 / Math.PI; -const DEFAULT_ROTATE_HANDLE_HEIGHT = ROTATE_SIZE / 2 + ROTATE_GAP; -const ROTATE_ICON_MARGIN = 8; +import { + DEFAULT_ROTATE_HANDLE_HEIGHT, + DEG_PER_RAD, + ROTATE_GAP, + ROTATE_ICON_MARGIN, + ROTATE_SIZE, +} from '../constants/constants'; /** * @internal From 356a6130a227faa9dccbddfd84b181967b7a3f2e Mon Sep 17 00:00:00 2001 From: Andres-CT98 <107568016+Andres-CT98@users.noreply.github.com> Date: Thu, 3 Aug 2023 11:44:27 -0600 Subject: [PATCH 13/17] hide on dragging (#2005) --- .../TableResize/editors/TableResizer.ts | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableResizer.ts b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableResizer.ts index 3214ee7f643..e2eefb64eeb 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableResizer.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableResizer.ts @@ -20,7 +20,7 @@ export default function createTableResizer( table: HTMLTableElement, editor: IEditor, onStart: () => void, - onDragEnd: () => false, + onEnd: () => false, onShowHelperElement?: ( elementData: CreateElementData, helperType: 'CellResizer' | 'TableInserter' | 'TableResizer' | 'TableSelector' @@ -58,6 +58,10 @@ export default function createTableResizer( table, zoomScale, onStart, + onEnd, + div, + editor, + contentDiv, }; setDivPosition(context, div); @@ -65,7 +69,7 @@ export default function createTableResizer( const featureHandler = new DragAndDropHelper( div, context, - setDivPosition, + hideResizer, // Resizer is hidden while dragging only { onDragStart, onDragging, @@ -82,6 +86,10 @@ interface DragAndDropContext { isRTL: boolean; zoomScale: number; onStart: () => void; + onEnd: () => false; + div: HTMLDivElement; + editor: IEditor; + contentDiv?: EventTarget | null; } interface DragAndDropInitValue { @@ -153,6 +161,25 @@ function onDragging( } } +function onDragEnd( + context: DragAndDropContext, + event: MouseEvent, + initValue: DragAndDropInitValue | undefined +) { + if ( + isTableBottomVisible( + context.editor, + normalizeRect(context.table.getBoundingClientRect()), + context.contentDiv + ) + ) { + context.div.style.visibility = 'visible'; + setDivPosition(context, context.div); + } + context.onEnd(); + return false; +} + function setDivPosition(context: DragAndDropContext, trigger: HTMLElement) { const { table, isRTL } = context; const rect = normalizeRect(table.getBoundingClientRect()); @@ -165,6 +192,10 @@ function setDivPosition(context: DragAndDropContext, trigger: HTMLElement) { } } +function hideResizer(context: DragAndDropContext, trigger: HTMLElement) { + trigger.style.visibility = 'hidden'; +} + function isTableBottomVisible( editor: IEditor, rect: Rect | null, From 4d1acbbd13e5dfa2f207eec04659ed5eedee944f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Roldi?= Date: Fri, 4 Aug 2023 10:31:24 -0300 Subject: [PATCH 14/17] fix tests and constants --- .../lib/plugins/ImageEdit/ImageEdit.ts | 2 +- .../plugins/ImageEdit/constants/constants.ts | 13 +++++++------ .../plugins/ImageEdit/imageEditors/Resizer.ts | 2 +- .../plugins/ImageEdit/imageEditors/Rotator.ts | 11 ++++++++--- .../test/imageEdit/rotatorTest.ts | 18 +++++++++--------- 5 files changed, 26 insertions(+), 20 deletions(-) diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts index 7714936ed52..abed1dd77ba 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts @@ -406,7 +406,7 @@ export default class ImageEdit implements EditorPlugin { * quit editing mode when editor lose focus */ private onBlur = () => { - this.setEditingImage(null, false /* selectImage */); + // this.setEditingImage(null, false /* selectImage */); }; /** * Create editing wrapper for the image diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/constants/constants.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/constants/constants.ts index 86a618a2bc7..754ad0f4a08 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/constants/constants.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/constants/constants.ts @@ -1,23 +1,24 @@ import { DNDDirectionX, DnDDirectionY } from '../types/DragAndDropContext'; export const RESIZE_HANDLE_SIZE = 10; -export const RESIZE_HANDLE_MARGIN = 3; +export const RESIZE_HANDLE_MARGIN = 6; export const ROTATE_SIZE = 32; export const ROTATE_GAP = 15; export const DEG_PER_RAD = 180 / Math.PI; export const DEFAULT_ROTATE_HANDLE_HEIGHT = ROTATE_SIZE / 2 + ROTATE_GAP; export const ROTATE_ICON_MARGIN = 8; - -export const CROP_HANDLE_SIZE = 22; -export const CROP_HANDLE_WIDTH = 7; -export const Xs: DNDDirectionX[] = ['w', 'e']; -export const Ys: DnDDirectionY[] = ['s', 'n']; export const ROTATION: Record = { sw: 0, nw: 90, ne: 180, se: 270, }; +export const ROTATE_WIDTH = 1; +export const ROTATE_HANDLE_TOP = ROTATE_GAP + RESIZE_HANDLE_MARGIN; +export const CROP_HANDLE_SIZE = 22; +export const CROP_HANDLE_WIDTH = 7; +export const Xs: DNDDirectionX[] = ['w', '', 'e']; +export const Ys: DnDDirectionY[] = ['s', '', 'n']; export const MIN_HEIGHT_WIDTH = 3 * RESIZE_HANDLE_SIZE + 2 * RESIZE_HANDLE_MARGIN; diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts index a15f96ffc8c..c9df9f99a2e 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts @@ -160,7 +160,7 @@ export function getCornerResizeHTML( export function getSideResizeHTML( { borderColor: resizeBorderColor }: ImageHtmlOptions, onShowResizeHandle?: OnShowResizeHandle -): CreateElementData[] | null { +): CreateElementData[] { const result: CreateElementData[] = []; Xs.forEach(x => Ys.forEach(y => { diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Rotator.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Rotator.ts index 089a8b52f2f..9616c242bd1 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Rotator.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Rotator.ts @@ -7,9 +7,12 @@ import { RotateInfo } from '../types/ImageEditInfo'; import { DEFAULT_ROTATE_HANDLE_HEIGHT, DEG_PER_RAD, + RESIZE_HANDLE_MARGIN, ROTATE_GAP, + ROTATE_HANDLE_TOP, ROTATE_ICON_MARGIN, ROTATE_SIZE, + ROTATE_WIDTH, } from '../constants/constants'; /** @@ -78,7 +81,7 @@ export function updateRotateHandleState( const rotateGap = Math.max(Math.min(ROTATE_GAP, adjustedDistance), 0); const rotateTop = Math.max(Math.min(ROTATE_SIZE, adjustedDistance - rotateGap), 0); - rotateCenter.style.top = -rotateGap + 'px'; + rotateCenter.style.top = -rotateGap - RESIZE_HANDLE_MARGIN + 'px'; rotateCenter.style.height = rotateGap + 'px'; rotateHandle.style.top = -rotateTop + 'px'; } @@ -98,12 +101,14 @@ export function getRotateHTML({ { tag: 'div', className: ImageEditElementClass.RotateCenter, - style: `position:absolute;left:50%;width:1px;background-color:${borderColor};top:${-ROTATE_GAP}px;height:${ROTATE_GAP}px;`, + style: `position:absolute;left:50%;width:1px;background-color:${borderColor};top:${-ROTATE_HANDLE_TOP}px;height:${ROTATE_GAP}px;margin-left:${-ROTATE_WIDTH}px;`, children: [ { tag: 'div', className: ImageEditElementClass.RotateHandle, - style: `position:absolute;background-color:${rotateHandleBackColor};border:solid 1px ${borderColor};border-radius:50%;width:${ROTATE_SIZE}px;height:${ROTATE_SIZE}px;left:-${handleLeft}px;cursor:move;top:${-ROTATE_SIZE}px;`, + style: `position:absolute;background-color:${rotateHandleBackColor};border:solid 1px ${borderColor};border-radius:50%;width:${ROTATE_SIZE}px;height:${ROTATE_SIZE}px;left:-${ + handleLeft + ROTATE_WIDTH + }px;cursor:move;top:${-ROTATE_SIZE}px;`, children: [getRotateIconHTML(borderColor)], }, ], diff --git a/packages/roosterjs-editor-plugins/test/imageEdit/rotatorTest.ts b/packages/roosterjs-editor-plugins/test/imageEdit/rotatorTest.ts index ec1d86492d6..58f790c1004 100644 --- a/packages/roosterjs-editor-plugins/test/imageEdit/rotatorTest.ts +++ b/packages/roosterjs-editor-plugins/test/imageEdit/rotatorTest.ts @@ -150,7 +150,7 @@ describe('updateRotateHandlePosition', () => { it('adjust rotate handle - ROTATOR HIDDEN ON TOP', () => { runTest( { - top: 1, + top: 0, bottom: 3, left: 3, right: 5, @@ -160,7 +160,7 @@ describe('updateRotateHandlePosition', () => { y: 3, toJSON: () => {}, }, - '0px', + '-6px', '0px', '0px' ); @@ -179,7 +179,7 @@ describe('updateRotateHandlePosition', () => { y: 3, toJSON: () => {}, }, - '-15px', + '-21px', '15px', '-32px' ); @@ -190,7 +190,7 @@ describe('updateRotateHandlePosition', () => { { top: 2, bottom: 3, - left: 1, + left: 0, right: 5, height: 2, width: 2, @@ -198,7 +198,7 @@ describe('updateRotateHandlePosition', () => { y: 3, toJSON: () => {}, }, - '0px', + '-6px', '0px', '0px' ); @@ -208,7 +208,7 @@ describe('updateRotateHandlePosition', () => { runTest( { top: 2, - bottom: 201, + bottom: 200, left: 1, right: 5, height: 2, @@ -217,7 +217,7 @@ describe('updateRotateHandlePosition', () => { y: 3, toJSON: () => {}, }, - '0px', + '-6px', '0px', '0px' ); @@ -229,14 +229,14 @@ describe('updateRotateHandlePosition', () => { top: 2, bottom: 3, left: 1, - right: 201, + right: 200, height: 2, width: 2, x: 1, y: 3, toJSON: () => {}, }, - '0px', + '-6px', '0px', '0px' ); From 37e834082980a3c6152f9c6b9031aafabd8118f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Roldi?= Date: Fri, 4 Aug 2023 15:15:56 -0300 Subject: [PATCH 15/17] remove comment --- .../roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts index abed1dd77ba..7714936ed52 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts @@ -406,7 +406,7 @@ export default class ImageEdit implements EditorPlugin { * quit editing mode when editor lose focus */ private onBlur = () => { - // this.setEditingImage(null, false /* selectImage */); + this.setEditingImage(null, false /* selectImage */); }; /** * Create editing wrapper for the image From 8604dbeb6349a7770a6c477de4eec3ac195e430f Mon Sep 17 00:00:00 2001 From: Andres-CT98 <107568016+Andres-CT98@users.noreply.github.com> Date: Fri, 4 Aug 2023 12:27:58 -0600 Subject: [PATCH 16/17] Fix Cell resizers and Table Inserters (#2007) * add anchor container * fix cel resize bug, pass anchors, optimise --- .../TableResize/editors/CellResizer.ts | 5 +++-- .../TableResize/editors/TableEditor.ts | 22 +++++++++++++------ .../TableResize/editors/TableInserter.ts | 5 +++-- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/CellResizer.ts b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/CellResizer.ts index 9de03c29b8e..3e2d63edd48 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/CellResizer.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/CellResizer.ts @@ -20,7 +20,8 @@ export default function createCellResizer( onShowHelperElement?: ( elementData: CreateElementData, helperType: 'CellResizer' | 'TableInserter' | 'TableResizer' | 'TableSelector' - ) => void + ) => void, + anchorContainer?: HTMLElement ): TableEditFeature | null { const document = td.ownerDocument; const createElementData = { @@ -32,7 +33,7 @@ export default function createCellResizer( const div = createElement(createElementData, document) as HTMLDivElement; - document.body.appendChild(div); + (anchorContainer || document.body).appendChild(div); const context: DragAndDropContext = { td, isRTL, zoomScale, onStart }; const setPosition = isHorizontal ? setHorizontalPosition : setVerticalPosition; diff --git a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableEditor.ts b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableEditor.ts index d5146c2981e..bfebe230958 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableEditor.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableEditor.ts @@ -127,7 +127,7 @@ export default class TableEditor { return; } - //Determine if cursor is on top or side + // Determine if cursor is on top or side const topOrSide = y <= firstCellRect.top + INSERTER_HOVER_OFFSET ? TOP_OR_SIDE.top @@ -138,14 +138,16 @@ export default class TableEditor { : x <= firstCellRect.left + INSERTER_HOVER_OFFSET ? TOP_OR_SIDE.side : undefined; + const topOrSideBinary = topOrSide ? 1 : 0; + // Get whole table rect + const tableRect = normalizeRect(this.table.getBoundingClientRect()); // i is row index, j is column index for (let i = 0; i < this.table.rows.length; i++) { const tr = this.table.rows[i]; let j = 0; for (; j < tr.cells.length; j++) { const td = tr.cells[j]; - const tableRect = normalizeRect(this.table.getBoundingClientRect()); const tdRect = normalizeRect(td.getBoundingClientRect()); if (!tdRect || !tableRect) { @@ -153,13 +155,14 @@ export default class TableEditor { } // Determine the cell the cursor is in range of + // Offset is only used for first row and column const lessThanBottom = y <= tdRect.bottom; const lessThanRight = this.isRTL - ? x <= tdRect.right + INSERTER_HOVER_OFFSET + ? x <= tdRect.right + INSERTER_HOVER_OFFSET * topOrSideBinary : x <= tdRect.right; const moreThanLeft = this.isRTL ? x >= tdRect.left - : x >= tdRect.left - INSERTER_HOVER_OFFSET; + : x >= tdRect.left - INSERTER_HOVER_OFFSET * topOrSideBinary; if (lessThanBottom && lessThanRight && moreThanLeft) { const isOnLeftOrRight = this.isRTL @@ -196,6 +199,7 @@ export default class TableEditor { this.setResizingTd(td); + //Cell found break; } } @@ -205,6 +209,7 @@ export default class TableEditor { } } + // Create Selector and Resizer this.setEditorFeatures(); } @@ -248,7 +253,8 @@ export default class TableEditor { true /*isHorizontal*/, this.onStartCellResize, this.onFinishEditing, - this.onShowHelperElement + this.onShowHelperElement, + this.anchorContainer ); this.verticalResizer = createCellResizer( td, @@ -257,7 +263,8 @@ export default class TableEditor { false /*isHorizontal*/, this.onStartCellResize, this.onFinishEditing, - this.onShowHelperElement + this.onShowHelperElement, + this.anchorContainer ); } } @@ -280,7 +287,8 @@ export default class TableEditor { !!isHorizontal, this.onInserted, this.getOnMouseOut, - this.onShowHelperElement + this.onShowHelperElement, + this.anchorContainer ); if (isHorizontal) { this.horizontalInserter = newInserter; diff --git a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableInserter.ts b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableInserter.ts index 94519b87a76..2fdf6af3639 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableInserter.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/TableResize/editors/TableInserter.ts @@ -21,7 +21,8 @@ export default function createTableInserter( onShowHelperElement?: ( elementData: CreateElementData, helperType: 'CellResizer' | 'TableInserter' | 'TableResizer' | 'TableSelector' - ) => void + ) => void, + anchorContainer?: HTMLElement ): TableEditFeature | null { const table = editor.getElementAtCursor('table', td); @@ -61,7 +62,7 @@ export default function createTableInserter( (div.firstChild as HTMLElement).style.height = `${tableRect.bottom - tableRect.top}px`; } - document.body.appendChild(div); + (anchorContainer || document.body).appendChild(div); const handler = new TableInsertHandler( div, From f31d5805debf2d00c60606eb0155c21a761d89bf Mon Sep 17 00:00:00 2001 From: Vi Nguyen <74168693+vinguyen12@users.noreply.github.com> Date: Fri, 4 Aug 2023 12:30:42 -0700 Subject: [PATCH 17/17] bump version to 8.53 and 0.13 --- versions.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/versions.json b/versions.json index 1e405343095..5e2f1465dcb 100644 --- a/versions.json +++ b/versions.json @@ -1,5 +1,5 @@ { - "packages": "8.52.0", + "packages": "8.53.0", "packages-ui": "8.50.1", - "packages-content-model": "0.12.0" + "packages-content-model": "0.13.0" }