Move `useElementShouldClose` and `normalizeKeyName` into frontend-shared package

`useElementShouldClose` is a useful generic hook for implementing non-modal dialogs. We
also use it in the LMS frontend, so it makes sense to share the implementation.

This function depends on `normalizeKeyName`, so I moved that as well.

`useElementShouldClose` also depended on a `listen` helper. This helper
is not currently used by any other code so I moved it into
`use-element-should-close.js` as a non-exported helper function and
simplified it.

In the process I found it was necessary to change the gulp task that
builds the frontend-shared/lib/ directory to handle subdirectories under
frontend-shared/src/.

src/sidebar/components/hooks/test/use-element-should-close-test.js → frontend-shared/src/hooks/test/use-element-should-close-test.js

@@ -3,7 +3,7 @@ import { createElement } from 'preact';
 import { useRef } from 'preact/hooks';
 import { act } from 'preact/test-utils';
 
-import useElementShouldClose from '../use-element-should-close';
+import { useElementShouldClose } from '../use-element-should-close';
 
 describe('useElementShouldClose', () => {
   let handleClose;

src/sidebar/components/hooks/use-element-should-close.js → frontend-shared/src/hooks/use-element-should-close.js

 import { useEffect } from 'preact/hooks';
 
-import { normalizeKeyName } from '../../../shared/browser-compatibility-utils';
-import { listen } from '../../util/dom';
+import { normalizeKeyName } from '../browser-compatibility-utils';
+
+/**
+ * Attach listeners for one or multiple events to an element and return a
+ * function that removes the listeners.
+ *
+ * @param {HTMLElement} element
+ * @param {string[]} events
+ * @param {EventListener} listener
+ * @param {Object} options
+ *   @param {boolean} [options.useCapture]
+ * @return {() => void} Function which removes the event listeners.
+ */
+function listen(element, events, listener, { useCapture = false } = {}) {
+  events.forEach(event =>
+    element.addEventListener(event, listener, useCapture)
+  );
+  return () => {
+    events.forEach(event =>
+      element.removeEventListener(event, listener, useCapture)
+    );
+  };
+}
 
 /**
  * @template T
@@ -22,11 +43,7 @@ import { listen } from '../../util/dom';
  * @param {boolean} isOpen - Whether the popup is currently visible/open
  * @param {() => void} handleClose - Callback invoked to close the popup
  */
-export default function useElementShouldClose(
-  closeableEl,
-  isOpen,
-  handleClose
-) {
+export function useElementShouldClose(closeableEl, isOpen, handleClose) {
   useEffect(() => {
     if (!isOpen) {
       return () => {};
@@ -44,7 +61,7 @@ export default function useElementShouldClose(
     // (key press, programmatic focus change).
     const removeFocusListener = listen(
       document.body,
-      'focus',
+      ['focus'],
       event => {
         if (!closeableEl.current.contains(/** @type {Node} */ (event.target))) {
           handleClose();

frontend-shared/src/index.js

-module.exports = {
-  ...require('./svg-icon'),
-};
+// Components
+export { SvgIcon, registerIcons } from './svg-icon';
+
+// Hooks
+export { useElementShouldClose } from './hooks/use-element-should-close';
+
+// Utilities
+export { normalizeKeyName } from './browser-compatibility-utils';

scripts/gulp/frontend-shared.js

@@ -15,7 +15,7 @@ const buildFrontendSharedJs = () => {
 
   return (
     gulp
-      .src('frontend-shared/src/*')
+      .src('frontend-shared/src/**/*.js')
       // Transpile the js source files and write the output in the frontend-shared/lib dir.
       // Additionally, add the sourcemaps into the same dir.
       .pipe(sourcemaps.init())

src/shared/shortcut.js

-import { useEffect } from 'preact/hooks';
+import { normalizeKeyName } from '@hypothesis/frontend-shared';
 
-import { normalizeKeyName } from './browser-compatibility-utils.js';
+import { useEffect } from 'preact/hooks';
 
 // Bit flags indicating modifiers required by a shortcut or pressed in a key event.
 const modifiers = {

src/sidebar/components/AnnotationEditor.js

+import { normalizeKeyName } from '@hypothesis/frontend-shared';
 import { createElement } from 'preact';
 import { useState } from 'preact/hooks';
 import propTypes from 'prop-types';
 
-import { normalizeKeyName } from '../../shared/browser-compatibility-utils';
 import { withServices } from '../service-context';
 import { applyTheme } from '../helpers/theme';
 import { useStoreProxy } from '../store/use-store';

src/sidebar/components/AnnotationShareControl.js

-import { SvgIcon } from '@hypothesis/frontend-shared';
+import { SvgIcon, useElementShouldClose } from '@hypothesis/frontend-shared';
 import { createElement } from 'preact';
 import { useEffect, useRef, useState } from 'preact/hooks';
 import propTypes from 'prop-types';
@@ -11,7 +11,6 @@ import { isIOS } from '../../shared/user-agent';
 
 import Button from './Button';
 import ShareLinks from './ShareLinks';
-import useElementShouldClose from './hooks/use-element-should-close';
 
 /**
  * @typedef {import('../../types/api').Annotation} Annotation

src/sidebar/components/MarkdownEditor.js

 import classnames from 'classnames';
-import { SvgIcon } from '@hypothesis/frontend-shared';
+import { SvgIcon, normalizeKeyName } from '@hypothesis/frontend-shared';
 import { createElement, createRef } from 'preact';
 import { useEffect, useMemo, useRef, useState } from 'preact/hooks';
 import propTypes from 'prop-types';
@@ -10,7 +10,6 @@ import {
   toggleBlockStyle,
   toggleSpanStyle,
 } from '../markdown-commands';
-import { normalizeKeyName } from '../../shared/browser-compatibility-utils';
 import { isMacOS } from '../../shared/user-agent';
 
 import MarkdownView from './MarkdownView';

src/sidebar/components/Menu.js

 import classnames from 'classnames';
-import { SvgIcon } from '@hypothesis/frontend-shared';
+import {
+  SvgIcon,
+  normalizeKeyName,
+  useElementShouldClose,
+} from '@hypothesis/frontend-shared';
 import { Fragment, createElement } from 'preact';
 import { useCallback, useEffect, useRef, useState } from 'preact/hooks';
 import propTypes from 'prop-types';
 
-import useElementShouldClose from './hooks/use-element-should-close';
-import { normalizeKeyName } from '../../shared/browser-compatibility-utils';
-
 import MenuKeyboardNavigation from './MenuKeyboardNavigation';
 
 // The triangular indicator below the menu toggle button that visually links it

src/sidebar/components/MenuItem.js

 import classnames from 'classnames';
-import { SvgIcon } from '@hypothesis/frontend-shared';
+import { SvgIcon, normalizeKeyName } from '@hypothesis/frontend-shared';
 import { Fragment, createElement } from 'preact';
 import { useEffect, useRef } from 'preact/hooks';
 import propTypes from 'prop-types';
 
-import { normalizeKeyName } from '../../shared/browser-compatibility-utils';
-
 import MenuKeyboardNavigation from './MenuKeyboardNavigation';
 import Slider from './Slider';
 

src/sidebar/components/MenuKeyboardNavigation.js

+import { normalizeKeyName } from '@hypothesis/frontend-shared';
 import { createElement } from 'preact';
 import { useEffect, useRef } from 'preact/hooks';
 import propTypes from 'prop-types';
 
-import { normalizeKeyName } from '../../shared/browser-compatibility-utils';
-
 function isElementVisible(element) {
   return element.offsetParent !== null;
 }

src/sidebar/components/TagEditor.js

-import { SvgIcon } from '@hypothesis/frontend-shared';
+import {
+  SvgIcon,
+  normalizeKeyName,
+  useElementShouldClose,
+} from '@hypothesis/frontend-shared';
 import { createElement } from 'preact';
 import { useRef, useState } from 'preact/hooks';
 import propTypes from 'prop-types';
@@ -6,8 +10,6 @@ import propTypes from 'prop-types';
 import { withServices } from '../service-context';
 
 import AutocompleteList from './AutocompleteList';
-import { normalizeKeyName } from '../../shared/browser-compatibility-utils';
-import useElementShouldClose from './hooks/use-element-should-close';
 
 /** @typedef {import("preact").JSX.Element} JSXElement */
 

src/sidebar/components/test/AnnotationShareControl-test.js

@@ -89,10 +89,12 @@ describe('AnnotationShareControl', () => {
 
     $imports.$mock(mockImportedComponents());
     $imports.$mock({
+      '@hypothesis/frontend-shared': {
+        useElementShouldClose: sinon.stub(),
+      },
       '../helpers/annotation-sharing': { isShareableURI: fakeIsShareableURI },
       '../util/copy-to-clipboard': fakeCopyToClipboard,
       '../helpers/permissions': { isPrivate: fakeIsPrivate },
-      './hooks/use-element-should-close': sinon.stub(),
       '../../shared/user-agent': { isIOS: fakeIsIOS },
     });
   });

src/sidebar/util/dom.js

@@ -22,28 +22,3 @@ export function getElementHeightWithMargins(element) {
 
   return elementHeight + marginHeight;
 }
-
-/**
- * Attach listeners for one or multiple events to an element and return a
- * function that removes the listeners.
- *
- * @param {HTMLElement} element
- * @param {string[]|string} events
- * @param {EventListener} listener
- * @param {Object} options
- *   @param {boolean} [options.useCapture]
- * @return {function} Function which removes the event listeners.
- */
-export function listen(element, events, listener, { useCapture = false } = {}) {
-  if (!Array.isArray(events)) {
-    events = [events];
-  }
-  events.forEach(event =>
-    element.addEventListener(event, listener, useCapture)
-  );
-  return () => {
-    /** @type {string[]} */ (events).forEach(event =>
-      element.removeEventListener(event, listener, useCapture)
-    );
-  };
-}

src/sidebar/util/test/dom-test.js

-import { listen, getElementHeightWithMargins } from '../dom';
+import { getElementHeightWithMargins } from '../dom';
 
 describe('sidebar/util/dom', () => {
   describe('getElementHeightWithMargins', () => {
@@ -23,60 +23,4 @@ describe('sidebar/util/dom', () => {
       assert.equal(getElementHeightWithMargins(testElement), 470);
     });
   });
-
-  describe('listen', () => {
-    const createFakeElement = () => ({
-      addEventListener: sinon.stub(),
-      removeEventListener: sinon.stub(),
-    });
-    [true, false].forEach(useCapture => {
-      it('adds listeners for specified events', () => {
-        const element = createFakeElement();
-        const handler = sinon.stub();
-
-        listen(element, ['click', 'mousedown'], handler, { useCapture });
-
-        assert.calledWith(
-          element.addEventListener,
-          'click',
-          handler,
-          useCapture
-        );
-        assert.calledWith(
-          element.addEventListener,
-          'mousedown',
-          handler,
-          useCapture
-        );
-      });
-    });
-
-    [true, false].forEach(useCapture => {
-      it('removes listeners when returned function is invoked', () => {
-        const element = createFakeElement();
-        const handler = sinon.stub();
-
-        const removeListeners = listen(
-          element,
-          ['click', 'mousedown'],
-          handler,
-          { useCapture }
-        );
-        removeListeners();
-
-        assert.calledWith(
-          element.removeEventListener,
-          'click',
-          handler,
-          useCapture
-        );
-        assert.calledWith(
-          element.removeEventListener,
-          'mousedown',
-          handler,
-          useCapture
-        );
-      });
-    });
-  });
 });