1. 20 May, 2021 2 commits
    • Eduardo Sanz García's avatar
      Display the annotation header on the top-level annotation · fcfde74a
      Eduardo Sanz García authored
      This PR ensures that the annotation header is always shown, even if the
      top-level annotation is hidden by a filter. The annotation header
      contains the document target. This information is specially important in
      the context of the notebook, where annotations from different documents
      are displayed together.
      
      I created a wrapper around the `AnnotationHeader` component. I couldn't
      use it directly because I needed a couple of methods from the store to
      mimic 100% the same behaviour as when `AnnotationHeader` is called
      through `Annotation` component.
      fcfde74a
    • Eduardo Sanz García's avatar
      Remove flag icon from annotation cards for LMS users · cf22b700
      Eduardo Sanz García authored
      1. Added an option (`allowFlagging`) in the service configuration to
         enable/disable flagging.
      
      2. Added logic in `AnnoationActionBar` component to hide the flag icon
         if the service configuration has the option `allowFlagging`.
      
      Closes https://github.com/hypothesis/product-backlog/issues/1126
      cf22b700
  2. 17 May, 2021 11 commits
  3. 14 May, 2021 2 commits
  4. 13 May, 2021 1 commit
    • Lyza Danger Gardner's avatar
      Add classname prefix to avoid conflicts with `frontend-shared` · 3b967dc6
      Lyza Danger Gardner authored
      Add a temporary class name prefix to the local `Panel` component styles.
      
      This will avoid conflicts with the styling for the new `Panel` component
      in the `frontend-shared` package.
      
      In the future:
      
      * This local `Panel` will be supplanted by the shared variant, and
      * Shared component classnames will be prefixed to avoid future conflicts
      3b967dc6
  5. 12 May, 2021 3 commits
    • Robert Knight's avatar
      Remove unused `store.getState` stubs in tests · 801b3fa8
      Robert Knight authored
      All of the components use selector methods now instead of accessing the
      store's internal state directly.
      801b3fa8
    • Robert Knight's avatar
      Use store selectors instead of `getState` in `FrameSyncService` · 0e523df3
      Robert Knight authored
      Remove the last remaining usage of `store.getState()` outside of the
      store implementation in favor of the `store.allAnnotations()` selector
      method.
      
      This allows simplifying the state structure in the fake store that the
      `FrameSyncService` tests create.
      0e523df3
    • Robert Knight's avatar
      Update export style and documentation for sidebar store · 4b7c566d
      Robert Knight authored
      Convert the `store/index.js` and `store/create-store.js` modules to use
      named rather than default exports per our current conventions, and
      revise the documentation.
      
      The goal of the revised documentation is to more clearly describe what the
      store is and how it is used within the sidebar/notebook app, assuming
      that the reader is likely have at least some familiarity with Redux (or
      can read the "Introduction" section of the linked website if not). In
      particular I have tried to convey:
      
       - The separation between the "base" `createStore` function which is not
         application-specific and the `createSidebarStore` function and the
         modules it uses which are
       - How the store in the sidebar app differs from a standard/base Redux store
       - Best practices around using the store from other parts of the app
         (use `useStoreProxy` in UI components, use selector and action
         methods rather than `getState` and `dispatch`)
      4b7c566d
  6. 11 May, 2021 11 commits
    • Robert Knight's avatar
      Replace `document.title` as a fallback for title in PDFs · 40eaf5c9
      Robert Knight authored
      Replace the usage of `document.title` as a way to get the document title
      if the PDF has no embedded title in either its _document info
      dictionary_ or _metadata stream_.
      
      In top-level frames using `document.title` (where `document` is the
      global HTML document, not the PDF) works because PDF.js sets the title
      based on the first non-empty value from:
      
       1. The embedded title
       2. The filename from the `Content-Disposition` header
       3. The last segment of the URL's path (eg. "test.pdf" in
          "https://example.com/test.pdf")
      
      When PDF.js is embedded in an iframe however, it does not set
      `document.title` by default. As a result, documents were ending up in
      Hypothesis with a generic "PDF.js viewer" title.
      
      This commit implements (roughly) the same logic that PDF.js uses to
      determine the value used to set `document.title`, in the case where the
      PDF has no embedded title. This means implementing steps (2) and (3)
      from the above list. The `Content-Disposition` filename is not exposed
      as a public property on `PDFViewerApplication`, so
      `PDFMetadata#getMetadata` was refactored to call the
      `pdfDocument.getMetadata` instead.
      
      Fixes https://github.com/hypothesis/client/issues/3372
      40eaf5c9
    • Robert Knight's avatar
      Add missing tests for various error paths · 96723e83
      Robert Knight authored
      I ignored one path that is only used in local development.
      96723e83
    • Robert Knight's avatar
      Convert `streamer` service to an ES class · ca5b5393
      Robert Knight authored
      Part of https://github.com/hypothesis/client/issues/3298
      
      Convert the `streamer` service to an ES class, modernize the code
      (functions => arrow functions, Promise chains => async/await) and
      improve the class documentation. There are no significant changes to the internal
      logic or API.
      ca5b5393
    • Robert Knight's avatar
      Simplify code using `filter`/`map` · 5ffeba96
      Robert Knight authored
      This makes it more obvious what is going on at a glance. Refactoring the
      code in this way revealed to TS a hazard where `newGroupId` could, at
      least based only on local reasoning, be null. In practice this shouldn't
      happen due to the contexts when the method is called, but I've added a
      check to be safe and keep TS happy.
      5ffeba96
    • Robert Knight's avatar
      Re-order `Group` constructor arguments · 97eae5d4
      Robert Knight authored
      Re-order the arguments based on an alphabetic sorting of the `@param`
      lines, which results in sorting by import path. This will make it more obvious
      where to slot in any new dependencies in future. Also sort the field
      initialization to match.
      97eae5d4
    • Robert Knight's avatar
      Type a reference to the groups service · ffa74228
      Robert Knight authored
      ffa74228
    • Robert Knight's avatar
      Convert `groups` service to an ES class · 82d434df
      Robert Knight authored
       - Convert the `groups` service to an ES class and the many closures in
         the constructor to private or public methods.
      
       - Remove the unused `all` and `get` methods of the groups service.
         These were trivial wrappers around store methods. UI components
         should use the store methods directly.
      82d434df
    • Robert Knight's avatar
      Improve some test descriptions and add a comment · 87956ee3
      Robert Knight authored
       - Clarify what a "non-file URL" means and add an additional test case
         for HTTPS URLs
       - Add a comment to clarify the precedence order of titles. The tests
         could more directly check that the expected precedence order is
         respected, but the logic here is expected to change very soon, so
         just add a comment for now.
      87956ee3
    • Robert Knight's avatar
      Convert remaining promise chains to async/await · c4612eda
      Robert Knight authored
      Convert remaining promise chains in `PDFMetadata` tests to async/await.
      c4612eda
    • Robert Knight's avatar
      Refactor `getMetadata` tests · bfe5d42b
      Robert Knight authored
       - Convert promise chains to async/await
      
       - Add missing tests to check that the PDF URL and fingerprint URL
         appear in the object returned by `getMetadata`. These were previously
         tested indirectly in other tests.
      
       - Change tests to only check the specific properties of the returned
         object that are of interest. This reduces the changes needed when
         tests change unrelated parts of the output.
      bfe5d42b
    • Robert Knight's avatar
      Refactor test setup in `PDFMetadata` tests · 1b539b87
      Robert Knight authored
      Refactor the setup steps in PDFMetadata tests to make it easier
      to customize the PDF metadata exposed by the fake PDF.js environment.
      Instead of creating a fake `PDFViewerApplication` and `PDFMetadata`
      instance before each test, provide a helper function that creates both
      using the provided metadata.
      1b539b87
  7. 10 May, 2021 10 commits