Skip to content

Commit

Permalink
Use post#markersContainedByRange in postEditor#splitMarkers
Browse files Browse the repository at this point in the history 
fixes #121
  • Loading branch information
@bantic
bantic committed Sep 9, 2015
1 parent fa83c91 commit 63cb72a
Show file tree
Hide file tree
Showing 7 changed files with 235 additions and 91 deletions.
44 changes: 4 additions & 40 deletions src/js/editor/post.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,6 @@ class PostEditor {
tailMarker,
tailMarkerOffset
} = range;
let selectedMarkers = [];

// These render nodes will be removed by the split functions. Mark them
// for removal before doing that. FIXME this seems prime for
Expand All @@ -384,51 +383,16 @@ class PostEditor {
tailMarker.section.renderNode.markDirty();

if (headMarker === tailMarker) {
let markers = headSection.splitMarker(headMarker, headMarkerOffset, tailMarkerOffset);
selectedMarkers = post.markersInRange({
headMarker: markers[0],
tailMarker: markers[markers.length-1],
headMarkerOffset,
tailMarkerOffset
});
headSection.splitMarker(headMarker, headMarkerOffset, tailMarkerOffset);
} else {
let newHeadMarkers = headSection.splitMarker(headMarker, headMarkerOffset);
let selectedHeadMarkers = post.markersInRange({
headMarker: newHeadMarkers[0],
tailMarker: newHeadMarkers[newHeadMarkers.length-1],
headMarkerOffset
});

let newTailMarkers = tailSection.splitMarker(tailMarker, 0, tailMarkerOffset);
let selectedTailMarkers = post.markersInRange({
headMarker: newTailMarkers[0],
tailMarker: newTailMarkers[newTailMarkers.length-1],
headMarkerOffset: 0,
tailMarkerOffset
});

let newHeadMarker = selectedHeadMarkers[0],
newTailMarker = selectedTailMarkers[selectedTailMarkers.length - 1];

let newMarkers = [];
if (newHeadMarker) {
newMarkers.push(newHeadMarker);
}
if (newTailMarker) {
newMarkers.push(newTailMarker);
}

if (newMarkers.length) {
this.editor.post.markersFrom(newMarkers[0], newMarkers[newMarkers.length-1], m => {
selectedMarkers.push(m);
});
}
headSection.splitMarker(headMarker, headMarkerOffset);
tailSection.splitMarker(tailMarker, 0, tailMarkerOffset);
}

this.scheduleRerender();
this.scheduleDidUpdate();

return selectedMarkers;
return post.markersContainedByRange(range);
}

splitMarker(marker, offset) {
Expand Down
7 changes: 4 additions & 3 deletions src/js/models/_markerable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ export default class Markerable extends LinkedItem {
return markups.toArray();
}

// calls the callback with (marker, {markerHead, markerTail})
// for each marker that is wholly or partially contained in the range
// calls the callback with (marker, {markerHead, markerTail, isContained})
// 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) {
Expand All @@ -172,8 +172,9 @@ export default class Markerable extends LinkedItem {
let markerHead = Math.max(headOffset - currentHead, 0);
let markerTail = currentMarker.length -
Math.max(currentTail - tailOffset, 0);
let isContained = markerHead === 0 && markerTail === currentMarker.length;

callback(currentMarker, {markerHead, markerTail});
callback(currentMarker, {markerHead, markerTail, isContained});
}

currentHead += currentMarker.length;
Expand Down
28 changes: 12 additions & 16 deletions src/js/models/post.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,21 @@ export default class Post {
});
}

