Skip to content

Commit

Permalink
[FEATURE] [BUGFIX] Refactor editor hooks
Browse files Browse the repository at this point in the history
  * Adds `postDidChange` hook -- called when post changes
  * [BUGFIX BREAKING] Changes `cursorDidChange` to only fire when the cursor
    changes (not when markup or section state changes)
  * [FEATURE] Add `inputModeDidChange` hook -- called when editor input
    mode (active markups or active sections) changes

Fixes #319.

**BREAKING CHANGE**: This is a potentially breaking change for consumers (such as
`ember-mobiledoc-editor`) that used the `cursorDidChange` hook to
maintain toolbar state.

Most of the time the editor's input mode (active
markups and active section tagNames) changes it is due to changing
cursor position/selection, so listening to the `cursorDidChange` is
often appropriate, but it is possible to change the editor's input mode
without changing cursor position (e.g., hitting "cmd-B" to bold text).
Previously, the `cursorDidChange` hook fired over-eagerly, resulting in
it firing in some cases (but not all) when the cursor did not change
(but editor input mode did). So it served fairly effectively for keeping the
toolbar's buttons in appropriate active/inactive state. This PR fixes
`cursorDidChange` so that it is only called when the cursor position
actually changes, making it even less effective as a hook to use to
infer the editor's active markups and sections (now *no* input mode change
that doesn't change the cursor will fire `cursorDidChange`).

This PR introduces a new `inputModeDidChange` hook that only fires on
input mode changes. This cleanly separates the ability to listen for cursor changes
from listening for input mode changes. It is also more efficient for a consumer to use as a
hook (because most cursor changes do not change input mode at all, so
listening to that hook as a proxy for detecting input mode changes creates unnecessary work).

Also:

  * Test that removing a card or saving a new payload for it triggers
    `postDidChange` hook. Transitioning to `edit` or calling
    `env.cancel` do not trigger the hook.
  * Deprecates `on('update')` hook
  * Add private `Editor#_notifyRangeChange` to use to alert the editor
    of possible cursor- or state-changing activity (e.g., keyup or mousedown,
    because they might have moved the cursor).
  * Test that `cursorDidChange` is not fired after the editor is
    destroyed
  * Refactor internals of `EditState` to better track changes to
    state/cursor.
  • Loading branch information
bantic committed Apr 12, 2016
1 parent 97140e9 commit de52092
Show file tree
Hide file tree
Showing 20 changed files with 676 additions and 335 deletions.
118 changes: 88 additions & 30 deletions src/js/editor/edit-state.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,69 @@
import { contains, isArrayEqual } from 'mobiledoc-kit/utils/array-utils';

export default class EditState {
/**
* Used by {@link Editor} to manage its current state (cursor, active markups
* and active sections).
* @private
*/
class EditState {
constructor(editor) {
this.editor = editor;
this._activeMarkups = [];
this._activeSections = [];
this.prevState = this.state = this._readState();
}

get activeSections() {
let { editor: { range, post } } = this;
if (range.isBlank) {
return [];
} else {
return post.sections.readRange(range.head.section, range.tail.section);
}
/**
* Cache the last state, force a reread of current state
*/
reset() {
this.prevState = this.state;
this.state = this._readState();
}

get activeMarkups() {
let { editor: { cursor, post, range } } = this;
/**
* @return {Boolean} Whether the input mode (active markups or active section tag names)
* has changed.
*/
inputModeDidChange() {
let { state, prevState } = this;
return (!isArrayEqual(state.activeMarkups, prevState.activeMarkups) ||
!isArrayEqual(state.activeSectionTagNames, prevState.activeSectionTagNames));
}

if (!cursor.hasCursor()) {
return [];
} else if (!this._activeMarkups) {
this._activeMarkups = post.markupsInRange(range);
}
/**
* @return {Boolean} Whether the range has changed.
*/
rangeDidChange() {
let { state, prevState } = this;
return !state.range.isEqual(prevState.range);
}

/**
* @return {Range}
*/
get range() {
return this.state.range;
}

/**
* @return {Section[]}
*/
get activeSections() {
return this.state.activeSections;
}

return this._activeMarkups;
/**
* @return {Markup[]}
*/
get activeMarkups() {
return this.state.activeMarkups;
}

/**
* Update the editor's markup state. This is used when, e.g.,
* a user types meta+B when the editor has a cursor but no selected text;
* in this case the editor needs to track that it has an active "b" markup
* and apply it to the next text the user types.
*/
toggleMarkupState(markup) {
if (contains(this.activeMarkups, markup)) {
this._removeActiveMarkup(markup);
Expand All @@ -36,24 +72,46 @@ export default class EditState {
}
}

/**
* @return {Boolean} Whether the markups after reset have changed
*/
resetActiveMarkups() {
let prevMarkups = this._activeMarkups || [];
delete this._activeMarkups;
let markups = this.activeMarkups || [];
_readState() {
let range = this._readRange();
let state = {
range: range,
activeMarkups: this._readActiveMarkups(range),
activeSections: this._readActiveSections(range)
};
// Section objects are 'live', so to check that they changed, we
// need to map their tagNames now (and compare to mapped tagNames later).
state.activeSectionTagNames = state.activeSections.map(s => s.tagName);
return state;
}

_readRange() {
return this.editor.range;
}

_readActiveSections(range) {
let { head, tail } = range;
let { editor: { post } } = this;
if (range.isBlank) {
return [];
} else {
return post.sections.readRange(head.section, tail.section);
}
}

let didChange = !isArrayEqual(prevMarkups, markups);
return didChange;
_readActiveMarkups(range) {
let { editor: { post } } = this;
return post.markupsInRange(range);
}

_removeActiveMarkup(markup) {
let index = this._activeMarkups.indexOf(markup);
this._activeMarkups.splice(index, 1);
let index = this.state.activeMarkups.indexOf(markup);
this.state.activeMarkups.splice(index, 1);
}

_addActiveMarkup(markup) {
this._activeMarkups.push(markup);
this.state.activeMarkups.push(markup);
}
}

export default EditState;

0 comments on commit de52092

Please sign in to comment.