Skip to content

Commit

Permalink
[BUGFIX] Ensure Editor#hasActiveMarkup detects complex markups
Browse files Browse the repository at this point in the history
When checking for a markup with attributes (e.g., an A markup with
'href' attribute), `hasActiveMarkup` used to coerce the 'a' string
argument to a markup and then fail to consider an existing (different)
"A" markup (with different attributes) as a match for 'a'.
  • Loading branch information
bantic committed Apr 12, 2016
1 parent 1b255c1 commit 36be12a
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 12 deletions.
11 changes: 9 additions & 2 deletions src/js/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -487,8 +487,15 @@ class Editor {
* @return {boolean}
*/
hasActiveMarkup(markup) {
markup = this.builder._coerceMarkup(markup);
return contains(this.activeMarkups, markup);
let matchesFn;
if (typeof markup === 'string') {
markup = markup.toLowerCase();
matchesFn = (_markup) => _markup.tagName === markup;
} else {
matchesFn = (_markup) => _markup === markup;
}

return !!detect(this.activeMarkups, matchesFn);
}

get markupsInSelection() {
Expand Down
10 changes: 0 additions & 10 deletions src/js/models/post-node-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,16 +162,6 @@ class PostNodeBuilder {

return markup;
}

/**
* @param {Markup|String} markupOrString
* @return {Markup}
* @private
*/
_coerceMarkup(markupOrString, attributes={}) {
let tagName = typeof markupOrString === 'string' ? markupOrString : markupOrString.tagName;
return this.createMarkup(tagName, attributes);
}
}

export default PostNodeBuilder;
25 changes: 25 additions & 0 deletions tests/unit/editor/editor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -338,3 +338,28 @@ test('#activeMarkups returns the markups at cursor when range is collapsed', (as
editor.selectRange(Range.create(head, 'abcdefg'.length));
assert.equal(editor.activeMarkups.length, 0, 'no active markups after end of bold text');
});

test('#hasActiveMarkup returns true for complex markups', (assert) => {
editor = Helpers.mobiledoc.renderInto(editorElement, ({post, markupSection, marker, markup}) => {
return post([markupSection('p', [
marker('abc'),
marker('def', [markup('a', {href: 'http://bustle.com'})]),
marker('ghi')
])]);
});

let head = editor.post.sections.head;
editor.selectRange(Range.create(head, 'abc'.length));
assert.equal(editor.activeMarkups.length, 0, 'no active markups at left of bold text');

editor.selectRange(Range.create(head, 'abcd'.length));
assert.equal(editor.activeMarkups.length, 1, 'active markups in linked text');
assert.ok(editor.hasActiveMarkup('a'), 'has A active markup');

editor.selectRange(Range.create(head, 'abcdef'.length));
assert.equal(editor.activeMarkups.length, 1, 'active markups at end of linked text');
assert.ok(editor.hasActiveMarkup('a'), 'has A active markup');

editor.selectRange(Range.create(head, 'abcdefg'.length));
assert.equal(editor.activeMarkups.length, 0, 'no active markups after end of linked text');
});

0 comments on commit 36be12a

Please sign in to comment.