Skip to content

Commit

Permalink
fix: Remove backwards compatibility for missing input events
Browse files Browse the repository at this point in the history 
The logic causes issues as it loses the first change event.
  • Loading branch information
@marcbachmann
marcbachmann committed Jul 22, 2024
1 parent ca44307 commit 8567558
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 82 deletions.
71 changes: 30 additions & 41 deletions spec/dispatcher.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {Editable} from '../src/core.js'
import Selection from '../src/selection.js'
const {key} = Keyboard

describe('Dispatcher', function () {
describe('Dispatcher:', function () {
let editable, elem

// create a Cursor object and set the selection to it
Expand Down Expand Up @@ -55,7 +55,7 @@ describe('Dispatcher', function () {
return obj
}

describe('for editable', function () {
describe('for editable:', function () {

beforeEach(function () {
elem = document.createElement('div')
Expand All @@ -71,7 +71,7 @@ describe('Dispatcher', function () {
editable.unload()
})

describe('on focus', function () {
describe('on focus:', function () {
it('should trigger the focus event', function () {
elem.blur()
const focus = on('focus', function (element) {
Expand Down Expand Up @@ -103,7 +103,7 @@ describe('Dispatcher', function () {
})
})

describe('on enter', function () {
describe('on enter:', function () {

it('fires insert "after" if cursor is at the end', function () {
// <div>foo\</div>
Expand Down Expand Up @@ -162,7 +162,7 @@ describe('Dispatcher', function () {
})
})

describe('on backspace', function () {
describe('on backspace:', function () {

it('fires "merge" if cursor is at the beginning', function (done) {
elem.innerHTML = 'foo'
Expand All @@ -175,21 +175,9 @@ describe('Dispatcher', function () {

elem.dispatchEvent(new KeyboardEvent('keydown', {keyCode: key.backspace}))
})

it('fires "change" if cursor is not at the beginning', (done) => {
elem.innerHTML = 'foo'
createCursor(createRangeAtEnd(elem))

on('change', (element) => {
expect(element).to.equal(elem)
done()
})

elem.dispatchEvent(new KeyboardEvent('keydown', {keyCode: key.backspace}))
})
})

describe('on delete', function () {
describe('on delete:', function () {

it('fires "merge" if cursor is at the end', (done) => {
elem.innerHTML = 'foo'
Expand All @@ -202,25 +190,9 @@ describe('Dispatcher', function () {

elem.dispatchEvent(new KeyboardEvent('keydown', {keyCode: key.delete}))
})

it('fires "change" if cursor is at the beginning', (done) => {
elem.innerHTML = 'foo'
createCursor(createRangeAtBeginning(elem))
on('change', () => done())
elem.dispatchEvent(new KeyboardEvent('keydown', {keyCode: key.delete}))
})
})

describe('on keydown', function () {

it('fires change when a character is pressed', (done) => {
const evt = new KeyboardEvent('keydown', {keyCode: 'e'.charCodeAt(0)})
on('change', () => done())
elem.dispatchEvent(evt)
})
})

describe('on newline', function () {
describe('on newline:', function () {

function typeKeys (element, chars) {
const selection = window.getSelection()
Expand All @@ -245,25 +217,28 @@ describe('Dispatcher', function () {
expect(elem.innerHTML).to.equal('<br>\uFEFF')
})

it('appends a zero-width space after the br tag to force a line break', () => {
it('appends a zero-width space after the br tag to force a line break', async () => {
typeKeys(elem, 'foobar')
shiftReturn(elem)
elem.addEventListener('input', function (e) { console.log('input event', e) })
await 1
expect(elem.innerHTML).to.equal(
`\uFEFFfoobar<br>\uFEFF`
)
})

it('does not append another zero-width space when one is present already', () => {
it('does not append another zero-width space when one is present already', async () => {
typeKeys(elem, 'foobar')
shiftReturn(elem)
shiftReturn(elem)
await 1
expect(elem.innerHTML).to.equal(
`\uFEFFfoobar<br><br>\uFEFF`
)
})
})

describe('on bold', function () {
describe('on bold:', function () {

it('fires toggleBold when ctrl + b is pressed', (done) => {
elem.innerHTML = 'foo'
Expand All @@ -280,7 +255,7 @@ describe('Dispatcher', function () {
})
})

describe('on italic', function () {
describe('on italic:', function () {

it('fires toggleEmphasis when ctrl + i is pressed', (done) => {
elem.innerHTML = 'foo'
Expand All @@ -297,7 +272,7 @@ describe('Dispatcher', function () {
})
})

describe('selectToBoundary event:', function () {
describe('selectToBoundary event::', function () {

it('fires "both" if all is selected', function () {
elem.innerHTML = 'People Make The World Go Round'
Expand Down Expand Up @@ -353,7 +328,7 @@ describe('Dispatcher', function () {
})
})

describe('on paste', function () {
describe('on paste:', function () {

it('inserts plain text clipboard content', (done) => {
on('paste', (block, blocks) => {
Expand Down Expand Up @@ -408,5 +383,19 @@ describe('Dispatcher', function () {
elem.dispatchEvent(evt)
})
})

describe('input event:', function () {
it('fires "change" event', (done) => {
elem.innerHTML = 'foo'
createCursor(createRangeAtEnd(elem))

on('change', (element) => {
expect(element).to.equal(elem)
done()
})

elem.dispatchEvent(new Event('input', {bubbles: true}))
})
})
})
})
47 changes: 6 additions & 41 deletions src/dispatcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,6 @@ import Keyboard from './keyboard.js'
import {closest} from './util/dom.js'
import {replaceLast, endsWithSingleSpace} from './util/string.js'

// This will be set to true once we detect the input event is working.
// Input event description on MDN:
// https://developer.mozilla.org/en-US/docs/Web/Reference/Events/input
let isInputEventSupported = false

/**
* The Dispatcher module is responsible for dealing with events and their handlers.
*
Expand All @@ -23,7 +18,6 @@ export default class Dispatcher {
constructor (editable) {
const win = editable.win
eventable(this, editable)
this.supportsInputEvent = false
this.document = win.document
this.config = editable.config
this.editable = editable
Expand Down Expand Up @@ -122,7 +116,6 @@ export default class Dispatcher {
const selection = this.selectionWatcher.getFreshSelection()
if (selection && selection.isSelection) {
this.notify('clipboard', block, 'cut', selection)
this.triggerChangeEvent(block)
}
})
.setupDocumentListener('paste', function pasteListener (evt) {
Expand Down Expand Up @@ -151,14 +144,7 @@ export default class Dispatcher {
.setupDocumentListener('input', function inputListener (evt) {
const block = this.getEditableBlockByEvent(evt)
if (!block) return
if (isInputEventSupported) {
this.notify('change', block)
} else {
// Most likely the event was already handled manually by
// triggerChangeEvent so the first time we just switch the
// isInputEventSupported flag without notifying the change event.
isInputEventSupported = true
}
this.notify('change', block)
})

.setupDocumentListener('formatEditable', function formatEditableListener (evt) {
Expand All @@ -168,26 +154,6 @@ export default class Dispatcher {
})
}

/**
* Trigger a change event
*
* This should be done in these cases:
* - typing a letter
* - delete (backspace and delete keys)
* - cut
* - paste
* - copy and paste (not easily possible manually as far as I know)
*
* Preferably this is done using the input event. But the input event is not
* supported on all browsers for contenteditable elements.
* To make things worse it is not detectable either. So instead of detecting
* we set 'isInputEventSupported' when the input event fires the first time.
*/
triggerChangeEvent (target) {
if (isInputEventSupported) return
this.notify('change', target)
}

dispatchSwitchEvent (event, element, direction) {
if (event.altKey || event.ctrlKey || event.metaKey || event.shiftKey) return
const cursor = this.selectionWatcher.getFreshSelection()
Expand Down Expand Up @@ -228,8 +194,7 @@ export default class Dispatcher {
this.setupDocumentListener('keydown', function (evt) {
const block = this.getEditableBlockByEvent(evt)
if (!block) return
const notifyCharacterEvent = !isInputEventSupported
this.keyboard.dispatchKeyEvent(evt, block, notifyCharacterEvent)
this.keyboard.dispatchKeyEvent(evt, block, false)
}, true)
}

Expand All @@ -253,10 +218,10 @@ export default class Dispatcher {

.on('backspace', function (event) {
const rangeContainer = self.selectionWatcher.getFreshRange()
if (!rangeContainer.isCursor) return self.triggerChangeEvent(this)
if (!rangeContainer.isCursor) return

const cursor = rangeContainer.getCursor()
if (!cursor.isAtBeginning()) return self.triggerChangeEvent(this)
if (!cursor.isAtBeginning()) return

event.preventDefault()
event.stopPropagation()
Expand All @@ -265,10 +230,10 @@ export default class Dispatcher {

.on('delete', function (event) {
const rangeContainer = self.selectionWatcher.getFreshRange()
if (!rangeContainer.isCursor) return self.triggerChangeEvent(this)
if (!rangeContainer.isCursor) return

const cursor = rangeContainer.getCursor()
if (!cursor.isAtTextEnd()) return self.triggerChangeEvent(this)
if (!cursor.isAtTextEnd()) return

event.preventDefault()
event.stopPropagation()
Expand Down

0 comments on commit 8567558

Please sign in to comment.