Unable to annotate on page with many annotations

[segdeha]

Steps to reproduce

1. Go to https://www.theatlantic.com/magazine/archive/1945/07/as-we-may-think/303881/
2. Activate the Chrome Extension
3. Select some text and click "Annotate"

Expected behaviour

Sidebar should open with the editor selected.

Actual behaviour

Sidebar opens with no editor visible. Scrolling around the sidebar does not reveal the editor.

Browser/system information

Chrome 57.0.2987.98
Extension version 1.6.0.0 (Official Build)

Additional details

Screenshot attached

[robertknight]

I can reproduce on the example page at localhost:3000 starting with an empty set of annotations as follows:

for n in 1..15
  1. select n'th word in document, highlight it and create an annotation
  2. scroll the sidebar back up to the top

For the first 11 annotations this scrolled the sidebar correctly. For the 12th annotation it did not. When reproducing the results might vary by screen height since I'm guessing this relates to windowing/virtualization of the thread list.

[robertknight]

Sidebar opens with no editor visible. Scrolling around the sidebar does not reveal the editor.

In my test on that article, creating an annotation at the first word resulted in the sidebar opening with no editor visible. However when I scrolled down by a view cards then an editor appeared.

[robertknight]

After an initial investigation, the problem is that the logic which computes the position to scroll to after a new annotation is added to the sidebar always returns 0, so the sidebar never scrolls. This is due to <thread-list> making an assumption about how to measure the maximum scroll offset for the sidebar which no longer holds after #306. Specifically, this code:

function scrollOffset(id) {
    // Note: This assumes that the element occupies the entire height of the
    // containing document. It would be preferable if only the contents of the
    // <thread-list> itself scrolled.
    var maxYOffset = document.body.clientHeight - window.innerHeight;
    return Math.min(maxYOffset, visibleThreads.yOffsetOf(id));
  }

PR #306 set the height of the <hypothesis-app> container element to 100%. As a result, document.body.clientHeight is now always equal to window.innerHeight instead of being however tall the thread list is plus its top offset.

In addition to the problem of the scroll offset being calculated as zero, there is also an issue of when the scroll is triggered. The <thread-list> component listens for a BEFORE_ANNOTATION_CREATED event and then assumes that when this event is received, it will be able to find the new annotation in the thread list data structure (which comes from rootThread.thread). If however it's subscriber for this event is emitted before that data structure is updated, it will fail to find the new annotation in the thread list and fail to calculate a scroll offset for it. This problem happens to be mitigated at the moment because scrollIntoView() sets up a timeout which triggers a second attempt to scroll the annotation into view just after the first attempt.