Skip to content

[Review] Refactor event listeners #148

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

Merged
merged 3 commits into from
Mar 31, 2016
Merged

[Review] Refactor event listeners #148

merged 3 commits into from
Mar 31, 2016

Conversation

juliankrispel
Copy link
Member

A first pr to change how event listeners are attached to Editor

fixes #144

cc @nikgraf @ianstormtaylor

@nikgraf
Copy link
Member

nikgraf commented Mar 29, 2016

@juliankrispel can you check why travis fails?

@juliankrispel juliankrispel force-pushed the refactor-event-listeners branch from 990bb32 to 1983df4 Compare March 29, 2016 09:47
@juliankrispel
Copy link
Member Author

bloody style :)

I squashed the commits into one...

createEventListeners = () => {
const listeners = {
onChange: this.onChange,
editorState: this.editorState,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't make sense for editorState to be in the listeners section since it's just data.

Copy link
Member Author

Choose a reason for hiding this comment

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

fair point, the variable name should be different

@nikgraf nikgraf merged commit 0be663a into master Mar 31, 2016
@nikgraf nikgraf deleted the refactor-event-listeners branch March 31, 2016 19:34
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.

support other handlers
3 participants