fix(change-event-highlight): update model on remove and add highlight

fix: on setVisibleSelection don't set the focus again

It should work also with Firefox.
On Comments otherwise the focus is set on the element
instead on the comment writing

the reason is the spellcheck which use the retainVisibleCollection

Author: meinradandermatt

spec/highlighting.spec.js

@@ -11,11 +11,21 @@ function setupHighlightEnv (context, text) {
   context.$div = $('<div>' + context.text + '</div>').appendTo(document.body)
   context.editable = new Editable()
   context.editable.add(context.$div)
-  context.highlightRange = (highlightId, start, end) => {
+  context.highlightRange = (highlightId, start, end, dispatcher) => {
     return highlightSupport.highlightRange(
       context.$div[0],
       highlightId,
-      start, end
+      start,
+      end,
+      dispatcher
+    )
+  }
+
+  context.removeHighlight = (highlightId, dispatcher) => {
+    return highlightSupport.removeHighlight(
+      context.$div[0],
+      highlightId,
+      dispatcher
     )
   }
 
@@ -538,5 +548,22 @@ o Round</span>`)
       expect(this.getHtml()).toEqual(expectedHtml)
       expect(this.extract()).toEqual(expectedRanges)
     })
+
+    it('notify change on add highlight when dispatcher is given', () => {
+      let called = 0
+      const dispatcher = {notify: () => called++}
+      this.highlightRange('first', 0, 20, dispatcher)
+
+      expect(called).toEqual(1)
+    })
+
+    it('notify change on remove highlight when dispatcher is given', () => {
+      let called = 0
+      const dispatcher = {notify: () => called++}
+      this.highlightRange('first', 0, 20)
+      this.removeHighlight('first', dispatcher)
+
+      expect(called).toEqual(1)
+    })
   })
 })

src/core.js

@@ -356,9 +356,10 @@ const Editable = module.exports = class Editable {
    * @param  {Object} [options.textRange] An optional range which gets used to set the markers.
    * @param  {Number} options.textRange.start
    * @param  {Number} options.textRange.end
+   * @param  {Boolean} options.raiseEvents do throw change events
    * @return {Number} The text-based start offset of the newly applied highlight or `-1` if the range was considered invalid.
    */
-  highlight ({editableHost, text, highlightId, textRange}) {
+  highlight ({editableHost, text, highlightId, textRange, raiseEvents}) {
     if (!textRange) {
       return highlightSupport.highlightText(editableHost, text, highlightId)
     }
@@ -374,7 +375,7 @@ const Editable = module.exports = class Editable {
       )
       return -1
     }
-    return highlightSupport.highlightRange(editableHost, highlightId, textRange.start, textRange.end)
+    return highlightSupport.highlightRange(editableHost, highlightId, textRange.start, textRange.end, raiseEvents ? this.dispatcher : undefined)
   }
 
   /**
@@ -396,8 +397,8 @@ const Editable = module.exports = class Editable {
     )
   }
 
-  removeHighlight ({editableHost, highlightId}) {
-    highlightSupport.removeHighlight(editableHost, highlightId)
+  removeHighlight ({editableHost, highlightId, raiseEvents}) {
+    highlightSupport.removeHighlight(editableHost, highlightId, raiseEvents ? this.dispatcher : undefined)
   }
 
   decorateHighlight ({editableHost, highlightId, addCssClass, removeCssClass}) {

src/cursor.js

@@ -90,11 +90,8 @@ export default class Cursor {
   }
 
   setVisibleSelection () {
-    // Without setting focus() Firefox is not happy (seems setting a selection is not enough.
-    // Probably because Firefox can handle multiple selections).
     if (this.win.document.activeElement !== this.host) {
       const {x, y} = viewport.getScrollPosition(this.win)
-      $(this.host).focus()
       this.win.scrollTo(x, y)
     }
     rangy.getSelection(this.win).setSingleRange(this.range)

src/highlight-support.js

@@ -31,7 +31,7 @@ const highlightSupport = {
     }
   },
 
-  highlightRange (editableHost, highlightId, startIndex, endIndex) {
+  highlightRange (editableHost, highlightId, startIndex, endIndex, dispatcher) {
     if (this.hasHighlight(editableHost, highlightId)) {
       this.removeHighlight(editableHost, highlightId)
     }
@@ -52,6 +52,9 @@ const highlightSupport = {
     range.deleteContents()
     range.insertNode(marker)
     highlightSupport.cleanupStaleMarkerNodes(editableHost, 'comment')
+    if (dispatcher) {
+      dispatcher.notify('change', editableHost)
+    }
     return startIndex
   },
 
@@ -65,10 +68,13 @@ const highlightSupport = {
       })
   },
 
-  removeHighlight (editableHost, highlightId) {
+  removeHighlight (editableHost, highlightId, dispatcher) {
     $(editableHost).find(`[data-word-id="${highlightId}"]`)
       .each((index, elem) => {
         content.unwrap(elem)
+        if (dispatcher) {
+          dispatcher.notify('change', editableHost)
+        }
       })
   },