Commit 06152dca authored by csillag's avatar csillag

Change the scrolling API

The goal of this change is to avoid any false appearance
that we are implementing the proposed scrollIntoView() API [1],
while we clearly don't.

So, changes:

 - Rename scrollIntoView to scrollToView
   on both Anchor and Highlight
 - Stop returning promises at both methods

I also updated all the tests to reflect this change.

[1] http://dev.w3.org/csswg/cssom-view/#dom-element-scrollintoview
parent cde59e0a
...@@ -142,7 +142,7 @@ class Annotator.Plugin.BucketBar extends Annotator.Plugin ...@@ -142,7 +142,7 @@ class Annotator.Plugin.BucketBar extends Annotator.Plugin
# Get an anchor from the page we want to go to # Get an anchor from the page we want to go to
anchor = next[0] anchor = next[0]
anchor.scrollIntoView() anchor.scrollToView()
_update: => _update: =>
wrapper = @annotator.wrapper wrapper = @annotator.wrapper
......
...@@ -60,10 +60,8 @@ class Anchor ...@@ -60,10 +60,8 @@ class Anchor
# If we are supposed to scroll to the highlight on a page, # If we are supposed to scroll to the highlight on a page,
# and it's available now, go scroll there. # and it's available now, go scroll there.
if @pendingScrollTargetPage? and (hl = @highlight[@pendingScrollTargetPage]) if @pendingScrollTargetPage? and (hl = @highlight[@pendingScrollTargetPage])
hl.scrollIntoView().then => hl.scrollToView()
@pendingScrollResolve() delete @pendingScrollTargetPage
delete @pendingScrollTargetPage
delete @pendingScrollResolve
# Remove the highlights for the given set of pages # Remove the highlights for the given set of pages
virtualize: (pageIndex) => virtualize: (pageIndex) =>
...@@ -94,12 +92,12 @@ class Anchor ...@@ -94,12 +92,12 @@ class Anchor
delete @anchoring.anchors[index] unless anchors.length delete @anchoring.anchors[index] unless anchors.length
# Scroll to this anchor # Scroll to this anchor
scrollIntoView: -> scrollToView: ->
currentPage = @anchoring.document.getPageIndex() currentPage = @anchoring.document.getPageIndex()
if @startPage is @endPage and currentPage is @startPage if @startPage is @endPage and currentPage is @startPage
# It's all in one page. Simply scrolling # It's all in one page. Simply scrolling
@highlight[@startPage].scrollIntoView() @highlight[@startPage].scrollToView()
else else
if currentPage < @startPage if currentPage < @startPage
# We need to go forward # We need to go forward
...@@ -118,13 +116,12 @@ class Anchor ...@@ -118,13 +116,12 @@ class Anchor
# Is this rendered? # Is this rendered?
if @anchoring.document.isPageMapped wantedPage if @anchoring.document.isPageMapped wantedPage
# The wanted page is already rendered, we can simply go there # The wanted page is already rendered, we can simply go there
@highlight[wantedPage].scrollIntoView() @highlight[wantedPage].scrollToView()
else else
# Not rendered yet. Go to the page, we will continue from there # Not rendered yet. Go to the page, we will continue from there
@pendingScrollTargetPage = wantedPage @pendingScrollTargetPage = wantedPage
new Promise (resolve, reject) => @anchoring.document.setPageIndex scrollPage
@pendingScrollResolve = resolve null
@anchoring.document.setPageIndex scrollPage
Annotator.Anchor = Anchor Annotator.Anchor = Anchor
......
...@@ -115,10 +115,9 @@ class TextHighlight ...@@ -115,10 +115,9 @@ class TextHighlight
getHeight: -> $(@_highlights).outerHeight true getHeight: -> $(@_highlights).outerHeight true
# Scroll the highlight into view # Scroll the highlight into view
scrollIntoView: -> scrollToView: ->
new Promise (resolve, reject) => $(@_highlights).scrollintoview()
$(@_highlights).scrollintoview complete: -> null
resolve()
class Annotator.Plugin.TextHighlights extends Annotator.Plugin class Annotator.Plugin.TextHighlights extends Annotator.Plugin
......
...@@ -166,7 +166,7 @@ class Annotator.Guest extends Annotator ...@@ -166,7 +166,7 @@ class Annotator.Guest extends Annotator
crossframe.on 'scrollToAnnotation', (ctx, tag) => crossframe.on 'scrollToAnnotation', (ctx, tag) =>
for a in @anchoring.getAnchors() for a in @anchoring.getAnchors()
if a.annotation.$$tag is tag if a.annotation.$$tag is tag
a.scrollIntoView() a.scrollToView()
return return
crossframe.on 'getDocumentInfo', (trans) => crossframe.on 'getDocumentInfo', (trans) =>
(@plugins.PDF?.getMetaData() ? Promise.reject()) (@plugins.PDF?.getMetaData() ? Promise.reject())
......
...@@ -197,11 +197,11 @@ describe 'Annotator.Guest', -> ...@@ -197,11 +197,11 @@ describe 'Annotator.Guest', ->
it 'scrolls to the anchor with the matching tag', -> it 'scrolls to the anchor with the matching tag', ->
guest = createGuest() guest = createGuest()
anchors = [ anchors = [
{annotation: {$$tag: 'tag1'}, scrollIntoView: sandbox.stub()} {annotation: {$$tag: 'tag1'}, scrollToView: sandbox.stub()}
] ]
sandbox.stub(guest.anchoring, 'getAnchors').returns(anchors) sandbox.stub(guest.anchoring, 'getAnchors').returns(anchors)
emitGuestEvent('scrollToAnnotation', 'ctx', 'tag1') emitGuestEvent('scrollToAnnotation', 'ctx', 'tag1')
assert.called(anchors[0].scrollIntoView) assert.called(anchors[0].scrollToView)
describe 'on "getDocumentInfo" event', -> describe 'on "getDocumentInfo" event', ->
guest = null guest = null
......
...@@ -26,6 +26,11 @@ class TestAnchor extends Annotator.Anchor ...@@ -26,6 +26,11 @@ class TestAnchor extends Annotator.Anchor
describe 'Annotator.Plugin.EnhancedAnchoring', -> describe 'Annotator.Plugin.EnhancedAnchoring', ->
sandbox = null sandbox = null
pendingTest = null
watchForScroll = ->
new Promise (resolve, reject) ->
pendingTest = resolve: resolve
createAnchoringManager = -> createAnchoringManager = ->
annotator = annotator =
...@@ -59,11 +64,11 @@ describe 'Annotator.Plugin.EnhancedAnchoring', -> ...@@ -59,11 +64,11 @@ describe 'Annotator.Plugin.EnhancedAnchoring', ->
anchor: anchor anchor: anchor
page: page page: page
removeFromDocument: sinon.spy() removeFromDocument: sinon.spy()
scrollIntoView: sinon.spy -> scrollToView: sinon.spy -> pendingTest?.resolve()
new Promise (resolve, reject) -> setTimeout -> resolve()
afterEach -> afterEach ->
sandbox.restore() sandbox.restore()
pendingTest = null
describe "createAnchor", -> describe "createAnchor", ->
...@@ -223,14 +228,13 @@ describe 'Annotator.Plugin.EnhancedAnchoring', -> ...@@ -223,14 +228,13 @@ describe 'Annotator.Plugin.EnhancedAnchoring', ->
assert.include hls, anchor2.highlight[anchor2.startPage] assert.include hls, anchor2.highlight[anchor2.startPage]
assert.notInclude hls, anchor3.highlight[anchor3.startPage] assert.notInclude hls, anchor3.highlight[anchor3.startPage]
describe 'Anchor.scrollIntoView()', -> describe 'Anchor.scrollToView()', ->
it 'calls scrollIntoView() on the highlight', -> it 'calls scrollIntoView() on the highlight', ->
am = createAnchoringManager() am = createAnchoringManager()
ann = createTestAnnotation "a1" ann = createTestAnnotation "a1"
anchor = am.createAnchor(ann, ann.target[0]).result anchor = am.createAnchor(ann, ann.target[0]).result
anchor.scrollIntoView().then -> anchor.scrollToView()
assert.called anchor.highlight[anchor.startPage].scrollToView
assert.called anchor.highlight[anchor.startPage].scrollIntoView
describe 'two-phased anchoring', -> describe 'two-phased anchoring', ->
...@@ -658,8 +662,7 @@ describe 'Annotator.Plugin.EnhancedAnchoring', -> ...@@ -658,8 +662,7 @@ describe 'Annotator.Plugin.EnhancedAnchoring', ->
am.document.currentIndex = 5 # We start from page 5 am.document.currentIndex = 5 # We start from page 5
am.document.setPageIndex = sinon.spy() am.document.setPageIndex = sinon.spy()
# Now we trigger the actual action anchor.scrollToView()
anchor.scrollIntoView()
assert.calledWith am.document.setPageIndex, 9 assert.calledWith am.document.setPageIndex, 9
it 'gets the wanted page rendered', -> it 'gets the wanted page rendered', ->
...@@ -673,11 +676,11 @@ describe 'Annotator.Plugin.EnhancedAnchoring', -> ...@@ -673,11 +676,11 @@ describe 'Annotator.Plugin.EnhancedAnchoring', ->
renderPage am.document, 9 renderPage am.document, 9
renderPage am.document, 10 renderPage am.document, 10
# Now we trigger the actual action anchor.scrollToView()
anchor.scrollIntoView().then -> watchForScroll().then ->
assert am.document.isPageMapped 10 assert am.document.isPageMapped 10
it 'calls scrollIntoView() on the highlight', -> it 'calls scrollToView() on the highlight', ->
am = createAnchoringManagerAndLazyDocument() am = createAnchoringManagerAndLazyDocument()
ann = createTestAnnotationForPages "a1", [10] ann = createTestAnnotationForPages "a1", [10]
anchor = am.createAnchor(ann, ann.target[0]).result anchor = am.createAnchor(ann, ann.target[0]).result
...@@ -688,5 +691,6 @@ describe 'Annotator.Plugin.EnhancedAnchoring', -> ...@@ -688,5 +691,6 @@ describe 'Annotator.Plugin.EnhancedAnchoring', ->
renderPage am.document, 9 renderPage am.document, 9
renderPage am.document, 10 renderPage am.document, 10
anchor.scrollIntoView().then -> anchor.scrollToView()
assert.called anchor.highlight[10].scrollIntoView watchForScroll().then ->
assert.called anchor.highlight[10].scrollToView
...@@ -55,19 +55,14 @@ describe 'Annotator.Plugin.TextHighlight', -> ...@@ -55,19 +55,14 @@ describe 'Annotator.Plugin.TextHighlight', ->
annotation = $(hl._highlights).data "annotation" annotation = $(hl._highlights).data "annotation"
assert.equal annotation, "test annotation" assert.equal annotation, "test annotation"
describe "scrollIntoView", -> describe "scrollToView", ->
it 'calls jQuery scrollintoview', -> it 'calls jQuery scrollintoview', ->
hl = createTestHighlight() hl = createTestHighlight()
hl.scrollIntoView() hl.scrollToView()
assert.called Annotator.$.fn.scrollintoview assert.called Annotator.$.fn.scrollintoview
it 'scrolls to the created highlight span', -> it 'scrolls to the created highlight span', ->
hl = createTestHighlight() hl = createTestHighlight()
hl.scrollIntoView() hl.scrollToView()
assert.equal scrollTarget, hl._highlights assert.equal scrollTarget, hl._highlights
it 'resolves the promise after scrolling', ->
hl = createTestHighlight()
hl.scrollIntoView().then ->
assert.ok scrollTarget
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