1. 16 Apr, 2021 9 commits
    • Robert Knight's avatar
      Fix a test that did not execute intended code path · 9c06ba9b
      Robert Knight authored
      The customized result for the first stub call was not used because a
      more specific response was defined earlier using `withArgs`.
      9c06ba9b
    • Robert Knight's avatar
      Convert `apiRoutes` service to ES class · ffa9de55
      Robert Knight authored
      Follow the pattern introduced in https://github.com/hypothesis/client/pull/3296
      to convert the API routes service to a class and improve various types
      along the way.
      ffa9de55
    • Eduardo Sanz García's avatar
      Kill gulp-karma process on Ctrl-C · 4db28b72
      Eduardo Sanz García authored
      The only way to kill `yarn test --watch` is sending a SIGINT via a
      <kbd>Ctrl</kbd>+<kbd>C</kbd> keyboard shorcut. Killing karma process in
      this way sometimes leaves the parent gulp process orphan. That's because
      when karma is killed by SIGINT sometimes doesn't run the `done` in the
      callback, hence leaving the gulp process waiting for the `done` signal.
      
      On more rare occasions, I have seen orphan karma process too.
      
      The solution presented here registers a listener for SIGINT and call the
      `done` function. It doesn't unregister the event.
      4db28b72
    • Robert Knight's avatar
      Add tests for hoving and clicking highlights · f49246b8
      Robert Knight authored
      Add tests for logic in Guest related to focusing/selecting annotation cards
      in the sidebar when the corresponding highlights in the document are
      hovered.
      f49246b8
    • Robert Knight's avatar
      Simplify Guest API for selecting annotations in the sidebar · 15ec6e28
      Robert Knight authored
      Previously there were three public methods for setting the selected annotations
      in the sidebar and opening it, but only one was used outside of `Guest`
      and there was an inconsistency when toggling vs selecting normally.
      
       - Combine the `showAnnotations`, `toggleAnnotationSelection` and
         `selectAnnotations` method into a single `selectAnnotations` method
         with a `toggle` parameter
      
       - Add tests for `selectAnnotations`
      
       - Fix an inconsistency where selecting an annotation without holding
         Ctrl to toggle the selection would open the sidebar, but selecting an
         annotation with Ctrl to toggle the selection did not
      15ec6e28
    • Robert Knight's avatar
      Add JSDoc to `anchors` property of Guest · bd8897c7
      Robert Knight authored
      bd8897c7
    • Robert Knight's avatar
      Rename `Guest#_connectAnnotationUISync` · 46ecdc53
      Robert Knight authored
      Give this method a name that more obviously explains what it does.
      46ecdc53
    • Robert Knight's avatar
      Add underscore to private fields in Guest · 425f1fe3
      Robert Knight authored
      Make the distinction between the private and public API for Guest
      clearer by marking private fields clearly.
      425f1fe3
    • Robert Knight's avatar
      Convert toastMessenger service to ES class · 6ce8d58d
      Robert Knight authored
       - Convert toastMessenger service to an ES class using a named export
       - Specify type of `toastMessenger` references to this service
      6ce8d58d
  2. 15 Apr, 2021 8 commits
    • Kyle Keating's avatar
      Use directLinkedGroupId when loading groups · 375d83d1
      Kyle Keating authored
      Change loadServiceSpecifiedGroups() so that it can pass along an optional group ID setting value from the `directLinkedGroupId` selector to be an initially focused group.
      
      When loading groups from a service e.g. `loadServiceSpecifiedGroups`, previously it assumed that the `directLinkedGroupId` value would never be used in this context. Since the introduction of the Notebook and specifically in the context of LMS, we have both a service setting which contains the groups  (`settings.services[0].groups <Promise>`) and a top level group to set focus too. (`settings.group`).
      375d83d1
    • Robert Knight's avatar
      Revise some comments following PR feedback · 5f3f5421
      Robert Knight authored
      5f3f5421
    • Robert Knight's avatar
      Ensure confirmation dialog appears above top bar · c815849a
      Robert Knight authored
      Set a Z-index that is sufficient to make the dialog appear on top of
      other content in the sidebar, mainly the top bar. We will likely have to
      revise this if `confirm` is used in other contexts in future.
      c815849a
    • Robert Knight's avatar
      Convert existing `window.confirm` calls to use `confirm` prompt · f2352018
      Robert Knight authored
      Convert existing uses of `window.confirm` to use our own `confirm`
      utility which uses the standard Dialog from our pattern library.
      
      Using our own prompt avoids an immediate problem that browsers, starting
      with Chrome v91, are starting to block the use of `window.confirm` in
      third-party iframe. It also makes the visual design of the prompt match
      the rest of the application.
      f2352018
    • Robert Knight's avatar
    • Robert Knight's avatar
      Import Dialog component from LMS frontend · 3336dfc4
      Robert Knight authored
      Import the Dialog component from the LMS frontend, adjusted to avoid
      dependencies on some shared variables that were specific to that
      application.
      3336dfc4
    • Robert Knight's avatar
      Introduce types for route name and route params · f879ec7e
      Robert Knight authored
       - Introduce a union type for current route names
      
       - Use `Record<Key, Value>` rather than `Object.<Key, Value>` as it is
         stricter (eg. it is non-nullable)
      f879ec7e
    • Robert Knight's avatar
      Convert `router` service to a native ES class · 179271b7
      Robert Knight authored
      Services in `src/sidebar/services` are effectively classes. They are
      currently implemented with a convention that was more common pre-ES6 where
      the constructor is a function that creates the fields as local variables
      and the methods using closures and returns an object that references the
      closures.
      
      This pattern has advantages, especially pre-ES6, as it avoids issues with
      incorrect use of `this` and hides internal state from consumers. However
      it also has downsides:
      
       - It is less obvious to readers that they are looking at something that
         is logically a class
      
       - This is not an idiom we use elsewhere in the codebase, where we use
         native classes instead
      
       - Static analysis tools don't support this pattern for creating a class
         as well as they support a native class. For example `TS` creates a
         named type for native classes, which is convenient to reference
         in JSDoc comments
      
      This commit starts a process of refactoring service classes to ES
      classes which are named `<Thing>Service`, using the router service as a
      first example. Per recently agreed conventions, the classes are named
      rather than default exports.
      179271b7
  3. 14 Apr, 2021 1 commit
    • Kyle Keating's avatar
      Allow `group` to be passed to the merged config · da4d67b5
      Kyle Keating authored
      When the host config value for `requestConfigFromFrame` is an object containing a string and a number, it is assumed that the remaining host config is be fetched from the parent frame via RPC. In this context, any other values passed along via the iframe URL hash are ignored. One such value is the `group` which is used by the notebook to focus a group and filter only to that groups annotations.
      
      This changes the method that merges the configs to allow that `group` value to be taken from the iframe's url and incorporated into the final merged config so that the notebook can set that value in its own store.
      da4d67b5
  4. 13 Apr, 2021 6 commits
  5. 12 Apr, 2021 15 commits
  6. 09 Apr, 2021 1 commit