Skip to content

C
coopwire-hypothesis
Switch branch/tag
  1. 16 Mar, 2021 6 commits
    • Robert Knight's avatar
      Improve comments for `load` method in `loadAnnotationsService` · 5445ad58
      Robert Knight authored 
      It is safe to call `load` while annotations are already being loaded,
in that it does all the necessary cleanup/interruption of the existing
fetch. However it wasn't immediately clear to me from the code that this
was the case.
      5445ad58
    • Robert Knight's avatar
      Fetch annotations in parallel with groups and profile where possible · 1d00a99b
      Robert Knight authored 
      Currently both the sidebar and notebook wait until the user's profile and groups are
fetched before fetching annotations. This is because the search query to
fetch annotations depends on the focused group. The groups query may
take several hundred ms to complete, so this ends up delaying the
initial fetch and display of annotations by that amount of time.

In the notebook the common case is that the user is already logged in
and the notebook is configured to display a specific group ("the direct
linked group"): whatever the user had previously selected in the sidebar.
In this scenario it is very likely that the direct-linked group will end
up being the focused one after groups are fetched.

Based on these assumptions, we can speed up the initial annotation by
loading annotations from the direct-linked group. If the focused group
ends up being different, annotations will just be re-fetched from the
correct group once that is known.
      1d00a99b
    • Robert Knight's avatar
      Remove obsolete references to `offset` query param · 8d237661
      Robert Knight authored 
       - Remove unused references to `offset` search query param
 - Add JSDoc to clarify what the `searchAfter` param is
      8d237661
    • Robert Knight's avatar
      Add a more comprehensive test of `search_after` query param · ae62cb39
      Robert Knight authored
      ae62cb39
    • Robert Knight's avatar
      Add additional SearchClient test for search params · c336a814
      Robert Knight authored 
      - Add an additional test that checks the query params sent for each of the
  pages sent for a simple single-URI query.

- Refactor the remaining SearchClient tests which still use
  promise-chains to use async/await instead for readability and
  consistency
      c336a814
    • Robert Knight's avatar
      Use cursor-based pagination through search API results · b68dae47
      Robert Knight authored 
      Optimize fetching pages of results from the backend by using
cursor-based paging (via `search_after`) rather than offset-based paging
(via `offset`, `limit`). Cursor-based paging is more efficient to handle in
the backend as the starting position of a page within the overall result set increases.

This also fixes a race condition where results at the boundary between
pages might be fetched twice or not fetched if new matches for the query
become available while results are fetched.
      b68dae47
  3. 15 Mar, 2021 28 commits
  5. 12 Mar, 2021 4 commits
    • Robert Knight's avatar
      Optimize annotation fetch order in notebook · cd65d211
      Robert Knight authored 
      Reduce the perceived loading time in the notebook by fetching
annotations in, roughly, the reverse chronological order in which they
are displayed.

By default `SearchClient` fetches annotations by ascending creation
date. This makes sense in the sidebar as it typically correlates,
roughly, with the order in which the sidebar presents annotations by
default (document location order). For the notebook however this is
suboptimal.

 - Add `sortBy` and `sortOrder` options to `SearchClient` and
   `loadAnnotations` to control fetch order, with the defaults remaining
   the same as before (`created`, `asc`).
 - In `NotebookView` set `sortBy` to `updated` and `sortOrder` to desc
   to fetch annotations in reverse chronological order.
      cd65d211
    • Eduardo Sanz García's avatar
      Added utility to manage collection of listeners · 2a814392
      Eduardo Sanz García authored 
      Following on @robertknight
[suggestion](https://github.com/hypothesis/client/pull/3035#pullrequestreview-599668554)
I created a tiny utility to make easier to unregister event listeners.
      2a814392
    • Robert Knight's avatar
      Simplify "no selectable text" banner styling · 5fbb3e20
      Robert Knight authored 
      Avoid the need to customize various inline styles of the
`<hypothesis-banner>` element by positioning it at the top of the DOM as
the first child of `document.body`. As a result it naturally pushes the
PDF.js UI down. The only change we need to make to PDF.js's UI is to adjust
the size of the `#outerContainer` element so that it does not overflow the
bottom of the viewport.

Aside from making styling simpler, making the banner's DOM position
better match its visual position may help with reading order for screen readers.
Related to this the banner is currently missing useful semantic
information for a11y purposes.

I also removed the banner resizing handling for the moment on the basis that it
isn't that important.
      5fbb3e20
    • Robert Knight's avatar
      Move "no selectable text" warning banner into shadow DOM · 15eb9ec1
      Robert Knight authored 
      For consistency with the rest of the UI elements rendered by the
annotator code, move the warning banner at the top of the PDF into
shadow DOM. This is also another step towards not needing to load any
global CSS (eg. our CSS reset) into the host page.

 - Change the container element for the banner from `<div>` to
   `<hypothesis-banner>` to match other `<hypothesis-*>` UI container
   elements
 - Move the `WarningBanner` component into `components/WarningBanner.js`
   and rename the CSS classes to match
      15eb9ec1
  7. 10 Mar, 2021 2 commits