-
Notifications
You must be signed in to change notification settings - Fork 152
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
add willHandleNewline hook #489
Conversation
8fe43db
to
fb03495
Compare
@@ -339,6 +340,12 @@ class Editor { | |||
return; | |||
} | |||
} | |||
|
|||
// Above logic might delete redundant range, so callback must run after it. | |||
let options = { insertNewline: 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.
@eguitarz Instead of using this ad hoc options hash, let's use a synthetic event
object that has a preventDefault
method on it. In user code, the developer can call the preventDefault
method on it, e.g.:
editor.willHandleNewline((event) => {
if (shouldPreventDefault) {
event.preventDefault();
}
});
bf0dcc7
to
74f3125
Compare
@bantic Please check if this version is what you preferred. Unfortunately we have to do some workarounds to support custom events in IE, the code is a bit longer than I thought. |
74f3125
to
741bacf
Compare
@eguitarz Thanks for doing all this work! I think I inadvertently led you astray, I apologize. I just meant passing a basic event-like object with a let defaultPrevented = false;
const event = { preventDefault() { defaultPrevented = true; };
this.runCallbacks(CALLBACK_QUEUES.WILL_HANDLE_NEWLINE, [event]);
if (defaultPrevented) { ... I'd prefer that to using a true |
741bacf
to
cf1024d
Compare
@bantic No problem. I enjoyed in this back-and-forth convo, so don't worry about it. Now, it should be the version you preferred. |
@eguitarz 👍 thank you for the hard work! |
released in v0.10.10. |
willHandleNewline
and handle newline base on its value is changed or not.