-
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
Refactor some method into public postEditor methods #73
Conversation
Rebase and merge with @bantic's latest, also handling of newlines is ported. Moving through the other items. |
@@ -149,3 +149,21 @@ test('#coalesceMarkers appends a single blank marker if all the markers were bla | |||
assert.equal(s.markers.length, 1, 'has 1 marker after coalescing'); | |||
assert.ok(s.markers.head.isEmpty, 'remaining marker is empty'); | |||
}); | |||
|
|||
test('.isBlank returns true if the text length is zero for two markers', (assert) => { |
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.
Elsewhere in the tests we use #
for instance method abbreviation and .
(dot) for class method abbreviations. To stay in sync with those this should say "#isBlank returns ..."
let markerRange = this.editor.cursor.offsets; | ||
if (!markerRange.leftRenderNode || !markerRange.rightRenderNode) { | ||
return; | ||
} |
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 feels like inside baseball. The italic command should never be exec
-able if there isn't a selection — if the right/left render nodes are not present then something else is wrong, or the command is being exec
'd when there's no selection to apply markup to.
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.
Ah. I was able to create a failure here in chrome with some wacky selection drags. We should look at it together, I'll make a ticket.
👍 this is great |
Refactor some method into public postEditor methods
This begins the exposure of a public API programmatically interacting with the editor. For example:
While the editor and the cursor may create post abstract objects or read data about the current state of the editor, any modifications must be make through the
postEditor
interface. The runloop of thepostEditor
ensures that no matter what operations are performed only a single render and update are performed.TODO:
postEditor
,this.rerender()
should bethis.scheduleRerender()
. Likewise fordidUpdate
handleNewline
must be migrated topostEditor
Operations duringactually, reparse should be going away now that we control delete/enter. Should discuss with @bantic.reparse
must move to thepostEditor
dropped loadModel, we don't really have an immediate need.loadModel
should become part of the editorseems coupled to loadModel concerns. Will revisit.rerender
itself should be moved to thepostEditor
, and the renderer should be passed to thepostEditor
instance.