Skip to content

Commit

Permalink
Merge pull request #765 from yabwe/list-item-anchors
Browse files Browse the repository at this point in the history 
Fix #732 - Fix issue with selection inside nested block elements
  • Loading branch information
@nmielnik
nmielnik committed Aug 10, 2015
2 parents f0278c3 + 04db09b commit a79ee24
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 35 deletions.
56 changes: 40 additions & 16 deletions spec/selection.spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*global MediumEditor, describe, it, expect, spyOn,
afterEach, beforeEach, fireEvent, Util,
afterEach, beforeEach, fireEvent,
jasmine, selectElementContents, setupTestHelpers,
selectElementContentsAndFire, Selection,
placeCursorInsideElement */
Expand Down Expand Up @@ -202,7 +202,7 @@ describe('Selection TestCase', function () {
'emptyBlocksIndex': 1
});

var startParagraph = Util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'p');
var startParagraph = MediumEditor.util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'p');
expect(startParagraph).toBe(editor.elements[0].getElementsByTagName('p')[1], 'empty paragraph');
});

Expand All @@ -217,7 +217,7 @@ describe('Selection TestCase', function () {
'emptyBlocksIndex': 2
});

var startParagraph = Util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'p');
var startParagraph = MediumEditor.util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'p');
expect(startParagraph).toBe(editor.elements[0].getElementsByTagName('p')[2], 'paragraph after empty paragraph');
});

Expand All @@ -233,7 +233,7 @@ describe('Selection TestCase', function () {
'emptyBlocksIndex': 2
});

var startParagraph = Util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'p');
var startParagraph = MediumEditor.util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'p');
expect(startParagraph).toBe(editor.elements[1].getElementsByTagName('p')[2], 'paragraph after empty paragraph');
});

Expand All @@ -248,7 +248,7 @@ describe('Selection TestCase', function () {
'emptyBlocksIndex': 2
});

var startParagraph = Util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'h2');
var startParagraph = MediumEditor.util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'h2');
expect(startParagraph).toBe(editor.elements[0].querySelector('h2'), 'block element after empty block element');
});

Expand All @@ -263,7 +263,7 @@ describe('Selection TestCase', function () {
'emptyBlocksIndex': 2
});

var startParagraph = Util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'h2');
var startParagraph = MediumEditor.util.getClosestTag(window.getSelection().getRangeAt(0).startContainer, 'h2');
expect(startParagraph).toBe(editor.elements[0].querySelector('h2'), 'block element after empty block element');
});

Expand All @@ -279,7 +279,7 @@ describe('Selection TestCase', function () {
});

var innerElement = window.getSelection().getRangeAt(0).startContainer;
expect(Util.isDescendant(editor.elements[0].querySelector('i'), innerElement, true)).toBe(true, 'nested inline elment inside block element after empty block element');
expect(MediumEditor.util.isDescendant(editor.elements[0].querySelector('i'), innerElement, true)).toBe(true, 'nested inline elment inside block element after empty block element');
});

['br', 'img'].forEach(function (tagName) {
Expand Down Expand Up @@ -317,7 +317,7 @@ describe('Selection TestCase', function () {

var innerElement = window.getSelection().getRangeAt(0).startContainer;
// The behavior varies from browser to browser for this case, some select TH, some #textNode
expect(Util.isDescendant(editor.elements[0].querySelector('th'), innerElement, true))
expect(MediumEditor.util.isDescendant(editor.elements[0].querySelector('th'), innerElement, true))
.toBe(true, 'expect selection to be of TH or a descendant');
expect(innerElement).toBe(window.getSelection().getRangeAt(0).endContainer);
});
Expand All @@ -335,8 +335,32 @@ describe('Selection TestCase', function () {
});

var innerElement = window.getSelection().getRangeAt(0).startContainer;
expect(Util.isDescendant(editor.elements[0].querySelectorAll('p')[1], innerElement, true)).toBe(false, 'moved selection beyond non-empty block element');
expect(Util.isDescendant(editor.elements[0].querySelector('h2'), innerElement, true)).toBe(true, 'moved selection to element to incorrect block element');
expect(MediumEditor.util.isDescendant(editor.elements[0].querySelectorAll('p')[1], innerElement, true)).toBe(false, 'moved selection beyond non-empty block element');
expect(MediumEditor.util.isDescendant(editor.elements[0].querySelector('h2'), innerElement, true)).toBe(true, 'moved selection to element to incorrect block element');
});

