Improve typechecking

group-list-item.js
group-list-section.js
group-list.js

src/sidebar/components/group-list-item.js

@@ -8,11 +8,28 @@ import { withServices } from '../util/service-context';
 
 import MenuItem from './menu-item';
 
+/**
+ * @typedef {import('../../types/api').Group} Group
+ */
+
+/**
+ * @typedef GroupListItemProps
+ * @prop {Group} group
+ * @prop {boolean} [isExpanded] - Whether the submenu for this group is expanded
+ * @prop {(expand: boolean) => any} [onExpand] -
+ *   Callback invoked to expand or collapse the current group
+ * @prop {Object} analytics - Injected service
+ * @prop {Object} groups - Injected service
+ * @prop {Object} toastMessenger - Injected service
+ */
+
 /**
  * An item in the groups selection menu.
  *
  * The item has a primary action which selects the group, along with a set of
  * secondary actions accessible via a toggle menu.
+ *
+ * @param {GroupListItemProps} props
  */
 function GroupListItem({
   analytics,
@@ -24,7 +41,8 @@ function GroupListItem({
 }) {
   const activityUrl = group.links.html;
   const hasActionMenu = activityUrl || group.canLeave;
-  const isSelectable = !group.scopes.enforced || group.isScopedToUri;
+  const isSelectable =
+    (group.scopes && !group.scopes.enforced) || group.isScopedToUri;
 
   const focusedGroupId = useStore(store => store.focusedGroupId());
   const isSelected = group.id === focusedGroupId;
@@ -134,20 +152,8 @@ function GroupListItem({
 
 GroupListItem.propTypes = {
   group: propTypes.object.isRequired,
-
-  /**
-   * Whether the submenu for this group is expanded.
-   */
   isExpanded: propTypes.bool,
-
-  /**
-   * Callback invoked to expand or collapse the current group.
-   *
-   * @type {(expand: boolean) => any}
-   */
   onExpand: propTypes.func,
-
-  // Injected services.
   analytics: propTypes.object.isRequired,
   groups: propTypes.object.isRequired,
   toastMessenger: propTypes.object.isRequired,

src/sidebar/components/group-list-section.js

@@ -4,8 +4,25 @@ import propTypes from 'prop-types';
 import GroupListItem from './group-list-item';
 import MenuSection from './menu-section';
 
+/**
+ * @typedef {import('../../types/api').Group} Group
+ */
+
+/**
+ * @typedef GroupListSectionProps
+ * @prop {Group|null} [expandedGroup]
+ *  - The `Group` whose submenu is currently expanded, or `null` if no group is currently expanded
+ * @prop {Group[]} [groups] - The list of groups to be displayed in the group list section
+ * @prop {string} [heading] - The string name of the group list section
+ * @prop {(group: Group|null) => any} [onExpandGroup] -
+ *   Callback invoked when a group is expanded or collapsed.  The argument is the group being
+ *   expanded, or `null` if the expanded group is being collapsed.
+ */
+
 /**
  * A labeled section of the groups list.
+ *
+ * @param {GroupListSectionProps} props
  */
 export default function GroupListSection({
   expandedGroup,
@@ -28,22 +45,8 @@ export default function GroupListSection({
 }
 
 GroupListSection.propTypes = {
-  /**
-   * The `Group` whose submenu is currently expanded, or `null` if no group
-   * is currently expanded.
-   */
   expandedGroup: propTypes.object,
-  /* The list of groups to be displayed in the group list section. */
   groups: propTypes.arrayOf(propTypes.object),
-  /* The string name of the group list section. */
   heading: propTypes.string,
-  /**
-   * Callback invoked when a group is expanded or collapsed.
-   *
-   * The argument is the group being expanded, or `null` if the expanded group
-   * is being collapsed.
-   *
-   * @type {(group: Group|null) => any}
-   */
   onExpandGroup: propTypes.func,
 };

src/sidebar/components/group-list.js

@@ -14,6 +14,12 @@ import GroupListSection from './group-list-section';
 import Menu from './menu';
 import MenuItem from './menu-item';
 
+/**
+ * @typedef {import('../../types/config').MergedConfig} MergedConfig
+ * @typedef {import('../../types/api').Group} Group
+ * @typedef {import('../services/service-url').ServiceUrlGetter} ServiceUrlGetter
+ */
+
 /**
  * Return the custom icon for the top bar configured by the publisher in
  * the Hypothesis client configuration.
@@ -23,9 +29,17 @@ function publisherProvidedIcon(settings) {
   return svc && svc.icon ? svc.icon : null;
 }
 
+/**
+ * @typedef GroupListProps
+ * @prop {ServiceUrlGetter} [serviceUrl]
+ * @prop {MergedConfig} [settings]
+ */
+
 /**
  * Menu allowing the user to select which group to show and also access
  * additional actions related to groups.
+ *
+ * @param {GroupListProps} props
  */
 function GroupList({ serviceUrl, settings }) {
   const currentGroups = useStore(store => store.getCurrentlyViewingGroups());
@@ -57,12 +71,14 @@ function GroupList({ serviceUrl, settings }) {
   //
   // nb. If we create other menus that behave similarly in future, we may want
   // to move this state to the `Menu` component.
-  const [expandedGroup, setExpandedGroup] = useState(null);
+  const [expandedGroup, setExpandedGroup] = useState(
+    /** @type {Group|null} */ (null)
+  );
 
   let label;
   if (focusedGroup) {
     const icon =
-      focusedGroup.organization.logo || publisherProvidedIcon(settings);
+      focusedGroup.organization.logo || publisherProvidedIcon(settings) || '';
 
     // If org name is missing, then treat this icon like decoration
     // and pass an empty string.
@@ -70,11 +86,7 @@ function GroupList({ serviceUrl, settings }) {
     label = (
       <span className="group-list__menu-label">
         {icon && (
-          <img
-            className="group-list__menu-icon"
-            src={icon || publisherProvidedIcon(settings)}
-            alt={altName}
-          />
+          <img className="group-list__menu-icon" src={icon} alt={altName} />
         )}
         {focusedGroup.name}
       </span>
@@ -131,12 +143,7 @@ function GroupList({ serviceUrl, settings }) {
       )}
 
       {canCreateNewGroup && (
-        <MenuItem
-          icon="add"
-          href={newGroupLink}
-          label="New private group"
-          style="shaded"
-        />
+        <MenuItem icon="add" href={newGroupLink} label="New private group" />
       )}
     </Menu>
   );

src/sidebar/components/menu-item.js

@@ -33,7 +33,7 @@ import Slider from './slider';
  * @prop {boolean} [isSubmenuItem] -
  *   True if this item is part of a submenu, in which case it is rendered with a different
  *   style (shaded background)
- * @prop {boolean} [isSubmenuVisible] -
+ * @prop {boolean|undefined} [isSubmenuVisible] -
  *   If present, display a button to toggle the sub-menu associated with this item and
  *   indicate the current state; `true` if the submenu is visible. Note. Omit this prop,
  *    or set it to null, if there is no `submenu`.
@@ -220,10 +220,10 @@ export default function MenuItem({
     <Fragment>
       {menuItem}
       {hasSubmenuVisible && (
-        <Slider visible={isSubmenuVisible}>
+        <Slider visible={/** @type {boolean} */ (isSubmenuVisible)}>
           <MenuKeyboardNavigation
             closeMenu={onCloseSubmenu}
-            visible={isSubmenuVisible}
+            visible={/** @type {boolean} */ (isSubmenuVisible)}
             className="menu-item__submenu"
           >
             {submenu}

src/sidebar/components/test/group-list-item-test.js

@@ -312,6 +312,16 @@ describe('GroupListItem', () => {
     });
   });
 
+  it('disables menu item and shows note in submenu if `group.scopes` is missing', () => {
+    fakeGroup.scopes = undefined;
+    const wrapper = createGroupListItem(fakeGroup, {
+      isExpanded: true,
+    });
+    assert.equal(wrapper.find('MenuItem').first().prop('isDisabled'), true);
+    const submenu = getSubmenu(wrapper);
+    assert.equal(submenu.exists('.group-list-item__footer'), true);
+  });
+
   [
     {
       groupType: 'private',

src/sidebar/store/modules/groups.js

@@ -211,6 +211,10 @@ const getCurrentlyViewingGroups = createSelector(
  * @prop {() => Group[]} getFeaturedGroups
  * @prop {(id: string) => Group|undefined} getGroup
  * @prop {() => Group[]} getInScopeGroups
+ *
+ * // Root selectors
+ * @prop {() => Group[]} getCurrentlyViewingGroups,
+ * @prop {() => Group[]} getMyGroups,
  */
 
 export default {

src/tsconfig.json

@@ -30,9 +30,6 @@
     "sidebar/components/annotation-viewer-content.js",
     "sidebar/components/annotation.js",
     "sidebar/components/filter-status.js",
-    "sidebar/components/group-list-item.js",
-    "sidebar/components/group-list-section.js",
-    "sidebar/components/group-list.js",
     "sidebar/components/help-panel.js",
     "sidebar/components/hypothesis-app.js",
     "sidebar/components/share-annotations-panel.js",

src/types/api.js

@@ -37,8 +37,8 @@
  * @typedef Target
  * @prop {string} source
  * @prop {Selector[]} [selector]
- * 
- 
+ *
+
 /**
  * TODO - Fill out remaining properties
  *
@@ -96,6 +96,8 @@
  * @typedef Organization
  * @prop {string} name
  * @prop {string} logo
+ * @prop {string} id
+ * @prop {boolean} [default]
  */
 
 /**
@@ -113,6 +115,8 @@
  * @prop {Organization} organization - nb. This field is nullable in the API, but
  *   we assign a default organization on the client.
  * @prop {GroupScopes|null} scopes
+ * @prop {Object} links
+ *   @prop {string} links.html
  *
  * // Properties not present on API objects, but added by utilities in the client.
  * @prop {string} logo

src/types/config.js

@@ -11,6 +11,7 @@
  * @prop {string} apiUrl
  * @prop {string} authority
  * @prop {string} grantToken
+ * @prop {string} [icon]
  * @prop {string[]|Promise<string[]>} [groups] -
  *   List of groups to show. The embedder can specify an array. In the sidebar
  *   this may be converted to a Promise if this information is fetched asynchronously.