Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose the insertImageEditingWrapper api #2787

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -335,11 +335,13 @@ export class ImageEditPlugin implements ImageEditor, EditorPlugin {
isNodeOfType(node, 'ELEMENT_NODE') &&
isElementOfType(node, 'img')
) {
if (isCropMode) {
this.startCropMode(editor, node);
} else {
this.startRotateAndResize(editor, node);
}
node.onload = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

after node is loaded, need to remove this handler.

And what if load fails? we also need to remove the handler.

if (isCropMode) {
this.startCropMode(editor, node);
} else {
this.startRotateAndResize(editor, node);
}
};
}
},
},
Expand All @@ -349,6 +351,26 @@ export class ImageEditPlugin implements ImageEditor, EditorPlugin {
);
}

/**
* Insert the image editing wrapper in the selected image
*/
public insertImageEditingWrapper() {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the different between this function and startRotateAndResize?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The startRotateAndResize only handles the DOM manipulation, this one also handles the changes in content model.
This one gets the selected the image set isEditing in the image and when the image node is created call startRotateAndResize

Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the code to set isEditing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Under applyFormatWithContentModel function

if (this.editor) {
const selection = this.editor.getDOMSelection();
if (!this.editor.getEnvironment().isSafari) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we should also only do focus if selection is image, right? so move this under line 363

this.editor.focus(); // Safari will keep the selection when click crop, then the focus() call should not be called
}
if (this.editor && selection?.type == 'image') {
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to check this.editor again here

this.applyFormatWithContentModel(
this.editor,
false /* isCropMode */,
false /** should selectImage */,
false /* isApiOperation */
);
}
}
}

private startEditing(
editor: IEditor,
image: HTMLImageElement,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,4 +430,20 @@ describe('ImageEditPlugin', () => {
expect(formatContentModelSpy).toHaveBeenCalledTimes(3);
plugin.dispose();
});

it('insertImageEditingWrapper', () => {
const mockedImage = {
id: 'image_0',
getAttribute: getAttributeSpy,
} as any;
const plugin = new ImageEditPlugin();
plugin.initialize(editor);
getDOMSelectionSpy.and.returnValue({
type: 'image',
image: mockedImage,
});
plugin.insertImageEditingWrapper();
expect(formatContentModelSpy).toHaveBeenCalled();
plugin.dispose();
});
});
Loading