markersInRange({headMarker, headMarkerOffset, tailMarker, tailMarkerOffset}) {
let offset = 0;
let foundMarkers = [];
let toEnd = tailMarkerOffset === undefined;
if (toEnd) { tailMarkerOffset = 0; }

this.markersFrom(headMarker, tailMarker, marker => {
if (toEnd) {
tailMarkerOffset += marker.length;
}

if (offset >= headMarkerOffset && offset < tailMarkerOffset) {
foundMarkers.push(marker);
}
/**
* @param {Range} range
* @return {Array} markers that are completely contained by the range
*/
markersContainedByRange(range) {
const markers = [];

offset += marker.length;
this.walkMarkerableSections(range, section => {
section._markersInRange(
range.trimTo(section),
(m, {isContained}) => { if (isContained) { markers.push(m); } }
);
});

return foundMarkers;
return markers;
}

cutMarkers(markers) {
Expand Down
12 changes: 8 additions & 4 deletions src/js/utils/cursor/range.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ export default class Range {
this.tail = tail;
}

static create(headSection, headOffset, tailSection, tailOffset) {
return new Range(
new Position(headSection, headOffset),
new Position(tailSection, tailOffset)
);
}

/**
* @param {Markerable} section
* @return {Range} A range that is constrained to only the part that
Expand All @@ -22,10 +29,7 @@ export default class Range {
let tailOffset = section === this.tail.section ?
Math.min(this.tail.offset, length) : length;

return new Range(
new Position(section, headOffset),
new Position(section, tailOffset)
);
return Range.create(section, headOffset, section, tailOffset);
}

// "legacy" APIs
Expand Down
30 changes: 30 additions & 0 deletions tests/acceptance/editor-selections-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,3 +484,33 @@ test('selecting text that touches bold text should not be considered bold by the
});
});
});

// https://github.com/bustlelabs/content-kit-editor/issues/121
test('selecting text that includes a 1-character marker and unbolding it', (assert) => {
const done = assert.async();

const mobiledoc = Helpers.mobiledoc.build(({post, markupSection, marker, markup}) => {
const b = markup('strong');
return post([markupSection('p', [
marker('a'),
marker('b',[b]),
marker('c')
])]);
});
editor = new Editor({mobiledoc});
editor.render(editorElement);

assert.hasElement('#editor strong:contains(b)', 'precond - bold');

Helpers.dom.selectText('b', editorElement, 'c', editorElement);
Helpers.dom.triggerEvent(document, 'mouseup');

setTimeout(() => {
assert.activeButton('bold');
Helpers.toolbar.clickButton(assert, 'bold');

assert.hasNoElement('#editor strong', 'bold text is unboldened');

done();
});
});
86 changes: 70 additions & 16 deletions tests/unit/editor/post-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,16 @@ import { Editor } from 'content-kit-editor';
import Helpers from '../../test-helpers';
import { DIRECTION } from 'content-kit-editor/utils/key';
import PostNodeBuilder from 'content-kit-editor/models/post-node-builder';
import {Position, Range} from 'content-kit-editor/utils/cursor';
import Range from 'content-kit-editor/utils/cursor/range';

const { FORWARD } = DIRECTION;

const { module, test } = window.QUnit;
const { module, test } = Helpers;

let editor, editorElement;

let builder, postEditor, mockEditor;

function makeRange(headSection, headOffset, tailSection, tailOffset) {
return new Range(
new Position(headSection, headOffset),
new Position(tailSection, tailOffset)
);
}

function getSection(sectionIndex) {
return editor.post.sections.objectAt(sectionIndex);
}
Expand Down Expand Up @@ -219,7 +212,7 @@ test('#deleteRange when within the same marker', (assert) => {

renderBuiltAbstract(post);

const range = makeRange(section, 3, section, 4);
const range = Range.create(section, 3, section, 4);

postEditor.deleteRange(range);

Expand All @@ -242,7 +235,7 @@ test('#deleteRange when same section, different markers, same markups', (assert)

renderBuiltAbstract(post);

const range = makeRange(section, 3, section, 4);
const range = Range.create(section, 3, section, 4);
postEditor.deleteRange(range);
postEditor.complete();

Expand All @@ -264,7 +257,7 @@ test('#deleteRange when same section, different markers, different markups', (as

renderBuiltAbstract(post);

const range = makeRange(section, 3, section, 4);
const range = Range.create(section, 3, section, 4);
postEditor.deleteRange(range);
postEditor.complete();

Expand All @@ -286,7 +279,7 @@ test('#deleteRange across contiguous sections', (assert) => {

renderBuiltAbstract(post);

const range = makeRange(s1, 3, s2, 1);
const range = Range.create(s1, 3, s2, 1);
postEditor.deleteRange(range);
postEditor.complete();

Expand All @@ -305,7 +298,7 @@ test('#deleteRange across entire sections', (assert) => {

renderBuiltAbstract(post);

const range = makeRange(s1, 3, s3, 0);
const range = Range.create(s1, 3, s3, 0);
postEditor.deleteRange(range);
postEditor.complete();

Expand All @@ -323,7 +316,7 @@ test('#deleteRange across all content', (assert) => {

renderBuiltAbstract(post);

const range = makeRange(s1, 0, s2, 3);
const range = Range.create(s1, 0, s2, 3);
postEditor.deleteRange(range);

postEditor.complete();
Expand Down Expand Up @@ -351,7 +344,7 @@ test('#cutSection with one marker', (assert) => {
assert.equal(post.sections.head.markers.length, 2, 'two markers remain');
});

test('#cutSection at bounderies across markers', (assert) => {
test('#cutSection at boundaries across markers', (assert) => {
let post, section;
Helpers.postAbstract.build(({marker, markupSection, post: buildPost}) => {
section = markupSection('p', [
Expand Down Expand Up @@ -415,3 +408,64 @@ test('#cutSection in tail marker', (assert) => {
assert.equal(post.sections.length, 1, 'only 1 section remains');
assert.equal(post.sections.head.markers.length, 1, 'two markers remain');
});

test('#splitMarkers when headMarker = tailMarker', (assert) => {
let post, section;
Helpers.postAbstract.build(({marker, markupSection, post: buildPost}) => {
section = markupSection('p', [
marker('abcd')
]);
post = buildPost([ section ]);
});

renderBuiltAbstract(post);

const range = Range.create(section, 1, section, 3);
const markers = postEditor.splitMarkers(range);
postEditor.complete();

assert.equal(markers.length, 1, 'markers');
assert.equal(markers[0].value, 'bc', 'marker 0');
});

test('#splitMarkers when head section = tail section, but different markers', (assert) => {
const post = Helpers.postAbstract.build(({marker, markupSection, post}) =>
post([
markupSection('p', [marker('abc'), marker('def')])
])
);

renderBuiltAbstract(post);

const section = post.sections.head;
const range = Range.create(section, 2, section, 5);
const markers = postEditor.splitMarkers(range);
postEditor.complete();

assert.equal(markers.length, 2, 'markers');
assert.equal(markers[0].value, 'c', 'marker 0');
assert.equal(markers[1].value, 'de', 'marker 1');
});

// see https://github.com/bustlelabs/content-kit-editor/issues/121
test('#splitMarkers when single-character marker at start', (assert) => {
let post, section;
Helpers.postAbstract.build(({marker, markupSection, post: buildPost}) => {
section = markupSection('p', [
marker('a'),
marker('b'),
marker('c')
]);
post = buildPost([ section ]);
});

renderBuiltAbstract(post);

const range = Range.create(section, 1, section, 3);
const markers = postEditor.splitMarkers(range);
postEditor.complete();

assert.equal(markers.length, 2, 'markers');
assert.equal(markers[0].value, 'b', 'marker 0');
assert.equal(markers[1].value, 'c', 'marker 1');
});
Loading

0 comments on commit 63cb72a

Please sign in to comment.