Commit 64740e76 authored by Robert Knight's avatar Robert Knight

Fix adder breaking after making a selection containing no text

Ignore the selection change if the new selection does not contain
any text.

 - Add missing check for rangeUtil.selectionFocusRect() returning null,
   which happens if the selection contains no text

 - Add tests for selection change handling

 - Create the `Guest` instance with dependencies stubbed out afresh
   for each test case in guest-test.js

Fixes #3523
parent 0a76622c
...@@ -350,6 +350,14 @@ module.exports = class Guest extends Annotator ...@@ -350,6 +350,14 @@ module.exports = class Guest extends Annotator
@crossframe?.call('focusAnnotations', tags) @crossframe?.call('focusAnnotations', tags)
_onSelection: (range) -> _onSelection: (range) ->
selection = Annotator.Util.getGlobal().getSelection()
isBackwards = rangeUtil.isSelectionBackwards(selection)
focusRect = rangeUtil.selectionFocusRect(selection)
if !focusRect
# The selected range does not contain any text
this._onClearSelection()
return
@selectedRanges = [range] @selectedRanges = [range]
Annotator.$('.annotator-toolbar .h-icon-note') Annotator.$('.annotator-toolbar .h-icon-note')
...@@ -357,9 +365,6 @@ module.exports = class Guest extends Annotator ...@@ -357,9 +365,6 @@ module.exports = class Guest extends Annotator
.removeClass('h-icon-note') .removeClass('h-icon-note')
.addClass('h-icon-annotate'); .addClass('h-icon-annotate');
selection = Annotator.Util.getGlobal().getSelection()
isBackwards = rangeUtil.isSelectionBackwards(selection)
focusRect = rangeUtil.selectionFocusRect(selection)
{left, top, arrowDirection} = this.adderCtrl.target(focusRect, isBackwards) {left, top, arrowDirection} = this.adderCtrl.target(focusRect, isBackwards)
this.adderCtrl.showAt(left, top, arrowDirection) this.adderCtrl.showAt(left, top, arrowDirection)
......
Annotator = require('annotator') Annotator = require('annotator')
proxyquire = require('proxyquire')
adder = require('../adder')
Observable = require('../../util/observable').Observable
$ = Annotator.$ $ = Annotator.$
Annotator['@noCallThru'] = true; Annotator['@noCallThru'] = true;
highlighter = {} Guest = null
anchoring = {} anchoring = {}
highlighter = {}
rangeUtil = null
selections = null
raf = sinon.stub().yields() raf = sinon.stub().yields()
raf['@noCallThru'] = true raf['@noCallThru'] = true
...@@ -11,14 +19,15 @@ raf['@noCallThru'] = true ...@@ -11,14 +19,15 @@ raf['@noCallThru'] = true
scrollIntoView = sinon.stub() scrollIntoView = sinon.stub()
scrollIntoView['@noCallThru'] = true scrollIntoView['@noCallThru'] = true
proxyquire = require('proxyquire') class FakeAdder
Guest = proxyquire('../guest', { instance: null
'./highlighter': highlighter,
'./anchoring/html': anchoring, constructor: ->
'annotator': Annotator, FakeAdder::instance = this
'raf': raf,
'scroll-into-view': scrollIntoView, this.hide = sinon.stub()
}) this.showAt = sinon.stub()
this.target = sinon.stub()
# A little helper which returns a promise that resolves after a timeout # A little helper which returns a promise that resolves after a timeout
timeoutPromise = (millis = 0) -> timeoutPromise = (millis = 0) ->
...@@ -34,6 +43,28 @@ describe 'Guest', -> ...@@ -34,6 +43,28 @@ describe 'Guest', ->
return new Guest(element, options || {}) return new Guest(element, options || {})
beforeEach -> beforeEach ->
FakeAdder::instance = null
rangeUtil = {
isSelectionBackwards: sinon.stub()
selectionFocusRect: sinon.stub()
}
selections = null
Guest = proxyquire('../guest', {
'./adder': {Adder: FakeAdder},
'./anchoring/html': anchoring,
'./highlighter': highlighter,
'./range-util': rangeUtil,
'./selections': (document) ->
new Observable((obs) ->
selections = obs
return () ->
)
'annotator': Annotator,
'raf': raf,
'scroll-into-view': scrollIntoView,
})
fakeCrossFrame = { fakeCrossFrame = {
onConnect: sinon.stub() onConnect: sinon.stub()
on: sinon.stub() on: sinon.stub()
...@@ -206,6 +237,28 @@ describe 'Guest', -> ...@@ -206,6 +237,28 @@ describe 'Guest', ->
emitGuestEvent('getDocumentInfo', assertComplete) emitGuestEvent('getDocumentInfo', assertComplete)
describe 'when the selection changes', ->
it 'shows the adder if the selection contains text', ->
guest = createGuest()
rangeUtil.selectionFocusRect.returns({left: 0, top: 0, width: 5, height: 5})
FakeAdder::instance.target.returns({
left: 0, top: 0, arrowDirection: adder.ARROW_POINTING_UP
})
selections.next({})
assert.called FakeAdder::instance.showAt
it 'hides the adder if the selection does not contain text', ->
guest = createGuest()
rangeUtil.selectionFocusRect.returns(null)
selections.next({})
assert.called FakeAdder::instance.hide
assert.notCalled FakeAdder::instance.showAt
it 'hides the adder if the selection is empty', ->
guest = createGuest()
selections.next(null)
assert.called FakeAdder::instance.hide
describe '#getDocumentInfo()', -> describe '#getDocumentInfo()', ->
guest = null guest = null
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment