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

Refactor some method into public postEditor methods #73

Merged
merged 6 commits into from
Aug 17, 2015

Conversation

mixonic
Copy link
Contributor

@mixonic mixonic commented Aug 12, 2015

This begins the exposure of a public API programmatically interacting with the editor. For example:

let editor = new ContentKit.Editor(someElement);

$('button').append(document.body).on('click', () => {

  let markerRange = editor.cursor.offsets;
  let markup = editor.builder.createMarkup('strong');
  editor.run((postEditor) => {
    postEditor.removeMarkupFromMarkers(markerRange, markup);
  });

});

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 the postEditor ensures that no matter what operations are performed only a single render and update are performed.

TODO:

  • On the postEditor, this.rerender() should be this.scheduleRerender(). Likewise for didUpdate
  • Docs
  • The internals of handleNewline must be migrated to postEditor
  • Operations during reparse must move to the postEditor actually, reparse should be going away now that we control delete/enter. Should discuss with @bantic.
  • loadModel should become part of the editor dropped loadModel, we don't really have an immediate need.
  • rerender itself should be moved to the postEditor, and the renderer should be passed to the postEditor instance. seems coupled to loadModel concerns. Will revisit.

@mixonic
Copy link
Contributor Author

mixonic commented Aug 17, 2015

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) => {
Copy link
Collaborator

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;
}
Copy link
Collaborator

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.

Copy link
Contributor Author

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.

@bantic
Copy link
Collaborator

bantic commented Aug 17, 2015

👍 this is great

mixonic added a commit that referenced this pull request Aug 17, 2015
Refactor some method into public postEditor methods
@mixonic mixonic merged commit b7b9694 into bustle:master Aug 17, 2015
@mixonic mixonic mentioned this pull request Aug 17, 2015
10 tasks
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