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

add willHandleNewline hook #489

Merged
merged 1 commit into from
Sep 9, 2016
Merged

Conversation

eguitarz
Copy link
Contributor

@eguitarz eguitarz commented Sep 7, 2016

  • It addresses feature: Able to match newline in onTextInput handler #481
  • I was planning to handle newline base on callback return value. But current callback structure omits the return values. Instead, I passed a parameter into willHandleNewline and handle newline base on its value is changed or not.

@@ -339,6 +340,12 @@ class Editor {
return;
}
}

// Above logic might delete redundant range, so callback must run after it.
let options = { insertNewline: true };
Copy link
Collaborator

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();
  }
});

@eguitarz eguitarz force-pushed the will-handle-newline branch 7 times, most recently from bf0dcc7 to 74f3125 Compare September 8, 2016 06:45
@eguitarz
Copy link
Contributor Author

eguitarz commented Sep 8, 2016

@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.

@bantic
Copy link
Collaborator

bantic commented Sep 8, 2016

@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 preventDefault method on it:

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 CustomEvent because of the inconsistencies between browsers.
If you can make that one change this will be good to go and I'd be happy to merge. Sorry about the back-and-forth.

@eguitarz
Copy link
Contributor Author

eguitarz commented Sep 9, 2016

@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.

@bantic
Copy link
Collaborator

bantic commented Sep 9, 2016

@eguitarz 👍 thank you for the hard work!

@bantic bantic merged commit f1d2262 into bustle:master Sep 9, 2016
@bantic
Copy link
Collaborator

bantic commented Sep 13, 2016

released in v0.10.10.

johnelliott pushed a commit to dailybeast/mobiledoc-kit that referenced this pull request Feb 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants