Skip to content

Commit

Permalink
handle spaces semantically
Browse files Browse the repository at this point in the history
Refactor text expansion code to happen on keyDown of the `trigger`
rather than using setTimeout and reparse

Fixes #292
  • Loading branch information
bantic committed Jan 14, 2016
1 parent 819085b commit fb093f7
Show file tree
Hide file tree
Showing 7 changed files with 217 additions and 179 deletions.
55 changes: 30 additions & 25 deletions src/js/editor/editor.js
Expand Up @@ -37,7 +37,7 @@ import {
setClipboardCopyData
} from '../utils/paste-utils';
import { DIRECTION } from 'mobiledoc-kit/utils/key';
import { TAB } from 'mobiledoc-kit/utils/characters';
import { TAB, SPACE } from 'mobiledoc-kit/utils/characters';
import assert from '../utils/assert';

export const EDITOR_ELEMENT_CLASS_NAME = '__mobiledoc-editor';
Expand Down Expand Up @@ -97,14 +97,6 @@ class Editor {
this._parser = new DOMParser(this.builder);
this._renderer = new Renderer(this, this.cards, this.unknownCardHandler, this.cardOptions);

this._handleLastKeydownExpansion = () => {
// Chrome does not report cursor selection properly from a Mutation event
// until the next frame of JavaScript
setTimeout(() => {
this.handleExpansion(this._lastKeydownEvent);
}, 0);
};

this.post = this.loadPost();
this._renderTree = new RenderTree(this.post);
}
Expand Down Expand Up @@ -229,14 +221,6 @@ class Editor {
this.keyCommands.unshift(keyCommand);
}

handleExpansion(event) {
const expansion = findExpansion(this.expansions, event, this);
if (expansion) {
event.preventDefault();
expansion.run(this);
}
}

/**
* @param {KeyEvent} event optional
* @private
Expand Down Expand Up @@ -687,28 +671,49 @@ class Editor {
let { offsets: range } = this.cursor;
let { isCollapsed } = range;
let nextPosition = range.head;

if (this.handleExpansion(event)) {
event.preventDefault();
break;
}

let shouldPreventDefault = isCollapsed && range.head.section.isCardSection;
this.run(postEditor => {
if (!isCollapsed) {
nextPosition = postEditor.deleteRange(range);
}
if (key.isTab() && !range.head.section.isCardSection) {
nextPosition = postEditor.insertText(nextPosition, TAB);
let isMarkerable = range.head.section.isMarkerable;
if (isMarkerable &&
(key.isTab() || key.isSpace())
) {
let toInsert = key.isTab() ? TAB : SPACE;
shouldPreventDefault = true;
nextPosition = postEditor.insertText(nextPosition, toInsert);
}
if (nextPosition && nextPosition !== range.head) {
postEditor.setRange(new Range(nextPosition));
}
});
if (
(isCollapsed && range.head.section.isCardSection) ||
key.isTab()
) {
if (shouldPreventDefault) {
event.preventDefault();
}
break;
}
}

this._lastKeydownEvent = event;
this.addCallbackOnce(CALLBACK_QUEUES.DID_REPARSE, this._handleLastKeydownExpansion);
/**
* Finds and runs first matching text expansion for this event
* @param {Event} event keyboard event
* @return {Boolean} True when an expansion was found and run
* @private
*/
handleExpansion(keyEvent) {
let expansion = findExpansion(this.expansions, keyEvent, this);
if (expansion) {
expansion.run(this);
return true;
}
return false;
}

