Merge pull request #1932 from hypothesis/remove-invisible-threads

Remove "invisible threads" logic

src/sidebar/components/thread-list.js

@@ -37,16 +37,6 @@ function getThreadHeight(id) {
   return elementHeight + marginHeight;
 }
 
-const virtualThreadOptions = {
-  // identify the thread types that need to be rendered
-  // but not actually visible to the user
-  invisibleThreadFilter: function(thread) {
-    // new highlights should always get rendered so we don't
-    // miss saving them via the render-save process
-    return thread.annotation.$highlight && metadata.isNew(thread.annotation);
-  },
-};
-
 // @ngInject
 function ThreadListController($element, $scope, settings, store) {
   // `visibleThreads` keeps track of the subset of all threads matching the
@@ -66,12 +56,7 @@ function ThreadListController($element, $scope, settings, store) {
 
   this.isThemeClean = settings.theme === 'clean';
 
-  const options = Object.assign(
-    {
-      scrollRoot: this.scrollRoot,
-    },
-    virtualThreadOptions
-  );
+  const options = { scrollRoot: this.scrollRoot };
   let visibleThreads;
 
   this.$onInit = () => {
@@ -98,7 +83,6 @@ function ThreadListController($element, $scope, settings, store) {
   function onVisibleThreadsChanged(state) {
     self.virtualThreadList = {
       visibleThreads: state.visibleThreads,
-      invisibleThreads: state.invisibleThreads,
       offscreenUpperHeight: state.offscreenUpperHeight + 'px',
       offscreenLowerHeight: state.offscreenLowerHeight + 'px',
     };

src/sidebar/templates/thread-list.html

@@ -17,11 +17,6 @@
     <hr ng-if="vm.isThemeClean"
         class="thread-list__separator--theme-clean" />
   </li>
-  <li id="{{child.id}}"
-      ng-show="false"
-      ng-repeat="child in vm.virtualThreadList.invisibleThreads track by child.id">
-      <annotation-thread thread="child" />
-  </li>
   <li class="thread-list__spacer"
       ng-style="{height: vm.virtualThreadList.offscreenLowerHeight}"></li>
 </ul>

src/sidebar/test/virtual-thread-list-test.js

@@ -4,9 +4,7 @@ import { $imports } from '../virtual-thread-list';
 describe('VirtualThreadList', function() {
   let lastState;
   let threadList;
-  const threadOptions = {
-    invisibleThreadFilter: null,
-  };
+  const threadOptions = {};
 
   let fakeScope;
   let fakeScrollRoot;
@@ -95,7 +93,6 @@ describe('VirtualThreadList', function() {
       innerHeight: 100,
     };
 
-    threadOptions.invisibleThreadFilter = sinon.stub().returns(false);
     threadOptions.scrollRoot = fakeScrollRoot;
 
     const rootThread = { annotation: undefined, children: [] };
@@ -145,19 +142,10 @@ describe('VirtualThreadList', function() {
       fakeScrollRoot.scrollTop = testCase.scrollOffset;
       fakeWindow.innerHeight = testCase.windowHeight;
 
-      // make sure for everything that is not being presented in the
-      // visible viewport, we pass it to this function.
-      threadOptions.invisibleThreadFilter.returns(true);
-
       threadList.setRootThread(thread);
 
       const visibleIDs = threadIDs(lastState.visibleThreads);
-      const invisibleIDs = threadIDs(lastState.invisibleThreads);
       assert.deepEqual(visibleIDs, testCase.expectedVisibleThreads);
-      assert.equal(
-        invisibleIDs.length,
-        testCase.threads - testCase.expectedVisibleThreads.length
-      );
       assert.equal(
         lastState.offscreenUpperHeight,
         testCase.expectedHeightAbove

src/sidebar/virtual-thread-list.js

@@ -3,9 +3,6 @@ import EventEmitter from 'tiny-emitter';
 
 /**
  * @typedef Options
- * @property {Function} [invisibleThreadFilter] - Function used to determine
- *   whether an off-screen thread should be rendered or not.  Called with a
- *   `Thread` and if it returns `true`, the thread is rendered even if offscreen.
  * @property {Element} [scrollRoot] - The scrollable Element which contains the
  *   thread list. The set of on-screen threads is determined based on the scroll
  *   position and height of this element.
@@ -36,8 +33,6 @@ export default class VirtualThreadList extends EventEmitter {
 
     this._rootThread = rootThread;
 
-    this._options = Object.assign({}, options);
-
     // Cache of thread ID -> last-seen height
     this._heights = {};
 
@@ -151,12 +146,6 @@ export default class VirtualThreadList extends EventEmitter {
     // actually be created.
     const visibleThreads = [];
 
-    // List of annotations which are required to be rendered but we do not
-    // want them visible. This is to ensure that we allow items to be rendered
-    // and initialized (for saving purposes) without having them be presented
-    // in out of context scenarios (i.e. in wrong order for sort)
-    const invisibleThreads = [];
-
     const allThreads = this._rootThread.children;
     const visibleHeight = this.window.innerHeight;
     let usedHeight = 0;
@@ -166,8 +155,6 @@ export default class VirtualThreadList extends EventEmitter {
       thread = allThreads[i];
       const threadHeight = this._height(thread.id);
 
-      let added = false;
-
       if (
         usedHeight + threadHeight <
         this.scrollRoot.scrollTop - MARGIN_ABOVE
@@ -180,24 +167,11 @@ export default class VirtualThreadList extends EventEmitter {
       ) {
         // Thread is either in or close to the viewport
         visibleThreads.push(thread);
-        added = true;
       } else {
         // Thread is below viewport
         offscreenLowerHeight += threadHeight;
       }
 
-      // any thread that is not going to go through the render process
-      // because it is already outside of the viewport should be checked
-      // to see if it needs to be added as an invisible render. So it will
-      // be available to go through rendering but not visible to the user
-      if (
-        !added &&
-        this._options.invisibleThreadFilter &&
-        this._options.invisibleThreadFilter(thread)
-      ) {
-        invisibleThreads.push(thread);
-      }
-
       usedHeight += threadHeight;
     }
 
@@ -205,13 +179,11 @@ export default class VirtualThreadList extends EventEmitter {
       offscreenLowerHeight: offscreenLowerHeight,
       offscreenUpperHeight: offscreenUpperHeight,
       visibleThreads: visibleThreads,
-      invisibleThreads: invisibleThreads,
     });
     return {
       offscreenLowerHeight,
       offscreenUpperHeight,
       visibleThreads,
-      invisibleThreads,
     };
   }
 }