Merge pull request #2074 from hypothesis/convert-annotation-viewer-content

Convert single annotation page content to Preact

src/sidebar/components/annotation-viewer-content.js

+import { createElement } from 'preact';
+import { useEffect } from 'preact/hooks';
+import propTypes from 'prop-types';
+
+import useStore from '../store/use-store';
+import { withServices } from '../util/service-context';
+
+import ThreadList from './thread-list';
+
 /**
- * Fetch all annotations in the same thread as `id`.
- *
- * @return Promise<Array<Annotation>>
+ * The main content for the single annotation page (aka. https://hypothes.is/a/<annotation ID>)
  */
-function fetchThread(api, id) {
-  let annot;
-  return api.annotation
-    .get({ id: id })
-    .then(function (annot) {
-      if (annot.references && annot.references.length) {
-        // This is a reply, fetch the top-level annotation
-        return api.annotation.get({ id: annot.references[0] });
-      } else {
-        return annot;
-      }
-    })
-    .then(function (annot_) {
-      annot = annot_;
-      return api.search({ references: annot.id });
-    })
-    .then(function (searchResult) {
-      return [annot].concat(searchResult.rows);
-    });
-}
-
-// @ngInject
-function AnnotationViewerContentController(
-  store,
+function AnnotationViewerContent({
   api,
-  rootThread,
+  rootThread: rootThreadService,
   streamer,
-  streamFilter
-) {
-  store.clearAnnotations();
-
-  const annotationId = store.routeParams().id;
+  streamFilter,
+}) {
+  const addAnnotations = useStore(store => store.addAnnotations);
+  const annotationId = useStore(store => store.routeParams().id);
+  const clearAnnotations = useStore(store => store.clearAnnotations);
+  const highlightAnnotations = useStore(store => store.highlightAnnotations);
+  const rootThread = useStore(store =>
+    rootThreadService.thread(store.getState())
+  );
+  const setCollapsed = useStore(store => store.setCollapsed);
 
-  this.rootThread = () => rootThread.thread(store.getState());
+  useEffect(() => {
+    clearAnnotations();
 
-  this.setCollapsed = function (id, collapsed) {
-    store.setCollapsed(id, collapsed);
-  };
+    // TODO - Handle exceptions during the `fetchThread` call.
+    fetchThread(api, annotationId).then(annots => {
+      addAnnotations(annots);
 
-  this.ready = fetchThread(api, annotationId).then(function (annots) {
-    store.addAnnotations(annots);
+      // Find the top-level annotation in the thread that `annotationId` is
+      // part of. This will be different to `annotationId` if `annotationId`
+      // is a reply.
+      const topLevelAnnot = annots.filter(
+        ann => (ann.references || []).length === 0
+      )[0];
 
-    const topLevelAnnot = annots.filter(function (annot) {
-      return (annot.references || []).length === 0;
-    })[0];
+      if (!topLevelAnnot) {
+        // We were able to fetch annotations in the thread that `annotationId`
+        // is part of (note that `annotationId` may refer to a reply) but
+        // couldn't find a top-level (non-reply) annotation in that thread.
+        //
+        // This might happen if the top-level annotation was deleted or
+        // moderated or had its permissions changed.
+        //
+        // We need to decide what what be the most useful behavior in this case
+        // and implement it.
+        /* istanbul ignore next */
+        return;
+      }
 
-    if (!topLevelAnnot) {
-      return;
-    }
+      // Configure the connection to the real-time update service to send us
+      // updates to any of the annotations in the thread.
+      streamFilter
+        .addClause('/references', 'one_of', topLevelAnnot.id, true)
+        .addClause('/id', 'equals', topLevelAnnot.id, true);
+      streamer.setConfig('filter', { filter: streamFilter.getFilter() });
+      streamer.connect();
 
-    streamFilter
-      .addClause('/references', 'one_of', topLevelAnnot.id, true)
-      .addClause('/id', 'equals', topLevelAnnot.id, true);
-    streamer.setConfig('filter', { filter: streamFilter.getFilter() });
-    streamer.connect();
+      // Make the full thread of annotations visible. By default replies are
+      // not shown until the user expands the thread.
+      annots.forEach(annot => setCollapsed(annot.id, false));
 
-    annots.forEach(function (annot) {
-      store.setCollapsed(annot.id, false);
+      // FIXME - This should show a visual indication of which reply the
+      // annotation ID in the URL refers to. That isn't currently working.
+      if (topLevelAnnot.id !== annotationId) {
+        highlightAnnotations([annotationId]);
+      }
     });
+  }, [
+    annotationId,
+
+    // Static dependencies.
+    addAnnotations,
+    api,
+    clearAnnotations,
+    highlightAnnotations,
+    setCollapsed,
+    streamFilter,
+    streamer,
+  ]);
 
-    if (topLevelAnnot.id !== annotationId) {
-      store.highlightAnnotations([annotationId]);
-    }
-  });
+  return <ThreadList thread={rootThread} />;
 }
 
-export default {
-  controller: AnnotationViewerContentController,
-  controllerAs: 'vm',
-  bindings: {},
-  template: require('../templates/annotation-viewer-content.html'),
+AnnotationViewerContent.propTypes = {
+  // Injected.
+  api: propTypes.object,
+  rootThread: propTypes.object,
+  streamer: propTypes.object,
+  streamFilter: propTypes.object,
 };
+
+AnnotationViewerContent.injectedProps = [
+  'api',
+  'rootThread',
+  'streamer',
+  'streamFilter',
+];
+
+// NOTE: The function below is intentionally at the bottom of the file.
+//
+// Putting it at the top resulted in an issue where the `createElement` import
+// wasn't correctly referenced in the body of `AnnotationViewerContent` in
+// the compiled JS, causing a runtime error.
+
+/**
+ * Fetch all annotations in the same thread as `id`.
+ *
+ * @param {Object} api - API client
+ * @param {string} id - Annotation ID. This may be an annotation or a reply.
+ * @return Promise<Annotation[]> - The annotation, followed by any replies.
+ */
+async function fetchThread(api, id) {
+  let annot = await api.annotation.get({ id });
+
+  if (annot.references && annot.references.length) {
+    // This is a reply, fetch the top-level annotation
+    annot = await api.annotation.get({ id: annot.references[0] });
+  }
+
+  // Fetch all replies to the top-level annotation.
+  const replySearchResult = await api.search({ references: annot.id });
+
+  return [annot, ...replySearchResult.rows];
+}
+
+export default withServices(AnnotationViewerContent);

src/sidebar/components/test/annotation-viewer-content-test.js

-import angular from 'angular';
-
-import annotationViewerContent from '../annotation-viewer-content';
-
-// Fake implementation of the API for fetching annotations and replies to
-// annotations.
-function FakeApi(annots) {
-  this.annots = annots;
-
-  this.annotation = {
-    get: function (query) {
-      let result;
-      if (query.id) {
-        result = annots.find(function (a) {
-          return a.id === query.id;
-        });
-      }
-      return Promise.resolve(result);
-    },
-  };
-
-  this.search = function (query) {
-    let result;
+import { createElement } from 'preact';
+import { mount } from 'enzyme';
+
+import { waitFor } from '../../../test-util/wait';
+import mockImportedComponents from '../../../test-util/mock-imported-components';
+
+import AnnotationViewerContent, {
+  $imports,
+} from '../annotation-viewer-content';
+
+/**
+ * Fake implementation of the `api` service.
+ */
+class FakeApi {
+  constructor(annots) {
+    this.annotations = annots;
+
+    this.annotation = {
+      get: async query => this.annotations.find(a => a.id === query.id),
+    };
+  }
+
+  async search(query) {
+    let matches = [];
     if (query.references) {
-      result = annots.filter(function (a) {
-        return a.references && a.references.indexOf(query.references) !== -1;
-      });
+      matches = this.annotations.filter(
+        a => a.references && a.references.includes(query.references)
+      );
     }
-    return Promise.resolve({ rows: result });
-  };
+    return { rows: matches };
+  }
 }
 
-describe('annotationViewerContent', function () {
-  before(function () {
-    angular
-      .module('h', [])
-      .component('annotationViewerContent', annotationViewerContent);
-  });
+describe('AnnotationViewerContent', () => {
+  let fakeStore;
+  let fakeRootThread;
+  let fakeStreamer;
+  let fakeStreamFilter;
 
-  beforeEach(angular.mock.module('h'));
-
-  function createController(opts) {
-    const locals = {
-      store: {
-        addAnnotations: sinon.stub(),
-        clearAnnotations: sinon.stub(),
-        setCollapsed: sinon.stub(),
-        highlightAnnotations: sinon.stub(),
-        routeParams: sinon.stub().returns({ id: 'test_annotation_id' }),
-        subscribe: sinon.stub(),
-      },
-      api: opts.api,
-      rootThread: { thread: sinon.stub() },
-      streamer: {
-        setConfig: function () {},
-        connect: function () {},
-      },
-      streamFilter: {
-        addClause: function () {
-          return {
-            addClause: function () {},
-          };
-        },
-        getFilter: function () {},
+  beforeEach(() => {
+    fakeStore = {
+      addAnnotations: sinon.stub(),
+      clearAnnotations: sinon.stub(),
+      getState: sinon.stub().returns({}),
+      highlightAnnotations: sinon.stub(),
+      routeParams: sinon.stub().returns({ id: 'test_annotation_id' }),
+      setCollapsed: sinon.stub(),
+    };
+
+    fakeRootThread = { thread: sinon.stub().returns({}) };
+
+    fakeStreamer = {
+      setConfig: () => {},
+      connect: () => {},
+    };
+
+    fakeStreamFilter = {
+      addClause: () => {
+        return {
+          addClause: () => {},
+        };
       },
+      getFilter: () => {},
     };
 
-    let $componentController;
-    angular.mock.inject(function (_$componentController_) {
-      $componentController = _$componentController_;
-    });
-    locals.ctrl = $componentController('annotationViewerContent', locals, {
-      search: {},
+    $imports.$mock(mockImportedComponents());
+    $imports.$mock({
+      '../store/use-store': callback => callback(fakeStore),
     });
-    return locals;
+  });
+
+  afterEach(() => {
+    $imports.$restore();
+  });
+
+  function createComponent({ api }) {
+    return mount(
+      <AnnotationViewerContent
+        api={api}
+        rootThread={fakeRootThread}
+        streamer={fakeStreamer}
+        streamFilter={fakeStreamFilter}
+      />
+    );
   }
 
-  describe('the standalone view for a top-level annotation', function () {
-    it('loads the annotation and all replies', function () {
+  function waitForAnnotationsToLoad() {
+    return waitFor(() => fakeStore.addAnnotations.called);
+  }
+
+  describe('the standalone view for a top-level annotation', () => {
+    it('loads the annotation and all replies', async () => {
       const fakeApi = new FakeApi([
         { id: 'test_annotation_id' },
         { id: 'test_reply_id', references: ['test_annotation_id'] },
       ]);
-      const controller = createController({ api: fakeApi });
-      return controller.ctrl.ready.then(function () {
-        assert.calledOnce(controller.store.addAnnotations);
-        assert.calledWith(
-          controller.store.addAnnotations,
-          sinon.match(fakeApi.annots)
-        );
-      });
+      createComponent({ api: fakeApi });
+
+      await waitForAnnotationsToLoad();
+
+      assert.calledOnce(fakeStore.addAnnotations);
+      assert.calledWith(
+        fakeStore.addAnnotations,
+        sinon.match(fakeApi.annotations)
+      );
     });
 
-    it('does not highlight any annotations', function () {
+    it('does not highlight any annotations', async () => {
       const fakeApi = new FakeApi([
         { id: 'test_annotation_id' },
         { id: 'test_reply_id', references: ['test_annotation_id'] },
       ]);
-      const controller = createController({ api: fakeApi });
-      return controller.ctrl.ready.then(function () {
-        assert.notCalled(controller.store.highlightAnnotations);
-      });
+      createComponent({ api: fakeApi });
+
+      await waitForAnnotationsToLoad();
+
+      assert.notCalled(fakeStore.highlightAnnotations);
     });
   });
 
-  describe('the standalone view for a reply', function () {
-    it('loads the top-level annotation and all replies', function () {
+  describe('the standalone view for a reply', () => {
+    it('loads the top-level annotation and all replies', async () => {
       const fakeApi = new FakeApi([
         { id: 'parent_id' },
         { id: 'test_annotation_id', references: ['parent_id'] },
       ]);
-      const controller = createController({ api: fakeApi });
-      return controller.ctrl.ready.then(function () {
-        assert.calledWith(
-          controller.store.addAnnotations,
-          sinon.match(fakeApi.annots)
-        );
-      });
+      createComponent({ api: fakeApi });
+
+      await waitForAnnotationsToLoad();
+
+      assert.calledWith(
+        fakeStore.addAnnotations,
+        sinon.match(fakeApi.annotations)
+      );
     });
 
-    it('expands the thread', function () {
+    it('expands the thread', async () => {
       const fakeApi = new FakeApi([
         { id: 'parent_id' },
         { id: 'test_annotation_id', references: ['parent_id'] },
       ]);
-      const controller = createController({ api: fakeApi });
-      return controller.ctrl.ready.then(function () {
-        assert.calledWith(controller.store.setCollapsed, 'parent_id', false);
-        assert.calledWith(
-          controller.store.setCollapsed,
-          'test_annotation_id',
-          false
-        );
-      });
+
+      createComponent({ api: fakeApi });
+
+      await waitForAnnotationsToLoad();
+
+      assert.calledWith(fakeStore.setCollapsed, 'parent_id', false);
+      assert.calledWith(fakeStore.setCollapsed, 'test_annotation_id', false);
     });
 
-    it('highlights the reply', function () {
+    it('highlights the reply', async () => {
       const fakeApi = new FakeApi([
         { id: 'parent_id' },
         { id: 'test_annotation_id', references: ['parent_id'] },
       ]);
-      const controller = createController({ api: fakeApi });
-      return controller.ctrl.ready.then(function () {
-        assert.calledWith(
-          controller.store.highlightAnnotations,
-          sinon.match(['test_annotation_id'])
-        );
-      });
+      createComponent({ api: fakeApi });
+
+      await waitForAnnotationsToLoad();
+
+      assert.calledWith(
+        fakeStore.highlightAnnotations,
+        sinon.match(['test_annotation_id'])
+      );
     });
   });
 });

src/sidebar/index.js

@@ -119,6 +119,7 @@ registerIcons(iconSet);
 // Preact UI components that are wrapped for use within Angular templates.
 
 import Annotation from './components/annotation';
+import AnnotationViewerContent from './components/annotation-viewer-content';
 import FocusedModeHeader from './components/focused-mode-header';
 import HelpPanel from './components/help-panel';
 import LoggedOutMessage from './components/logged-out-message';
@@ -136,7 +137,6 @@ import TopBar from './components/top-bar';
 
 // Remaining UI components that are still built with Angular.
 
-import annotationViewerContent from './components/annotation-viewer-content';
 import hypothesisApp from './components/hypothesis-app';
 import sidebarContent from './components/sidebar-content';
 import streamContent from './components/stream-content';
@@ -257,7 +257,10 @@ function startAngularApp(config) {
 
     // UI components
     .component('annotation', wrapComponent(Annotation))
-    .component('annotationViewerContent', annotationViewerContent)
+    .component(
+      'annotationViewerContent',
+      wrapComponent(AnnotationViewerContent)
+    )
     .component('helpPanel', wrapComponent(HelpPanel))
     .component('loginPromptPanel', wrapComponent(LoginPromptPanel))
     .component('loggedOutMessage', wrapComponent(LoggedOutMessage))

src/sidebar/templates/annotation-viewer-content.html

-<thread-list
-  on-change-collapsed="vm.setCollapsed(id, collapsed)"
-  on-force-visible="vm.forceVisible(thread)"
-  show-document-info="true"
-  thread="vm.rootThread()">
-</thread-list>