-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Update last focus manager #3004
Conversation
@easylogic Could you remove |
src/js/base/module/AutoReplace.js
Outdated
@@ -0,0 +1,75 @@ | |||
import $ from 'jquery'; |
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.
How about to add a credit of @dubiousandrew who created AutoReplace
module at first?
@@ -49,7 +49,7 @@ describe('LinkDialog', () => { | |||
dialog.show(); | |||
|
|||
var checked = dialog.$dialog.find('#sn-checkbox-open-in-new-window').is(':checked'); | |||
expect(checked).to.be.false; | |||
expect(checked).to.be.true; |
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.
Is this an intended change?
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.
It does not seem to be related to the commit. strange.
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.
true
is default value to open newWindow in LinkDialog.
checked: true |
checked: true
so test was always fail.
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.
I modified above code to use createRange in LinkDialog, but I always get an error
src/js/base/module/Editor.js
Outdated
@@ -442,7 +447,20 @@ export default class Editor { | |||
*/ | |||
createRange() { | |||
this.focus(); | |||
return range.create(this.editable); | |||
// return range.create(this.editable); |
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.
I saw that there are many range.create
calling at internal and external modules. Is there a chance to use range.create
from outside of Editor.js
module?
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.
It would be better to remove comment.
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.
ok, i will remove comment.
Can you tell us when to use getLastRange and createRange respectively? |
|
src/js/bs4/settings.js
Outdated
@@ -300,5 +302,3 @@ $.summernote = $.extend($.summernote, { | |||
} | |||
} | |||
}); | |||
|
|||
import '../summernote'; // eslint-disable-line |
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.
This need to recover.
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.
i added this code.
What does this PR do?
Where should the reviewer start?
How should this be manually tested?
$(".summernote").summernote("editor.insertText", "aaa")
Any background context you want to provide?
What are the relevant tickets?
#33
#2141
#1520
Screenshot (if for frontend)
Checklist