diff --git a/README.md b/README.md index e8dcb10ca..38b86ae64 100644 --- a/README.md +++ b/README.md @@ -110,10 +110,10 @@ To change the post in code, use the `editor.run` API. For example, the following usage would mark currently selected text as bold: ```js -let strongMarkup = editor.builder.createMarkup('strong'); -let markerRange = editor.cursor.offsets; +const strongMarkup = editor.builder.createMarkup('strong'); +const range = editor.cursor.offsets; editor.run((postEditor) => { - postEditor.applyMarkupToMarkers(markerRange, strongMarkup); + postEditor.applyMarkupToRange(range, strongMarkup); }); ``` diff --git a/src/js/commands/link.js b/src/js/commands/link.js index f03ada987..e0be85d5a 100644 --- a/src/js/commands/link.js +++ b/src/js/commands/link.js @@ -11,36 +11,23 @@ export default class LinkCommand extends TextFormatCommand { } isActive() { - return any(this.editor.activeMarkers, m => m.hasMarkup(this.tag)); + return any(this.editor.markupsInSelection, m => m.hasTag(this.tag)); } exec(url) { const range = this.editor.cursor.offsets; - - let markers = this.editor.run(postEditor => { + this.editor.run(postEditor => { const markup = postEditor.builder.createMarkup('a', ['href', url]); - return postEditor.applyMarkupToMarkers(range, markup); + postEditor.applyMarkupToRange(range, markup); }); - - if (markers.length) { - let lastMarker = markers[markers.length - 1]; - this.editor.cursor.moveToMarker(lastMarker, lastMarker.length); - } /* else { - // FIXME should handle the case when linking creating no new markers - // this.editor.cursor.moveToSection(range.head.section); - } */ + this.editor.moveToPosition(range.tail); } unexec() { const range = this.editor.cursor.offsets; - - const markers = this.editor.run(postEditor => { - return postEditor.removeMarkupFromMarkers( - range, - markup => markup.hasTag('a') - ); + this.editor.run(postEditor => { + postEditor.removeMarkupFromRange(range, markup => markup.hasTag('a')); }); - - this.editor.selectMarkers(markers); + this.editor.selectRange(range); } } diff --git a/src/js/commands/text-format.js b/src/js/commands/text-format.js index 248adf870..cc330bc59 100644 --- a/src/js/commands/text-format.js +++ b/src/js/commands/text-format.js @@ -10,28 +10,25 @@ export default class TextFormatCommand extends Command { get markup() { if (this._markup) { return this._markup; } - const { builder } = this.editor; - this._markup = builder.createMarkup(this.tag); + this._markup = this.editor.builder.createMarkup(this.tag); return this._markup; } isActive() { - return any(this.editor.activeMarkers, m => m.hasMarkup(this.markup)); + return any(this.editor.markupsInSelection, m => m === this.markup); } exec() { - const range = this.editor.cursor.offsets; - const markers = this.editor.run((postEditor) => { - return postEditor.applyMarkupToMarkers(range, this.markup); - }); - this.editor.selectMarkers(markers); + const range = this.editor.cursor.offsets, { markup } = this; + this.editor.run( + postEditor => postEditor.applyMarkupToRange(range, markup)); + this.editor.selectRange(range); } unexec() { - const range = this.editor.cursor.offsets; - const markers = this.editor.run((postEditor) => { - return postEditor.removeMarkupFromMarkers(range, this.markup); - }); - this.editor.selectMarkers(markers); + const range = this.editor.cursor.offsets, { markup } = this; + this.editor.run( + postEditor => postEditor.removeMarkupFromRange(range, markup)); + this.editor.selectRange(range); } } diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js index 0fc7253ea..f72a45c59 100644 --- a/src/js/editor/editor.js +++ b/src/js/editor/editor.js @@ -295,9 +295,18 @@ class Editor { this.reportSelection(); } - selectMarkers(markers) { - this.cursor.selectMarkers(markers); - this.reportSelection(); + selectRange(range){ + this.cursor.selectRange(range); + if (range.isCollapsed) { + this.reportNoSelection(); + } else { + this.reportSelection(); + } + } + + moveToPosition(position) { + this.cursor.moveToPosition(position); + this.reportNoSelection(); } get cursor() { @@ -371,21 +380,13 @@ class Editor { return this.cursor.activeSections; } - get activeMarkers() { - const { - headMarker, - tailMarker - } = this.cursor.offsets; - - let activeMarkers = []; - - if (headMarker && tailMarker) { - this.post.markersFrom(headMarker, tailMarker, m => { - activeMarkers.push(m); - }); + get markupsInSelection() { + if (this.cursor.hasSelection()) { + const range = this.cursor.offsets; + return this.post.markupsInRange(range); + } else { + return []; } - - return activeMarkers; } /* diff --git a/src/js/editor/post.js b/src/js/editor/post.js index ea86f07a0..94baeb9fc 100644 --- a/src/js/editor/post.js +++ b/src/js/editor/post.js @@ -145,18 +145,6 @@ class PostEditor { } } - removeMarkerRange(headMarker, tailMarker) { - let marker = headMarker; - while (marker) { - let nextMarker = marker.next; - this.removeMarker(marker); - if (marker === tailMarker) { - break; - } - marker = nextMarker; - } - } - _coalesceMarkers(section) { filter(section.markers, m => m.isEmpty).forEach(marker => { this.removeMarker(marker); @@ -357,8 +345,8 @@ class PostEditor { /** * Split markers at two positions, once at the head, and if necessary once - * at the tail. This method is designed to accept `editor.cursor.offsets` - * as an argument. + * at the tail. This method is designed to accept a range + * (e.g. `editor.cursor.offsets`) as an argument. * * Usage: * @@ -371,18 +359,23 @@ class PostEditor { * provided. Markers on the outside of the split may also have been modified. * * @method splitMarkers - * @param {Object} markerRange Object with offsets, {headMarker, headMarkerOffset, tailMarker, tailMarkerOffset} + * @param {Range} markerRange * @return {Array} of markers that are inside the split * @public */ - splitMarkers({headMarker, headMarkerOffset, tailMarker, tailMarkerOffset}) { + splitMarkers(range) { const { post } = this.editor; + const { + headSection, + tailSection, + headMarker, + headMarkerOffset, + tailMarker, + tailMarkerOffset + } = range; let selectedMarkers = []; - let headSection = headMarker.section; - let tailSection = tailMarker.section; - - // These render will be removed by the split functions. Mark them + // These render nodes will be removed by the split functions. Mark them // for removal before doing that. FIXME this seems prime for // refactoring onto the postEditor as a split function headMarker.renderNode.scheduleForRemoval(); @@ -550,10 +543,10 @@ class PostEditor { * * Usage: * - * let markerRange = editor.cursor.offsets; - * let strongMarkup = editor.builder.createMarkup('strong'); + * const range = editor.cursor.offsets; + * const strongMarkup = editor.builder.createMarkup('strong'); * editor.run((postEditor) => { - * postEditor.applyMarkupToMarkers(markerRange, strongMarkup); + * postEditor.applyMarkupToRange(range, strongMarkup); * }); * // Will result some markers possibly being split, and the markup * // being applied to all markers between the split. @@ -561,13 +554,13 @@ class PostEditor { * The return value will be all markers between the split, the same return * value as `splitMarkers`. * - * @method applyMarkupToMarkers - * @param {Object} markerRange Object with offsets - * @param {Object} markup A markup post abstract node + * @method applyMarkupToRange + * @param {Range} markerRange + * @param {Markup} markup A markup post abstract node * @return {Array} of markers that are inside the split * @public */ - applyMarkupToMarkers(markerRange, markup) { + applyMarkupToRange(markerRange, markup) { const markers = this.splitMarkers(markerRange); markers.forEach(marker => { marker.addMarkup(markup); @@ -587,10 +580,10 @@ class PostEditor { * * Usage: * - * let markerRange = editor.cursor.offsets; - * let markup = markerRange.headMarker.markups[0]; + * const range = editor.cursor.offsets; + * const markup = markerRange.headMarker.markups[0]; * editor.run((postEditor) => { - * postEditor.removeMarkupFromMarkers(markerRange, markup); + * postEditor.removeMarkupFromRange(range, markup); * }); * // Will result some markers possibly being split, and the markup * // being removed from all markers between the split. @@ -598,14 +591,14 @@ class PostEditor { * The return value will be all markers between the split, the same return * value as `splitMarkers`. * - * @method removeMarkupFromMarkers - * @param {Object} markerRange Object with offsets - * @param {Object} markup A markup post abstract node + * @method removeMarkupFromRange + * @param {Range} range Object with offsets + * @param {Markup} markup A markup post abstract node * @return {Array} of markers that are inside the split * @public */ - removeMarkupFromMarkers(markerRange, markupOrMarkupCallback) { - const markers = this.splitMarkers(markerRange); + removeMarkupFromRange(range, markupOrMarkupCallback) { + const markers = this.splitMarkers(range); markers.forEach(marker => { marker.removeMarkup(markupOrMarkupCallback); marker.section.renderNode.markDirty(); diff --git a/src/js/models/_markerable.js b/src/js/models/_markerable.js index 095e9292f..0c5e775c6 100644 --- a/src/js/models/_markerable.js +++ b/src/js/models/_markerable.js @@ -1,5 +1,6 @@ import { normalizeTagName } from '../utils/dom-utils'; import { forEach, filter, reduce } from '../utils/array-utils'; +import Set from '../utils/set'; import LinkedItem from '../utils/linked-item'; import LinkedList from '../utils/linked-list'; @@ -124,27 +125,64 @@ export default class Markerable extends LinkedItem { return reduce(this.markers, (prev, m) => prev + m.value, ''); } + get length() { + return this.text.length; + } + + /** + * @return {Array} New markers that match the boundaries of the + * range. + */ markersFor(headOffset, tailOffset) { + const range = {head: {section:this, offset:headOffset}, + tail: {section:this, offset:tailOffset}}; + let markers = []; - let adjustedHead = 0, adjustedTail = 0; - this.markers.forEach(m => { - adjustedTail += m.length; - - // if current 'window' of [adjustedHead..adjustedTail] is within - // [headOffset..tailOffset] range - if (adjustedTail > headOffset && adjustedHead < tailOffset) { - let head = Math.max(headOffset - adjustedHead, 0); - let tail = m.length - Math.max(adjustedTail - tailOffset, 0); - let cloned = m.clone(); - - cloned.value = m.value.slice(head, tail); - markers.push(cloned); - } - adjustedHead += m.length; + this._markersInRange(range, (marker, {markerHead, markerTail}) => { + const cloned = marker.clone(); + cloned.value = marker.value.slice(markerHead, markerTail); + markers.push(cloned); }); return markers; } + markupsInRange(range) { + const markups = new Set(); + this._markersInRange(range, marker => { + marker.markups.forEach(m => markups.add(m)); + }); + return markups.toArray(); + } + + // calls the callback with (marker, {markerHead, markerTail}) + // for each marker that is wholly or partially contained in the range + _markersInRange(range, callback) { + const { head, tail } = range; + if (head.section !== this || tail.section !== this) { + throw new Error('Cannot call #_markersInRange if range expands beyond this'); + } + const {offset:headOffset} = head, {offset:tailOffset} = tail; + + let currentHead = 0, currentTail = 0, currentMarker = this.markers.head; + + while (currentMarker) { + currentTail += currentMarker.length; + + if (currentTail > headOffset && currentHead < tailOffset) { + let markerHead = Math.max(headOffset - currentHead, 0); + let markerTail = currentMarker.length - + Math.max(currentTail - tailOffset, 0); + + callback(currentMarker, {markerHead, markerTail}); + } + + currentHead += currentMarker.length; + currentMarker = currentMarker.next; + + if (currentHead > tailOffset) { break; } + } + } + // mutates this by appending the other section's (cloned) markers to it join(otherSection) { let beforeMarker = this.markers.tail; diff --git a/src/js/models/post.js b/src/js/models/post.js index cff10cab7..d5eace880 100644 --- a/src/js/models/post.js +++ b/src/js/models/post.js @@ -1,6 +1,7 @@ export const POST_TYPE = 'post'; import LinkedList from 'content-kit-editor/utils/linked-list'; -import { compact } from 'content-kit-editor/utils/array-utils'; +import { forEach, compact } from 'content-kit-editor/utils/array-utils'; +import Set from 'content-kit-editor/utils/set'; export default class Post { constructor() { @@ -62,6 +63,7 @@ export default class Post { return {changedSections, removedSections}; } + /** * Invoke `callbackFn` for all markers between the headMarker and tailMarker (inclusive), * across sections @@ -83,6 +85,19 @@ export default class Post { } } + markupsInRange(range) { + const markups = new Set(); + + this.walkMarkerableSections(range, (section) => { + forEach( + section.markupsInRange(range.trimTo(section)), + m => markups.add(m) + ); + }); + + return markups.toArray(); + } + walkMarkerableSections(range, callback) { const {head, tail} = range; diff --git a/src/js/utils/cursor.js b/src/js/utils/cursor.js index 794d9abc8..a11ee3ace 100644 --- a/src/js/utils/cursor.js +++ b/src/js/utils/cursor.js @@ -84,21 +84,22 @@ const Cursor = class Cursor { const headSection = sections[0], tailSection = sections[sections.length - 1]; - const headMarker = headSection.markers.head, - tailMarker = tailSection.markers.tail; - - const headMarkerOffset = 0, - tailMarkerOffset = tailMarker.length; - - this.moveToMarker(headMarker, headMarkerOffset, tailMarker, tailMarkerOffset); + const range = new Range( + new Position(headSection, 0), + new Position(tailSection, tailSection.text.length) + ); + this.selectRange(range); } - selectMarkers(markers) { - const headMarker = markers[0], - tailMarker = markers[markers.length - 1], - headOffset = 0, - tailOffset = tailMarker.length; - this.moveToMarker(headMarker, headOffset, tailMarker, tailOffset); + selectRange(range) { + const { + headMarker, + headMarkerOffset, + tailMarker, + tailMarkerOffset + } = range; + this.moveToMarker( + headMarker, headMarkerOffset, tailMarker, tailMarkerOffset); } get selection() { @@ -109,6 +110,10 @@ const Cursor = class Cursor { return this.selection.toString(); } + moveToPosition(position) { + this.selectRange(new Range(position, position)); + } + /** * @private * @param {textNode} node diff --git a/src/js/utils/cursor/range.js b/src/js/utils/cursor/range.js index b78e61d6b..1813965ee 100644 --- a/src/js/utils/cursor/range.js +++ b/src/js/utils/cursor/range.js @@ -1,9 +1,33 @@ +import Position from './position'; + export default class Range { constructor(head, tail) { this.head = head; this.tail = tail; } + /** + * @param {Markerable} section + * @return {Range} A range that is constrained to only the part that + * includes the section. + * FIXME -- if the section isn't the head or tail, it's assumed to be + * wholly contained. It's possible to call `trimTo` with a selection that is + * outside of the range, though, which would invalidate that assumption. + */ + trimTo(section) { + const length = section.length; + + let headOffset = section === this.head.section ? + Math.min(this.head.offset, length) : 0; + let tailOffset = section === this.tail.section ? + Math.min(this.tail.offset, length) : length; + + return new Range( + new Position(section, headOffset), + new Position(section, tailOffset) + ); + } + // "legacy" APIs get headSection() { return this.head.section; diff --git a/src/js/utils/set.js b/src/js/utils/set.js new file mode 100644 index 000000000..72d377b59 --- /dev/null +++ b/src/js/utils/set.js @@ -0,0 +1,20 @@ +export default class Set { + constructor(items=[]) { + this.items = []; + items.forEach(i => this.push(i)); + } + + add(item) { + if (!this.has(item)) { + this.items.push(item); + } + } + + has(item) { + return this.items.indexOf(item) !== -1; + } + + toArray() { + return this.items; + } +} diff --git a/tests/acceptance/editor-selections-test.js b/tests/acceptance/editor-selections-test.js index f42d2df22..4f2ce4761 100644 --- a/tests/acceptance/editor-selections-test.js +++ b/tests/acceptance/editor-selections-test.js @@ -457,3 +457,30 @@ test('selecting text that includes a card section and deleting deletes card sect done(); }); }); + +test('selecting text that touches bold text should not be considered bold by the toolbar', (assert) => { + const done = assert.async(); + + const mobiledoc = Helpers.mobiledoc.build(({post, markupSection, marker}) => { + return post([markupSection('p', [marker('abc')])]); + }); + editor = new Editor({mobiledoc}); + editor.render(editorElement); + + Helpers.dom.selectText('b', editorElement); + Helpers.dom.triggerEvent(document, 'mouseup'); + + setTimeout(() => { + Helpers.toolbar.clickButton(assert, 'bold'); + + assert.hasElement('#editor strong:contains(b)', 'precond - bold text'); + + Helpers.dom.selectText('c', editorElement); + Helpers.dom.triggerEvent(document, 'mouseup'); + + setTimeout(() => { + assert.inactiveButton('bold', 'when selecting text next to bold text, bold button is inactive'); + done(); + }); + }); +}); diff --git a/tests/helpers/assertions.js b/tests/helpers/assertions.js index 568fb5272..bf7be42f6 100644 --- a/tests/helpers/assertions.js +++ b/tests/helpers/assertions.js @@ -42,4 +42,8 @@ export default function registerAssertions() { const btn = _getToolbarButton(name); QUnit.assert.ok(btn.is('.active'), message); }; + + QUnit.assert.inArray = function(element, array, message=`has "${element}" in "${array}"`) { + QUnit.assert.ok(array.indexOf(element) !== -1, message); + }; } diff --git a/tests/unit/models/post-test.js b/tests/unit/models/post-test.js index 518da032f..dc8ea2e7a 100644 --- a/tests/unit/models/post-test.js +++ b/tests/unit/models/post-test.js @@ -10,6 +10,10 @@ module('Unit: Post', { } }); +function makeRange(s1, o1, s2, o2) { + return new Range(new Position(s1, o1), new Position(s2, o2)); +} + test('#markersFrom finds markers across markup sections', (assert) => { const post = Helpers.postAbstract.build(({post, markupSection, marker}) => post([ @@ -101,3 +105,73 @@ test('#walkMarkerableSections skips non-markerable sections', (assert) => { 'iterates correct sections'); }); + +test('#markupsInRange returns all markups', (assert) => { + let b, i, a1, a2, found; + const post = Helpers.postAbstract.build(builder => { + const {post, markupSection, cardSection, marker, markup} = builder; + + b = markup('strong'); + i = markup('em'); + a1 = markup('a', ['href', 'example.com']); + a2 = markup('a', ['href', 'other-example.com']); + + return post([ + markupSection('p', [ + marker('plain text'), + marker('bold text', [b]), + marker('i text', [i]), + marker('bold+i text', [b, i]) + ]), + markupSection('p', [ + marker('link 1', [a1]) + ]), + cardSection('simple-card'), + markupSection('p', [ + marker('link 2', [a2]) + ]) + ]); + }); + + const [s1, s2,, s3] = post.sections.toArray(); + + assert.equal(s1.text, 'plain textbold texti textbold+i text', 'precond s1'); + assert.equal(s2.text, 'link 1', 'precond s2'); + assert.equal(s3.text, 'link 2', 'precond s3'); + + const collapsedRange = makeRange(s1, 0, s1, 0); + assert.equal(post.markupsInRange(collapsedRange).length, 0, + 'no markups in collapsed range'); + + const simpleRange = makeRange(s1, 0, s1, 'plain text'.length); + assert.equal(post.markupsInRange(simpleRange).length, 0, + 'no markups in simple range'); + + const singleMarkerRange = makeRange(s1, 'plain textb'.length, + s1, 'plain textbold'.length); + found = post.markupsInRange(singleMarkerRange); + assert.equal(found.length, 1, 'finds markup in marker'); + assert.inArray(b, found, 'finds b'); + + const singleSectionRange = makeRange(s1, 0, s1, s1.text.length); + found = post.markupsInRange(singleSectionRange); + assert.equal(found.length, 2, 'finds both markups in section'); + assert.inArray(b, found, 'finds b'); + assert.inArray(i, found, 'finds i'); + + const multiSectionRange = makeRange(s1, 'plain textbold te'.length, + s2, 'link'.length); + found = post.markupsInRange(multiSectionRange); + assert.equal(found.length, 3, 'finds all markups in multi-section range'); + assert.inArray(b, found, 'finds b'); + assert.inArray(i, found, 'finds i'); + assert.inArray(a1, found, 'finds a1'); + + const rangeSpanningCard = makeRange(s1, 0, s3, 'link'.length); + found = post.markupsInRange(rangeSpanningCard); + assert.equal(found.length, 4, 'finds all markups in spanning section range'); + assert.inArray(b, found, 'finds b'); + assert.inArray(i, found, 'finds i'); + assert.inArray(a1, found, 'finds a1'); + assert.inArray(a2, found, 'finds a2'); +}); diff --git a/tests/unit/utils/cursor-range-test.js b/tests/unit/utils/cursor-range-test.js new file mode 100644 index 000000000..59c1c857e --- /dev/null +++ b/tests/unit/utils/cursor-range-test.js @@ -0,0 +1,73 @@ +import Helpers from '../../test-helpers'; +import Position from 'content-kit-editor/utils/cursor/position'; +import Range from 'content-kit-editor/utils/cursor/range'; + +const {module, test} = Helpers; + +function makeRange(s1, o1, s2, o2) { + return new Range(new Position(s1, o1), new Position(s2, o2)); +} + +module('Unit: Utils: Range'); + +test('#trimTo(section) when range covers only one section', (assert) => { + const section = Helpers.postAbstract.build(({markupSection}) => markupSection()); + const range = makeRange(section, 0, section, 5); + + const newRange = range.trimTo(section); + assert.ok(newRange.head.section === section, 'head section is correct'); + assert.ok(newRange.tail.section === section, 'tail section is correct'); + assert.equal(newRange.head.offset, 0, 'head offset'); + assert.equal(newRange.tail.offset, 0, 'tail offset'); +}); + +test('#trimTo head section', (assert) => { + const text = 'abcdef'; + const section1 = Helpers.postAbstract.build( + ({markupSection, marker}) => markupSection('p', [marker(text)])); + const section2 = Helpers.postAbstract.build( + ({markupSection, marker}) => markupSection('p', [marker(text)])); + + const range = makeRange(section1, 0, section2, 5); + const newRange = range.trimTo(section1); + + assert.ok(newRange.head.section === section1, 'head section'); + assert.ok(newRange.tail.section === section1, 'tail section'); + assert.equal(newRange.head.offset, 0, 'head offset'); + assert.equal(newRange.tail.offset, text.length, 'tail offset'); +}); + +test('#trimTo tail section', (assert) => { + const text = 'abcdef'; + const section1 = Helpers.postAbstract.build( + ({markupSection, marker}) => markupSection('p', [marker(text)])); + const section2 = Helpers.postAbstract.build( + ({markupSection, marker}) => markupSection('p', [marker(text)])); + + const range = makeRange(section1, 0, section2, 5); + const newRange = range.trimTo(section2); + + assert.ok(newRange.head.section === section2, 'head section'); + assert.ok(newRange.tail.section === section2, 'tail section'); + assert.equal(newRange.head.offset, 0, 'head offset'); + assert.equal(newRange.tail.offset, 5, 'tail offset'); +}); + +test('#trimTo middle section', (assert) => { + const text = 'abcdef'; + const section1 = Helpers.postAbstract.build( + ({markupSection, marker}) => markupSection('p', [marker(text)])); + const section2 = Helpers.postAbstract.build( + ({markupSection, marker}) => markupSection('p', [marker(text)])); + const section3 = Helpers.postAbstract.build( + ({markupSection, marker}) => markupSection('p', [marker(text)])); + + const range = makeRange(section1, 0, section3, 5); + const newRange = range.trimTo(section2); + + assert.ok(newRange.head.section === section2, 'head section'); + assert.ok(newRange.tail.section === section2, 'tail section'); + assert.equal(newRange.head.offset, 0, 'head offset'); + assert.equal(newRange.tail.offset, section2.text.length, 'tail offset'); + +});