diff --git a/src/js/commands/bold.js b/src/js/commands/bold.js index 3ee143921..9b3756666 100644 --- a/src/js/commands/bold.js +++ b/src/js/commands/bold.js @@ -1,36 +1,25 @@ import TextFormatCommand from './text-format'; -import { inherit } from 'content-kit-utils'; import Markup from '../models/markup'; import { any } from '../utils/array-utils'; -function BoldCommand(editor) { - TextFormatCommand.call(this, { - name: 'bold', - tag: 'strong', - mappedTags: ['b'], - button: '' - }); - this.editor = editor; +export default class BoldCommand extends TextFormatCommand { + constructor(editor) { + super({ + name: 'bold', + button: '' + }); + this.editor = editor; + this.markup = Markup.create('b'); + } + exec() { + this.editor.applyMarkupToSelection(this.markup); + } + unexec() { + this.editor.removeMarkupFromSelection(this.markup); + } + isActive() { + return any(this.editor.activeMarkers, m => m.hasMarkup(this.markup)); + } } -inherit(BoldCommand, TextFormatCommand); - -BoldCommand.prototype.exec = function() { - const markup = Markup.ofType('b'); - this.editor.applyMarkupToSelection(markup); -}; - -BoldCommand.prototype.isActive = function() { - let val = any(this.editor.activeMarkers, m => { - return any(this.mappedTags, tag => m.hasMarkup(tag)); - }); - return val; -}; - -BoldCommand.prototype.unexec = function() { - const markup = Markup.ofType('b'); - this.editor.removeMarkupFromSelection(markup); -}; - -export default BoldCommand; diff --git a/src/js/commands/text-format.js b/src/js/commands/text-format.js index a0f465c3f..df3bc168b 100644 --- a/src/js/commands/text-format.js +++ b/src/js/commands/text-format.js @@ -6,7 +6,9 @@ export default class TextFormatCommand extends Command { this.tag = options.tag; this.mappedTags = options.mappedTags || []; - this.mappedTags.push(this.tag); + if (this.tag) { + this.mappedTags.push(this.tag); + } this.action = options.action || this.name; this.removeAction = options.removeAction || this.action; } diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js index fd27193da..d53a6d1d0 100644 --- a/src/js/editor/editor.js +++ b/src/js/editor/editor.js @@ -537,11 +537,7 @@ class Editor { } removeMarkupFromSelection(markup) { - const markers = this.activeMarkers; - // FIXME-NEXT Now we need to ensure we are using the singleton - // markup for the 'B' tag - // in order to get http://localhost:4200/tests/?testId=8cb07cab - // to pass + const markers = this.splitMarkersFromSelection(); markers.forEach(marker => { marker.removeMarkup(markup); marker.section.renderNode.markDirty(); diff --git a/src/js/models/marker.js b/src/js/models/marker.js index 80824c1b3..70578c67f 100644 --- a/src/js/models/marker.js +++ b/src/js/models/marker.js @@ -1,5 +1,8 @@ export const MARKER_TYPE = 'marker'; +import { + normalizeTagName +} from '../utils/dom-utils'; import { detect } from 'content-kit-editor/utils/array-utils'; const Marker = class Marker { @@ -55,9 +58,14 @@ const Marker = class Marker { this.value = left + right; } - hasMarkup(tagName) { - tagName = tagName.toLowerCase(); - return detect(this.markups, markup => markup.tagName === tagName); + hasMarkup(tagNameOrMarkup) { + if (typeof tagNameOrMarkup === 'string') { + let tagName = normalizeTagName(tagNameOrMarkup); + return detect(this.markups, markup => markup.tagName === tagName); + } else { + let targetMarkup = tagNameOrMarkup; + return detect(this.markups, markup => markup === targetMarkup); + } } getMarkup(tagName) { diff --git a/src/js/models/markup.js b/src/js/models/markup.js index 41ef233b2..921a245a5 100644 --- a/src/js/models/markup.js +++ b/src/js/models/markup.js @@ -11,14 +11,14 @@ export const VALID_MARKUP_TAGNAMES = [ 'li' ].map(normalizeTagName); -let markupsOfType = {}; +const markupMap = {}; class Markup { /* * @param {attributes} array flat array of key1,value1,key2,value2,... */ constructor(tagName, attributes=[]) { - this.tagName = tagName.toLowerCase(); + this.tagName = normalizeTagName(tagName); this.attributes = attributes; this.type = MARKUP_TYPE; @@ -27,17 +27,24 @@ class Markup { } } - static ofType(tagName) { + // Use `create` to make a new markup so that we can use singletons in the + // markupMap + static create(tagName, attributes=[]) { tagName = normalizeTagName(tagName); - if (!markupsOfType[tagName]) { - markupsOfType[tagName] = new Markup(tagName); - } - return markupsOfType[tagName]; + if (attributes.length === 0) { + if (!markupMap[tagName]) { + markupMap[tagName] = new Markup(tagName); + } + + return markupMap[tagName]; + } else { + return new Markup(tagName, attributes); + } } static isValidElement(element) { - let tagName = element.tagName.toLowerCase(); + let tagName = normalizeTagName(element.tagName); return VALID_MARKUP_TAGNAMES.indexOf(tagName) !== -1; } } diff --git a/src/js/parsers/dom.js b/src/js/parsers/dom.js index 6e2626779..c93ce27c3 100644 --- a/src/js/parsers/dom.js +++ b/src/js/parsers/dom.js @@ -2,6 +2,7 @@ import { generateBuilder } from '../utils/post-builder'; import { trim } from 'content-kit-utils'; import { VALID_MARKUP_SECTION_TAGNAMES } from '../models/markup-section'; import { VALID_MARKUP_TAGNAMES } from '../models/markup'; +import { normalizeTagName } from '../utils/dom-utils'; const ELEMENT_NODE = 1; const TEXT_NODE = 3; @@ -58,7 +59,8 @@ function readAttributes(node) { } function isValidMarkerElement(element) { - return VALID_MARKUP_TAGNAMES.indexOf(element.tagName.toLowerCase()) !== -1; + let tagName = normalizeTagName(element.tagName); + return VALID_MARKUP_TAGNAMES.indexOf(tagName) !== -1; } function parseMarkers(section, postBuilder, topNode) { @@ -128,9 +130,9 @@ NewHTMLParser.prototype = { var section; switch(sectionElement.nodeType) { case ELEMENT_NODE: - var tagName = sectionElement.tagName; + let tagName = normalizeTagName(sectionElement.tagName); //

, etc - if (VALID_MARKUP_SECTION_TAGNAMES.indexOf(tagName.toLowerCase()) !== -1) { + if (VALID_MARKUP_SECTION_TAGNAMES.indexOf(tagName) !== -1) { section = postBuilder.generateMarkupSection(tagName, readAttributes(sectionElement)); var node = sectionElement.firstChild; while (node) { diff --git a/src/js/parsers/section.js b/src/js/parsers/section.js index 1e36e6bb8..cc449db69 100644 --- a/src/js/parsers/section.js +++ b/src/js/parsers/section.js @@ -10,7 +10,10 @@ import { import Marker from 'content-kit-editor/models/marker'; import Markup from 'content-kit-editor/models/markup'; import { VALID_MARKUP_TAGNAMES } from 'content-kit-editor/models/markup'; -import { getAttributes } from 'content-kit-editor/utils/dom-utils'; +import { + getAttributes, + normalizeTagName +} from 'content-kit-editor/utils/dom-utils'; import { forEach } from 'content-kit-editor/utils/array-utils'; import { generateBuilder } from 'content-kit-editor/utils/post-builder'; @@ -88,19 +91,19 @@ export default { isSectionElement(element) { return element.nodeType === ELEMENT_NODE && - VALID_MARKUP_SECTION_TAGNAMES.indexOf(element.tagName.toLowerCase()) !== -1; + VALID_MARKUP_SECTION_TAGNAMES.indexOf(normalizeTagName(element.tagName)) !== -1; }, markupFromElement(element) { - const tagName = element.tagName.toLowerCase(); + const tagName = normalizeTagName(element.tagName); if (VALID_MARKUP_TAGNAMES.indexOf(tagName) === -1) { return null; } - return new Markup(tagName, getAttributes(element)); + return Markup.create(tagName, getAttributes(element)); }, sectionTagNameFromElement(element) { let tagName = element.tagName; - tagName = tagName && tagName.toLowerCase(); + tagName = tagName && normalizeTagName(tagName); if (VALID_MARKUP_SECTION_TAGNAMES.indexOf(tagName) === -1) { tagName = DEFAULT_TAG_NAME; } return tagName; } diff --git a/src/js/utils/element-utils.js b/src/js/utils/element-utils.js index 6f491add3..c1f4d8c3e 100644 --- a/src/js/utils/element-utils.js +++ b/src/js/utils/element-utils.js @@ -1,5 +1,8 @@ import { UNPRINTABLE_CHARACTER } from 'content-kit-editor/renderers/editor-dom'; import { dasherize } from 'content-kit-editor/utils/string-utils'; +import { + normalizeTagName +} from 'content-kit-editor/utils/dom-utils'; function createDiv(className) { var div = document.createElement('div'); @@ -22,10 +25,11 @@ function swapElements(elementToShow, elementToHide) { showElement(elementToShow); } -function getEventTargetMatchingTag(tag, target, container) { +function getEventTargetMatchingTag(tagName, target, container) { + tagName = normalizeTagName(tagName); // Traverses up DOM from an event target to find the node matching specifed tag while (target && target !== container) { - if (target.tagName.toLowerCase() === tag) { + if (normalizeTagName(target.tagName) === tagName) { return target; } target = target.parentNode; diff --git a/src/js/utils/post-builder.js b/src/js/utils/post-builder.js index 83344f28e..0b98f4dfe 100644 --- a/src/js/utils/post-builder.js +++ b/src/js/utils/post-builder.js @@ -35,11 +35,11 @@ var builder = { generateMarkup(tagName, attributes) { if (attributes) { // FIXME: This could also be cached - return new Markup(tagName, attributes); + return Markup.create(tagName, attributes); } var markerType = this._markerTypeCache[tagName]; if (!markerType) { - this._markerTypeCache[tagName] = markerType = new Markup(tagName); + this._markerTypeCache[tagName] = markerType = Markup.create(tagName); } return markerType; } diff --git a/src/js/utils/selection-utils.js b/src/js/utils/selection-utils.js index b549df211..f9b8248ec 100644 --- a/src/js/utils/selection-utils.js +++ b/src/js/utils/selection-utils.js @@ -1,4 +1,7 @@ -import { containsNode } from './dom-utils'; +import { + containsNode, + normalizeTagName +} from './dom-utils'; // TODO: remove, pass in Editor's current block set var RootTags = [ @@ -54,25 +57,25 @@ function isSelectionInElement(element) { function getSelectionBlockElement(selection) { selection = selection || window.getSelection(); var element = getSelectionElement(); - var tag = element && element.tagName.toLowerCase(); + let tag = element && normalizeTagName(element.tagName); while (tag && RootTags.indexOf(tag) === -1) { if (element.contentEditable === 'true') { return null; // Stop traversing up dom when hitting an editor element } element = element.parentNode; - tag = element.tagName && element.tagName.toLowerCase(); + tag = element.tagName && normalizeTagName(element.tagName); } return element; } function getSelectionTagName() { var element = getSelectionElement(); - return element ? element.tagName.toLowerCase() : null; + return element ? normalizeTagName(element.tagName) : null; } function getSelectionBlockTagName() { var element = getSelectionBlockElement(); - return element ? element.tagName && element.tagName.toLowerCase() : null; + return element ? element.tagName && normalizeTagName(element.tagName) : null; } function tagsInSelection(selection) { @@ -81,7 +84,7 @@ function tagsInSelection(selection) { while(element) { if (element.contentEditable === 'true') { break; } // Stop traversing up dom when hitting an editor element if (element.tagName) { - tags.push(element.tagName.toLowerCase()); + tags.push(normalizeTagName(element.tagName)); } element = element.parentNode; } diff --git a/tests/acceptance/editor-commands-test.js b/tests/acceptance/editor-commands-test.js index 150e0c580..02cb72b8f 100644 --- a/tests/acceptance/editor-commands-test.js +++ b/tests/acceptance/editor-commands-test.js @@ -276,5 +276,32 @@ test('click bold button applies bold to selected text', (assert) => { }); }); +test('can unbold part of a larger set of bold text', (assert) => { + const done = assert.async(); + + setTimeout(() => { + assertInactiveToolbarButton(assert, 'bold', 'precond - bold button is not active'); + clickToolbarButton(assert, 'bold'); + assertActiveToolbarButton(assert, 'bold'); + + assert.hasElement('#editor b:contains(IS A)'); + + Helpers.dom.selectText('S A', editorElement); + Helpers.dom.triggerEvent(document, 'mouseup'); + + setTimeout(() => { + assertToolbarVisible(assert); + assertActiveToolbarButton(assert, 'bold'); + clickToolbarButton(assert, 'bold'); + + assert.hasElement('#editor b:contains(I)', 'unselected text is bold'); + assert.hasNoElement('#editor b:contains(IS A)', 'unselected text is bold'); + assert.hasElement('#editor p:contains(S A)', 'unselected text is bold'); + + done(); + }); + }); +}); + // test selecting across markers and boldening // test selecting across markers in sections and bolding diff --git a/tests/unit/models/marker-test.js b/tests/unit/models/marker-test.js index f192dc9a6..33d9b6cc6 100644 --- a/tests/unit/models/marker-test.js +++ b/tests/unit/models/marker-test.js @@ -29,22 +29,22 @@ test('a marker can truncated to an offset', (assert) => { test('a marker can have a markup applied to it', (assert) => { const m1 = new Marker('hi there!'); - m1.addMarkup(new Markup('b')); + m1.addMarkup(Markup.create('b')); assert.ok(m1.hasMarkup('b')); }); test('a marker can have the same markup tagName applied twice', (assert) => { const m1 = new Marker('hi there!'); - m1.addMarkup(new Markup('b')); - m1.addMarkup(new Markup('b')); + m1.addMarkup(Markup.create('b')); + m1.addMarkup(Markup.create('b')); assert.equal(m1.markups.length, 2, 'markup only applied once'); }); test('a marker can have a complex markup applied to it', (assert) => { const m1 = new Marker('hi there!'); - const markup = new Markup('a', {href:'blah'}); + const markup = Markup.create('a', {href:'blah'}); m1.addMarkup(markup); assert.ok(m1.hasMarkup('a')); @@ -53,8 +53,8 @@ test('a marker can have a complex markup applied to it', (assert) => { test('a marker can have the same complex markup tagName applied twice, even with different attributes', (assert) => { const m1 = new Marker('hi there!'); - const markup1 = new Markup('a', {href:'blah'}); - const markup2 = new Markup('a', {href:'blah2'}); + const markup1 = Markup.create('a', {href:'blah'}); + const markup2 = Markup.create('a', {href:'blah2'}); m1.addMarkup(markup1); m1.addMarkup(markup2); @@ -65,9 +65,9 @@ test('a marker can have the same complex markup tagName applied twice, even with test('a marker can be joined to another', (assert) => { const m1 = new Marker('hi'); - m1.addMarkup(new Markup('b')); + m1.addMarkup(Markup.create('b')); const m2 = new Marker(' there!'); - m2.addMarkup(new Markup('i')); + m2.addMarkup(Markup.create('i')); const m3 = m1.join(m2); assert.equal(m3.value, 'hi there!'); @@ -77,7 +77,7 @@ test('a marker can be joined to another', (assert) => { test('#split splits a marker in 2 when no endOffset is passed', (assert) => { const m1 = new Marker('hi there!'); - m1.addMarkup(new Markup('b')); + m1.addMarkup(Markup.create('b')); const [_m1, m2] = m1.split(5); assert.ok(_m1.hasMarkup('b') && m2.hasMarkup('b'), @@ -89,7 +89,7 @@ test('#split splits a marker in 2 when no endOffset is passed', (assert) => { test('#split splits a marker in 3 when endOffset is passed', (assert) => { const m = new Marker('hi there!'); - m.addMarkup(new Markup('b')); + m.addMarkup(Markup.create('b')); const newMarkers = m.split(2, 4); diff --git a/tests/unit/models/markup-section-test.js b/tests/unit/models/markup-section-test.js index 03981973b..1c46ecc9f 100644 --- a/tests/unit/models/markup-section-test.js +++ b/tests/unit/models/markup-section-test.js @@ -87,7 +87,7 @@ test('#markerContaining finds the marker at the given offset when multiple marke }); test('a section can be split, splitting its markers', (assert) => { - const m = new Marker('hi there!', [new Markup('b')]); + const m = new Marker('hi there!', [Markup.create('b')]); const s = new Section('p', [m]); const [s1, s2] = s.split(5);