-
Notifications
You must be signed in to change notification settings - Fork 156
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
base: master
Are you sure you want to change the base?
Conversation
Please wait |
} else { | ||
this.startRotateAndResize(editor, node); | ||
} | ||
node.onload = () => { |
There was a problem hiding this comment.
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.
/** | ||
* Insert the image editing wrapper in the selected image | ||
*/ | ||
public insertImageEditingWrapper() { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.getEnvironment().isSafari) { | ||
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') { |
There was a problem hiding this comment.
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
public insertImageEditingWrapper() { | ||
if (this.editor) { | ||
const selection = this.editor.getDOMSelection(); | ||
if (!this.editor.getEnvironment().isSafari) { |
There was a problem hiding this comment.
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
/** | ||
* Insert the image editing wrapper in the selected image | ||
*/ | ||
public insertImageEditingWrapper() { |
There was a problem hiding this comment.
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?
Expose the
insertImageEditingWrapper
api to insert the resize and rotate wrapper into the selected image.