/**
Expand Down
33 changes: 18 additions & 15 deletions src/js/editor/text-expansions.js
Expand Up @@ -73,22 +73,25 @@ export const DEFAULT_TEXT_EXPANSIONS = [
];

export function findExpansion(expansions, keyEvent, editor) {
const key = Key.fromEvent(keyEvent);
let key = Key.fromEvent(keyEvent);
if (!key.isPrintable()) { return; }

const {head, head:{section, offset}} = editor.cursor.offsets;
let { range } = editor;
if (!range.isCollapsed) { return; }

let {head, head:{section}} = range;

if (head.isBlank) { return; }
if (!section.isMarkupSection) { return; }
if (!head.isEqual(section.tailPosition())) { return; }

// FIXME this is potentially expensive to calculate and might be better
// perf to first find expansions matching the trigger and only if matches
// are found then calculating the _text
const _text = section.textUntil(offset);
return detect(
expansions,
({trigger, text}) => {
return key.keyCode === trigger &&
_text === (text + String.fromCharCode(trigger));
}
);

let marker = head.marker;

// Only fire expansions at start of section
if (marker && marker.prev) { return; }

let _text = marker && marker.value;

return detect(expansions, ({trigger, text}) => {
return key.keyCode === trigger && _text === text;
});
}
22 changes: 18 additions & 4 deletions src/js/renderers/editor-dom.js
Expand Up @@ -28,22 +28,36 @@ function createElementFromMarkup(doc, markup) {
return element;
}

const TWO_SPACES = `${SPACE}${SPACE}`;
const SPACE_AND_NO_BREAK = `${SPACE}${NO_BREAK_SPACE}`;
const SPACES_REGEX = new RegExp(TWO_SPACES, 'g');
const TAB_REGEX = new RegExp(TAB, 'g');
const endsWithSpace = function(text) {
return endsWith(text, SPACE);
};
const startsWithSpace = function(text) {
return startsWith(text, SPACE);
};

// FIXME: This can be done more efficiently with a single pass
// building a correct string based on the original.
function renderHTMLText(marker) {
let text = marker.value;
text = text.replace(SPACES_REGEX, SPACE_AND_NO_BREAK)
.replace(TAB_REGEX, TAB_CHARACTER);

// If the first marker has a leading space or the last marker has a
// trailing space, the browser will collapse the space when we position
// the cursor.
// See https://github.com/bustlelabs/mobiledoc-kit/issues/68
// and https://github.com/bustlelabs/mobiledoc-kit/issues/75
if (!marker.next && endsWith(text, SPACE)) {
if (endsWithSpace(text) && !marker.next) {
text = text.substr(0, text.length - 1) + NO_BREAK_SPACE;
} else if ((!marker.prev || endsWith(marker.prev.value, SPACE)) && startsWith(text, SPACE)) {
}
if (startsWithSpace(text) &&
(!marker.prev || endsWithSpace(marker.prev.value))) {
text = NO_BREAK_SPACE + text.substr(1);
}
text = text.replace(/ ( )/g, ' '+NO_BREAK_SPACE);
text = text.replace(new RegExp(TAB, 'g'), TAB_CHARACTER);
return text;
}

Expand Down
1 change: 1 addition & 0 deletions src/js/utils/characters.js
@@ -1,2 +1,3 @@
export const TAB = '\t';
export const ENTER = '\n';
export const SPACE = ' ';
13 changes: 13 additions & 0 deletions tests/acceptance/editor-sections-test.js
Expand Up @@ -543,6 +543,19 @@ test('deleting when after deletion there is a leading space positions cursor at
});
});

test('inserting multiple spaces renders them with nbsps', (assert) => {
let mobiledoc = Helpers.mobiledoc.build(({post, markupSection}) => {
return post([markupSection()]);
});
editor = new Editor({mobiledoc});
editor.render(editorElement);

Helpers.dom.insertText(editor, ' ');
assert.equal($('#editor p:eq(0)').text(),
`${NO_BREAK_SPACE}${NO_BREAK_SPACE}${NO_BREAK_SPACE}`,
'correct nbsps in text');
});

test('deleting when the previous section is also blank', (assert) => {
editor = new Editor({mobiledoc: mobileDocWithNoCharacter});
editor.render(editorElement);
Expand Down

0 comments on commit fb093f7

Please sign in to comment.