-
-
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 Consolidation #28
Conversation
In the future, we could implement the features we use (list continue, indent) in core.
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.
Looks good, just that one idea. Can we wait for review comments on flarum/framework#2826 before merging though, as a fix for Win + Enter submitting could be used here to reduce complexity.
view(vnode) { | ||
return <div id="MarkdownToolbar" data-for={this.attrs.for} style={{ display: 'inline-block' }}> | ||
return <div class="MarkdownToolbar"> |
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 change from id
to class
necessary? Are we intentionally breaking styles that use the ID here?
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 do agree with this change, just in case theres two of these toolbars on screen at the same time, despite how ridiculously unlikely that would be
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 also agree with this change, while in theory it shouldn't be needed it also makes a lot of sense.
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.
Iirc, there isn't ID based stuff anywhere else in the composer hierarchy. I don't think it should be here either.
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.
Reconfirming the approve because GitHub is GitHub
- Move insertText to core - Move styles and apply to core - Simplify shortcut and button system - Drop mdarea for now. In the future, we could implement the features we use (list continue, indent) in core. - Remove admin dist (only admin setting was mdarea) - Move inline style to css
- Move insertText to core - Move styles and apply to core - Simplify shortcut and button system - Drop mdarea for now. In the future, we could implement the features we use (list continue, indent) in core. - Remove admin dist (only admin setting was mdarea) - Move inline style to css
Companion to flarum/framework#2826