From 88554612dbd5125ce832d3204d7c56d89fa372bb Mon Sep 17 00:00:00 2001 From: Matthew Beale Date: Thu, 22 Oct 2015 11:33:00 -0400 Subject: [PATCH] execCommands for bold, italic without selection Currently, Content-Kit still parses the DOM after each keystroke. We are pretty good at it (text and DOM nodes from parsed section are re-used instead of being rerendered) however it does mean we cannot easily create our own stateful markup system for bold and italic. Specifically, for when bold or italic are toggled without a selection. This patch restores a native behavior for contentEditables: Toggling italic or bold without a selection will just call the proper execCommand. This then applied to the next input. The compromise is that execCommands vary from implementation to implementation, for example on Chrome they output a `b` tag, however on IE a `strong` tag is generated. See the following link for more details: http://help.dottoro.com/ljcvtcaw.php Here we normalize the `b` tag during section parse (there are two DOM parsers, this is the "safe" DOM one) into a `strong` markup. This means buttons toggling `strong` continue to work and the `b` tag is not persisted into the mobiledoc. However, the `b` tag stays in DOM. Additionally: * Cleans up setup/teardown for some tests * Use `ok` and equality checks to avoid QUnits terrible diffing system for displayed failures of `equal` * During reparse, we were resetting the cursor position. This was related to ctrl-k on OSX, which clears to the end of the line. Here that functionality is re-implemented as a key command. --- src/js/editor/editor.js | 18 +-- src/js/editor/key-commands.js | 40 +++++- src/js/parsers/post.js | 13 +- tests/acceptance/editor-key-commands-test.js | 134 ++++++++++++++++--- tests/acceptance/editor-sections-test.js | 28 ++-- 5 files changed, 185 insertions(+), 48 deletions(-) diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js index befcae648..92a92eb32 100644 --- a/src/js/editor/editor.js +++ b/src/js/editor/editor.js @@ -305,13 +305,6 @@ class Editor { } reparse() { - let { headSection, headSectionOffset } = this.cursor.offsets; - if (headSectionOffset === 0) { - // FIXME if the offset is 0, the user is typing the first character - // in an empty section, so we need to move the cursor 1 letter forward - headSectionOffset = 1; - } - this._reparseCurrentSection(); this._removeDetachedSections(); @@ -320,8 +313,6 @@ class Editor { this.run(() => {}); this.rerender(); this.trigger('update'); - - this.cursor.moveToSection(headSection, headSectionOffset); } // FIXME this should be able to be removed now -- if any sections are detached, @@ -595,13 +586,12 @@ class Editor { } handleKeydown(event) { - if (!this.isEditable) { return; } - if (this.post.isBlank) { - this._insertEmptyMarkupSectionAtCursor(); + if (!this.isEditable || this.handleKeyCommand(event)) { + return; } - if (this.handleKeyCommand(event)) { - return; + if (this.post.isBlank) { + this._insertEmptyMarkupSectionAtCursor(); } const key = Key.fromEvent(event); diff --git a/src/js/editor/key-commands.js b/src/js/editor/key-commands.js index 927169a8a..a1164337e 100644 --- a/src/js/editor/key-commands.js +++ b/src/js/editor/key-commands.js @@ -2,36 +2,66 @@ import Key from '../utils/key'; import { MODIFIERS, SPECIAL_KEYS } from '../utils/key'; import { filter } from '../utils/array-utils'; import LinkCommand from '../commands/link'; +import Position from '../utils/cursor/position'; export const DEFAULT_KEY_COMMANDS = [{ modifier: MODIFIERS.META, str: 'B', run(editor) { - editor.run(postEditor => postEditor.toggleMarkup('strong')); + if (editor.cursor.hasSelection()) { + editor.run(postEditor => postEditor.toggleMarkup('strong')); + } else { + document.execCommand('bold', false, null); + } } }, { modifier: MODIFIERS.CTRL, str: 'B', run(editor) { - editor.run(postEditor => postEditor.toggleMarkup('strong')); + if (editor.cursor.hasSelection()) { + editor.run(postEditor => postEditor.toggleMarkup('strong')); + } else { + document.execCommand('bold', false, null); + } } }, { modifier: MODIFIERS.META, str: 'I', run(editor) { - editor.run(postEditor => postEditor.toggleMarkup('em')); + if (editor.cursor.hasSelection()) { + editor.run(postEditor => postEditor.toggleMarkup('em')); + } else { + document.execCommand('italic', false, null); + } } }, { modifier: MODIFIERS.CTRL, str: 'I', run(editor) { - editor.run(postEditor => postEditor.toggleMarkup('em')); + if (editor.cursor.hasSelection()) { + editor.run(postEditor => postEditor.toggleMarkup('em')); + } else { + document.execCommand('italic', false, null); + } + } +}, { + modifier: MODIFIERS.CTRL, + str: 'K', + run(editor) { + let range = editor.cursor.offsets; + if (!editor.cursor.hasSelection()) { + range.tail = new Position(range.head.section, range.head.section.length); + } + editor.run(postEditor => postEditor.deleteRange(range)); + editor.cursor.moveToPosition(range.head); } }, { modifier: MODIFIERS.META, str: 'K', run(editor) { - if (!editor.cursor.hasSelection()) { return; } + if (!editor.cursor.hasSelection()) { + return; + } let selectedText = editor.cursor.selectedText(); let defaultUrl = ''; diff --git a/src/js/parsers/post.js b/src/js/parsers/post.js index dbb412229..561e83ffa 100644 --- a/src/js/parsers/post.js +++ b/src/js/parsers/post.js @@ -9,6 +9,17 @@ import { forEach } from 'content-kit-editor/utils/array-utils'; import { getAttributes, walkTextNodes } from '../utils/dom-utils'; import Markup from 'content-kit-editor/models/markup'; +const TAG_REMAPPING = { + 'b': 'strong', + 'i': 'em' +}; + +function normalizeTagName(tagName) { + let normalized = tagName.toLowerCase(); + let remapped = TAG_REMAPPING[normalized]; + return remapped || normalized; +} + export default class PostParser { constructor(builder) { this.builder = builder; @@ -53,7 +64,7 @@ export default class PostParser { if (Markup.isValidElement(node)) { const tagName = node.tagName; const attributes = getAttributes(node); - return this.builder.createMarkup(tagName, attributes); + return this.builder.createMarkup(normalizeTagName(tagName), attributes); } } diff --git a/tests/acceptance/editor-key-commands-test.js b/tests/acceptance/editor-key-commands-test.js index 3d7578e38..86c547bc1 100644 --- a/tests/acceptance/editor-key-commands-test.js +++ b/tests/acceptance/editor-key-commands-test.js @@ -13,40 +13,144 @@ module('Acceptance: Editor: Key Commands', { $('#qunit-fixture').append(editorElement); }, afterEach() { - if (editor) { editor.destroy(); } + if (editor) { + editor.destroy(); + editor = null; + } } }); -test('typing command-B bolds highlighted text', (assert) => { +function testStatefulCommand({modifier, key, command, markupName}) { + test(`${command} applies markup ${markupName} to highlighted text`, (assert) => { + let initialText = 'something'; + const mobiledoc = Helpers.mobiledoc.build( + ({post, markupSection, marker}) => post([ + markupSection('p', [marker(initialText)]) + ])); + + editor = new Editor({mobiledoc}); + editor.render(editorElement); + + assert.hasNoElement(`#editor ${markupName}`, `precond - no ${markupName} text`); + Helpers.dom.selectText(initialText, editorElement); + Helpers.dom.triggerKeyCommand(editor, key, modifier); + + assert.hasElement(`#editor ${markupName}:contains(${initialText})`, + `text wrapped in ${markupName}`); + }); + + test(`${command} applies ${markupName} to next entered text`, (assert) => { + let initialText = 'something'; + const mobiledoc = Helpers.mobiledoc.build( + ({post, markupSection, marker}) => post([ + markupSection('p', [marker(initialText)]) + ])); + + editor = new Editor({mobiledoc}); + editor.render(editorElement); + + assert.hasNoElement(`#editor ${markupName}`, `precond - no ${markupName} text`); + Helpers.dom.moveCursorTo( + editor.post.sections.head.markers.head.renderNode.element, + initialText.length); + Helpers.dom.triggerKeyCommand(editor, key, modifier); + Helpers.dom.insertText(editor, 'z'); + + let changedMobiledoc = editor.serialize(); + let expectedMobiledoc = Helpers.mobiledoc.build( + ({post, markupSection, marker, markup: buildMarkup}) => { + let markup = buildMarkup(markupName); + return post([ + markupSection('p', [ + marker(initialText), + marker('z', [markup]) + ]) + ]); + }); + assert.deepEqual(changedMobiledoc, expectedMobiledoc); + }); +} + +testStatefulCommand({ + modifier: MODIFIERS.META, + key: 'B', + command: 'command-B', + markupName: 'strong' +}); + +testStatefulCommand({ + modifier: MODIFIERS.CTRL, + key: 'B', + command: 'command-B', + markupName: 'strong' +}); + +testStatefulCommand({ + modifier: MODIFIERS.META, + key: 'I', + command: 'command-I', + markupName: 'em' +}); + +testStatefulCommand({ + modifier: MODIFIERS.CTRL, + key: 'I', + command: 'command-I', + markupName: 'em' +}); + +test(`ctrl-k clears to the end of a line`, (assert) => { + let initialText = 'something'; const mobiledoc = Helpers.mobiledoc.build( ({post, markupSection, marker}) => post([ - markupSection('p', [marker('something')]) + markupSection('p', [marker(initialText)]) ])); editor = new Editor({mobiledoc}); editor.render(editorElement); - assert.hasNoElement('#editor strong', 'precond - no strong text'); - Helpers.dom.selectText('something', editorElement); - Helpers.dom.triggerKeyCommand(editor, 'B', MODIFIERS.META); - - assert.hasElement('#editor strong:contains(something)', 'text is strengthened'); + let textElement = editor.post.sections.head.markers.head.renderNode.element; + Helpers.dom.moveCursorTo(textElement, 4); + Helpers.dom.triggerKeyCommand(editor, 'K', MODIFIERS.CTRL); + + let changedMobiledoc = editor.serialize(); + let expectedMobiledoc = Helpers.mobiledoc.build( + ({post, markupSection, marker}) => { + return post([ + markupSection('p', [ + marker('some') + ]) + ]); + }); + assert.deepEqual(changedMobiledoc, expectedMobiledoc, + 'mobiledoc updated appropriately'); }); -test('typing command-I italicizes highlighted text', (assert) => { +test(`ctrl-k clears selected text`, (assert) => { + let initialText = 'something'; const mobiledoc = Helpers.mobiledoc.build( ({post, markupSection, marker}) => post([ - markupSection('p', [marker('something')]) + markupSection('p', [marker(initialText)]) ])); editor = new Editor({mobiledoc}); editor.render(editorElement); - assert.hasNoElement('#editor em', 'precond - no strong text'); - Helpers.dom.selectText('something', editorElement); - Helpers.dom.triggerKeyCommand(editor, 'I', MODIFIERS.META); - - assert.hasElement('#editor em:contains(something)', 'text is emphasized'); + let textElement = editor.post.sections.head.markers.head.renderNode.element; + Helpers.dom.moveCursorTo(textElement, 4, textElement, 8); + Helpers.dom.triggerKeyCommand(editor, 'K', MODIFIERS.CTRL); + + let changedMobiledoc = editor.serialize(); + let expectedMobiledoc = Helpers.mobiledoc.build( + ({post, markupSection, marker}) => { + return post([ + markupSection('p', [ + marker('someg') + ]) + ]); + }); + assert.deepEqual(changedMobiledoc, expectedMobiledoc, + 'mobiledoc updated appropriately'); }); test('new key commands can be registered', (assert) => { diff --git a/tests/acceptance/editor-sections-test.js b/tests/acceptance/editor-sections-test.js index 647d3a4f8..636dc6a27 100644 --- a/tests/acceptance/editor-sections-test.js +++ b/tests/acceptance/editor-sections-test.js @@ -88,14 +88,16 @@ const mobileDocWithNoCharacter = { module('Acceptance: Editor sections', { beforeEach() { - fixture = document.getElementById('qunit-fixture'); - editorElement = document.createElement('div'); - editorElement.setAttribute('id', 'editor'); - fixture.appendChild(editorElement); + fixture = $('#qunit-fixture'); + editorElement = $('
')[0]; + fixture.append(editorElement); }, afterEach() { - if (editor) { editor.destroy(); } + if (editor) { + editor.destroy(); + editor = null; + } } }); @@ -405,14 +407,14 @@ test('when selection incorrectly contains P end tag, editor reports correct sele headSectionOffset, tailSectionOffset, headMarkerOffset, tailMarkerOffset } = editor.cursor.offsets; - assert.equal(headSection, editor.post.sections.objectAt(0), - 'returns first section head'); - assert.equal(tailSection, editor.post.sections.objectAt(1), - 'returns second section tail'); - assert.equal(headMarker, editor.post.sections.objectAt(0).markers.head, - 'returns first section marker head'); - assert.equal(tailMarker, editor.post.sections.objectAt(1).markers.head, - 'returns second section marker tail'); + assert.ok(headSection === editor.post.sections.objectAt(0), + 'returns first section head'); + assert.ok(tailSection === editor.post.sections.objectAt(1), + 'returns second section tail'); + assert.ok(headMarker === editor.post.sections.objectAt(0).markers.head, + 'returns first section marker head'); + assert.ok(tailMarker === editor.post.sections.objectAt(1).markers.head, + 'returns second section marker tail'); assert.equal(headMarkerOffset, 0, 'headMarkerOffset correct'); assert.equal(tailMarkerOffset, 0, 'tailMarkerOffset correct'); assert.equal(headSectionOffset, 0, 'headSectionOffset correct');