-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
Editor Drivers Abstraction #24
Conversation
|
||
field.addEventListener('keydown', this.shortcut.bind(this)); | ||
field.addEventListener('keydown', this.shortcut.bind(this)); | ||
}, 25); |
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 is all a bit arbitrary, but I'm not sure of a better place we could put this now that the actual textarea in core is added dynamically by JS
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 could also try to insert this as an input listener but that gets messy since that needs the component instance, which we won't have. So we'd need to do some tricky stuff where we pass a callback to input listeners, and another to the one here which hacks the local scope of the first one to insert this initialization.
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.
Actually, input listeners won't work, since they rely on events. And for that reason, I think it might be better to extend BasicEditorDriver to add the listener there. For the same reason, this does not require / is not enabled by any potential refactors to Core's new editor driver system, so it is not blocking.
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.
Fixed! This no longer relies on an arbitrary timeout, which frankly wouldn't be rigorous design.
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.
Great! That's definitely better than the timeout.
This is included because it fixes a bug where the last character isn't included, introduced by the shift in core's editor implementation + the way mdarea used to handle state
c2bd1c0
to
9e1c677
Compare
This will allow drop-in replacements of the editor with a more advanced WYSIWYG solution such as ProseMirror
See flarum/framework#2566