// https://github.com/yabwe/medium-editor/issues/732
it('should support a selection correctly when space + newlines are separating block elements', function () {
this.el.innerHTML = '<ul>\n' +
' <li><a href="#">a link</a></li>\n' +
' <li>a list item</li>\n' +
' <li>target</li>\n' +
'</ul>';
var editor = this.newMediumEditor('.editor'),
lastLi = this.el.querySelectorAll('ul > li')[2];

// Select the <li> with 'target'
selectElementContents(lastLi.firstChild);

var selectionData = editor.exportSelection();
expect(selectionData.emptyBlocksIndex).toBe(0);

editor.importSelection(selectionData);
var range = window.getSelection().getRangeAt(0);

expect(range.toString()).toBe('target', 'The selection is around the wrong element');
expect(MediumEditor.util.isDescendant(lastLi, range.startContainer, true)).toBe(true, 'The start of the selection is invalid');
expect(MediumEditor.util.isDescendant(lastLi, range.endContainer, true)).toBe(true, 'The end of the selection is invalid');
});
});

Expand Down Expand Up @@ -457,7 +481,7 @@ describe('Selection TestCase', function () {

describe('getSelectedElements', function () {
it('no selected elements on empty selection', function () {
var elements = Selection.getSelectedElements(document);
var elements = MediumEditor.selection.getSelectedElements(document);

expect(elements.length).toBe(0);
});
Expand All @@ -468,7 +492,7 @@ describe('Selection TestCase', function () {
elements;

selectElementContents(editor.elements[0].querySelector('i').firstChild);
elements = Selection.getSelectedElements(document);
elements = MediumEditor.selection.getSelectedElements(document);

expect(elements.length).toBe(1);
expect(elements[0].nodeName.toLowerCase()).toBe('i');
Expand All @@ -480,7 +504,7 @@ describe('Selection TestCase', function () {
var elements;

selectElementContents(this.el);
elements = Selection.getSelectedElements(document);
elements = MediumEditor.selection.getSelectedElements(document);

expect(elements.length).toBe(1);
expect(elements[0].nodeName.toLowerCase()).toBe('i');
Expand All @@ -490,8 +514,8 @@ describe('Selection TestCase', function () {

describe('getSelectedParentElement', function () {
it('should return null on bad range', function () {
expect(Selection.getSelectedParentElement(null)).toBe(null);
expect(Selection.getSelectedParentElement(false)).toBe(null);
expect(MediumEditor.selection.getSelectedParentElement(null)).toBe(null);
expect(MediumEditor.selection.getSelectedParentElement(false)).toBe(null);
});

it('should select the document', function () {
Expand All @@ -506,7 +530,7 @@ describe('Selection TestCase', function () {
sel.removeAllRanges();
sel.addRange(range);

element = Selection.getSelectedParentElement(range);
element = MediumEditor.selection.getSelectedParentElement(range);

expect(element).toBe(document);
});
Expand Down
69 changes: 52 additions & 17 deletions src/js/selection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,21 +120,7 @@ var Selection;
}

if (typeof selectionState.emptyBlocksIndex !== 'undefined') {
var targetNode = Util.getTopBlockContainer(range.startContainer),
index = 0;
// Skip over empty blocks until we hit the block we want the selection to be in
while ((index === 0 || index < selectionState.emptyBlocksIndex) && targetNode.nextSibling) {
targetNode = targetNode.nextSibling;
index++;
// If we find a non-empty block, ignore the emptyBlocksIndex and just put selection here
if (targetNode.textContent.length > 0) {
break;
}
}

// We're selecting a high-level block node, so make sure the cursor gets moved into the deepest
// element at the beginning of the block
range.setStart(Util.getFirstSelectableLeafNode(targetNode), 0);
range = Selection.importSelectionMoveCursorPastBlocks(doc, root, selectionState.emptyBlocksIndex, range);
}

// If the selection is right at the ending edge of a link, put it outside the anchor tag instead of inside.
Expand Down Expand Up @@ -180,6 +166,54 @@ var Selection;
return range;
},

// Uses the emptyBlocksIndex calculated by getIndexRelativeToAdjacentEmptyBlocks
// to move the cursor back to the start of the correct paragraph
importSelectionMoveCursorPastBlocks: function (doc, root, index, range) {
var treeWalker = doc.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, filterOnlyParentElements, false),
startContainer = range.startContainer,
startBlock,
targetNode,
currIndex = 0;
index = index || 1; // If index is 0, we still want to move to the next block

// Chrome counts newlines and spaces that separate block elements as actual elements.
// If the selection is inside one of these text nodes, and it has a previous sibling
// which is a block element, we want the treewalker to start at the previous sibling
// and NOT at the parent of the textnode
if (startContainer.nodeType === 3 && Util.isBlockContainer(startContainer.previousSibling)) {
startBlock = startContainer.previousSibling;
} else {
startBlock = Util.getClosestBlockContainer(startContainer);
}

// Skip over empty blocks until we hit the block we want the selection to be in
while (treeWalker.nextNode()) {
if (!targetNode) {
// Loop through all blocks until we hit the starting block element
if (startBlock === treeWalker.currentNode) {
targetNode = treeWalker.currentNode;
}
} else {
targetNode = treeWalker.currentNode;
currIndex++;
// We hit the target index, bail
if (currIndex === index) {
break;
}
// If we find a non-empty block, ignore the emptyBlocksIndex and just put selection here
if (targetNode.textContent.length > 0) {
break;
}
}
}

// We're selecting a high-level block node, so make sure the cursor gets moved into the deepest
// element at the beginning of the block
range.setStart(Util.getFirstSelectableLeafNode(targetNode), 0);

return range;
},

// Returns -1 unless the cursor is at the beginning of a paragraph/block
// If the paragraph/block is preceeded by empty paragraphs/block (with no text)
// it will return the number of empty paragraphs before the cursor.
Expand All @@ -202,14 +236,15 @@ var Selection;

// Walk over block elements, counting number of empty blocks between last piece of text
// and the block the cursor is in
var treeWalker = doc.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, filterOnlyParentElements, false),
var closestBlock = Util.getClosestBlockContainer(cursorContainer),
treeWalker = doc.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, filterOnlyParentElements, false),
emptyBlocksCount = 0;
while (treeWalker.nextNode()) {
var blockIsEmpty = treeWalker.currentNode.textContent === '';
if (blockIsEmpty || emptyBlocksCount > 0) {
emptyBlocksCount += 1;
}
if (Util.isDescendant(treeWalker.currentNode, cursorContainer, true)) {
if (treeWalker.currentNode === closestBlock) {
return emptyBlocksCount;
}
if (!blockIsEmpty) {
Expand Down
12 changes: 10 additions & 2 deletions src/js/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,15 @@ var Util;
return keyCode;
},

blockContainerElementNames: ['p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'blockquote', 'pre'],
blockContainerElementNames: [
// elements our editor generates
'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'blockquote', 'pre', 'ul', 'li', 'ol',
// all other known block elements
'address', 'article', 'aside', 'audio', 'canvas', 'dd', 'dl', 'dt', 'fieldset',
'figcaption', 'figure', 'footer', 'form', 'header', 'hgroup', 'main', 'nav',
'noscript', 'output', 'section', 'table', 'tbody', 'tfoot', 'video'
],

emptyElementNames: ['br', 'col', 'colgroup', 'hr', 'img', 'input', 'source', 'wbr'],

extend: function extend(/* dest, source1, source2, ...*/) {
Expand Down Expand Up @@ -461,7 +469,7 @@ var Util;

var parentNode = node.parentNode,
tagName = parentNode.nodeName.toLowerCase();
while (!this.isBlockContainer(parentNode) && tagName !== 'div') {
while (tagName === 'li' || (!this.isBlockContainer(parentNode) && tagName !== 'div')) {
if (tagName === 'li') {
return true;
}
Expand Down

0 comments on commit a79ee24

Please sign in to comment.