Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable event manager when editing is disabled #573

Merged
merged 1 commit into from
Aug 17, 2017

Conversation

bantic
Copy link
Collaborator

@bantic bantic commented Aug 17, 2017

Previously, manual (as opposed to programmatic) editing was being
prevented for an editor after calling #disableEditing by setting
"contenteditable" back to false for the editor's element. This prevents the
browser from sending most of the edit-related events (keyup, keydown,
etc) that occur when editing manually. But some events (like paste) are
still emitted by the browser even though "contenteditable" is false.

This change calls EventManager#stop when editing is disabled, and
internally the event manager ignores all events when it is stopped.

Also some rearrangement

Other changes

  • Change editor's internal isEditable prop to default to true rather
    than null (incidentally allows a user to specify that an editor
    should start disabled by passing in that prop to the constructor)
  • Add test for triggering paste event in a disabled editor
  • add assertion assert.isBlank
  • better assertion actual/expected values for hasElement and hasNoElement
  • add test helper getData (for parity with the setData in
    element-utils and to avoid using jQuery's .data() method)
  • consolidate the editing-disabled tests into a new suite
  • make editor#render call disableEditing or enableEditing after
    it sets hasRendered to ensure that the event manager is stopped if
    the user calls editor.disableEditing() before calling
    editor.render()

Fixes #572

@bantic bantic force-pushed the 572-disable-event-manager-when-editing-disabled branch from f5af1eb to 8320e0e Compare August 17, 2017 18:43
Previously, manual (as opposed to programmatic) editing was being
prevented for an editor after calling `#disableEditing` by setting
"contenteditable" back to false for the editor's element. This prevents the
browser from sending most of the edit-related events (keyup, keydown,
etc) that occur when editing manually. But some events (like paste) are
still emitted by the browser even though "contenteditable" is false.

This change calls `EventManager#stop` when editing is disabled, and
internally the event manager ignores all events when it is stopped.

Also some rearrangement

Other changes
  * Change editor's internal `isEditable` prop to default to true rather
    than null (incidentally allows a user to specify that an editor
    should start disabled by passing in that prop to the constructor)
  * Add test for triggering paste event in a disabled editor
  * add assertion `assert.isBlank`
  * better assertion actual/expected values for `hasElement` and `hasNoElement`
  * add test helper `getData` (for parity with the `setData` in
    element-utils and to avoid using jQuery's `.data()` method)
  * consolidate the editing-disabled tests into a new suite
  * make `editor#render` call `disableEditing` or `enableEditing` after
    it sets `hasRendered` to ensure that the event manager is stopped if
    the user calls `editor.disableEditing()` before calling
    `editor.render()`

Fixes #572
@bantic bantic force-pushed the 572-disable-event-manager-when-editing-disabled branch from 8320e0e to 43c6c8e Compare August 17, 2017 18:44
@bantic bantic merged commit 64c7f6c into master Aug 17, 2017
@bantic bantic deleted the 572-disable-event-manager-when-editing-disabled branch August 17, 2017 20:23
@bantic
Copy link
Collaborator Author

bantic commented Aug 17, 2017

released in v0.10.18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant