Only allow one group's submenu to be expanded at a time

Replace the per-GroupListItem state indicating whether the item's submenu is
open with a per-GroupList state variable indicating which group (or none) has
its submenu currently expanded.

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

@@ -2,7 +2,6 @@
 
 const propTypes = require('prop-types');
 const { Fragment, createElement } = require('preact');
-const { useState } = require('preact/hooks');
 
 const useStore = require('../store/use-store');
 const { orgName } = require('../util/group-list-item-common');
@@ -19,19 +18,17 @@ const MenuItem = require('./menu-item');
  */
 function GroupListItem({
   analytics,
-  defaultSubmenuOpen = false,
+  isExpanded,
   flash,
   group,
   groups: groupsService,
+  onExpand,
 }) {
   const canLeaveGroup = group.type === 'private';
   const activityUrl = group.links.html;
   const hasActionMenu = activityUrl || canLeaveGroup;
   const isSelectable = !group.scopes.enforced || group.isScopedToUri;
 
-  const [isExpanded, setExpanded] = useState(
-    hasActionMenu ? defaultSubmenuOpen : undefined
-  );
   const focusedGroupId = useStore(store => store.focusedGroupId());
   const isSelected = group.id === focusedGroupId;
 
@@ -63,7 +60,7 @@ function GroupListItem({
     // TODO - Fix this more cleanly in `MenuItem`.
     event.preventDefault();
 
-    setExpanded(!isExpanded);
+    onExpand(!isExpanded);
   };
 
   const copyLink = () => {
@@ -79,7 +76,7 @@ function GroupListItem({
     group.type === 'private' ? 'Copy invite link' : 'Copy activity link';
 
   // Close the submenu when any clicks happen which close the top-level menu.
-  const collapseSubmenu = () => setExpanded(false);
+  const collapseSubmenu = () => onExpand(false);
 
   return (
     <Fragment>
@@ -87,7 +84,7 @@ function GroupListItem({
         icon={group.logo || 'blank'}
         iconAlt={orgName(group)}
         isDisabled={!isSelectable}
-        isExpanded={isExpanded}
+        isExpanded={hasActionMenu ? isExpanded : undefined}
         isSelected={isSelected}
         isSubmenuVisible={isExpanded}
         label={group.name}
@@ -142,8 +139,17 @@ function GroupListItem({
 GroupListItem.propTypes = {
   group: propTypes.object.isRequired,
 
-  /** Whether the submenu is open when the item is initially rendered. */
-  defaultSubmenuOpen: propTypes.bool,
+  /**
+   * 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,

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

@@ -9,21 +9,39 @@ const MenuSection = require('./menu-section');
 /**
  * A labeled section of the groups list.
  */
-function GroupListSection({ groups, heading }) {
+function GroupListSection({ expandedGroup, onExpandGroup, groups, heading }) {
   return (
     <MenuSection heading={heading}>
       {groups.map(group => (
-        <GroupListItem key={group.id} group={group} />
+        <GroupListItem
+          key={group.id}
+          isExpanded={group === expandedGroup}
+          onExpand={expanded => onExpandGroup(expanded ? group : null)}
+          group={group}
+        />
       ))}
     </MenuSection>
   );
 }
 
 GroupListSection.propTypes = {
+  /**
+   * The group whose submenu is currently expanded, if any.
+   */
+  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,
 };
 
 module.exports = GroupListSection;

src/sidebar/components/group-list.js

 'use strict';
 
 const { createElement } = require('preact');
-const { useMemo } = require('preact/hooks');
+const { useMemo, useState } = require('preact/hooks');
 const propTypes = require('prop-types');
 
 const isThirdPartyService = require('../util/is-third-party-service');
@@ -53,6 +53,13 @@ function GroupList({ serviceUrl, settings }) {
   const canCreateNewGroup = userid && !isThirdPartyUser(userid, authDomain);
   const newGroupLink = serviceUrl('groups.new');
 
+  // The group whose submenu is currently open, or `null` if no group item is
+  // currently expanded.
+  //
+  // 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);
+
   let label;
   if (focusedGroup) {
     const icon = focusedGroup.organization.logo;
@@ -88,18 +95,27 @@ function GroupList({ serviceUrl, settings }) {
     >
       {currentGroupsSorted.length > 0 && (
         <GroupListSection
+          expandedGroup={expandedGroup}
+          onExpandGroup={setExpandedGroup}
           heading="Currently Viewing"
           groups={currentGroupsSorted}
         />
       )}
       {featuredGroupsSorted.length > 0 && (
         <GroupListSection
+          expandedGroup={expandedGroup}
+          onExpandGroup={setExpandedGroup}
           heading="Featured Groups"
           groups={featuredGroupsSorted}
         />
       )}
       {myGroupsSorted.length > 0 && (
-        <GroupListSection heading="My Groups" groups={myGroupsSorted} />
+        <GroupListSection
+          expandedGroup={expandedGroup}
+          onExpandGroup={setExpandedGroup}
+          heading="My Groups"
+          groups={myGroupsSorted}
+        />
       )}
 
       {canCreateNewGroup && (

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

@@ -179,8 +179,29 @@ describe('GroupListItem', () => {
     });
   });
 
+  it('expands submenu if `isExpanded` is `true`', () => {
+    const wrapper = createGroupListItem(fakeGroup, { isExpanded: true });
+    assert.isTrue(
+      wrapper
+        .find('MenuItem')
+        .first()
+        .prop('isExpanded')
+    );
+  });
+
+  it('collapses submenu if `isExpanded` is `false`', () => {
+    const wrapper = createGroupListItem(fakeGroup, { isExpanded: false });
+    assert.isFalse(
+      wrapper
+        .find('MenuItem')
+        .first()
+        .prop('isExpanded')
+    );
+  });
+
   it('toggles submenu when toggle is clicked', () => {
-    const wrapper = createGroupListItem(fakeGroup);
+    const onExpand = sinon.stub();
+    const wrapper = createGroupListItem(fakeGroup, { onExpand });
     const toggleSubmenu = () => {
       const dummyEvent = new Event();
       act(() => {
@@ -194,9 +215,12 @@ describe('GroupListItem', () => {
     };
 
     toggleSubmenu();
-    assert.isTrue(wrapper.exists('ul'));
+    assert.calledWith(onExpand, true);
+    onExpand.resetHistory();
+
+    wrapper.setProps({ isExpanded: true });
     toggleSubmenu();
-    assert.isFalse(wrapper.exists('ul'));
+    assert.calledWith(onExpand, false);
   });
 
   it('does not show submenu toggle if there are no available actions', () => {
@@ -209,14 +233,14 @@ describe('GroupListItem', () => {
   it('does not show link to activity page if not available', () => {
     fakeGroup.links.html = null;
     const wrapper = createGroupListItem(fakeGroup, {
-      defaultSubmenuOpen: true,
+      isExpanded: true,
     });
     assert.isFalse(wrapper.exists('MenuItem[label="View group activity"]'));
   });
 
   it('shows link to activity page if available', () => {
     const wrapper = createGroupListItem(fakeGroup, {
-      defaultSubmenuOpen: true,
+      isExpanded: true,
     });
     assert.isTrue(wrapper.exists('MenuItem[label="View group activity"]'));
   });
@@ -224,7 +248,7 @@ describe('GroupListItem', () => {
   it('does not show "Leave" action if user cannot leave', () => {
     fakeGroup.type = 'open';
     const wrapper = createGroupListItem(fakeGroup, {
-      defaultSubmenuOpen: true,
+      isExpanded: true,
     });
     assert.isFalse(wrapper.exists('MenuItem[label="Leave group"]'));
   });
@@ -232,14 +256,14 @@ describe('GroupListItem', () => {
   it('shows "Leave" action if user can leave', () => {
     fakeGroup.type = 'private';
     const wrapper = createGroupListItem(fakeGroup, {
-      defaultSubmenuOpen: true,
+      isExpanded: true,
     });
     assert.isTrue(wrapper.exists('MenuItem[label="Leave group"]'));
   });
 
   it('prompts to leave group if "Leave" action is clicked', () => {
     const wrapper = createGroupListItem(fakeGroup, {
-      defaultSubmenuOpen: true,
+      isExpanded: true,
     });
     clickMenuItem(wrapper, 'Leave group');
     assert.called(window.confirm);
@@ -248,7 +272,7 @@ describe('GroupListItem', () => {
 
   it('leaves group if "Leave" is clicked and user confirms', () => {
     const wrapper = createGroupListItem(fakeGroup, {
-      defaultSubmenuOpen: true,
+      isExpanded: true,
     });
     window.confirm.returns(true);
     clickMenuItem(wrapper, 'Leave group');
@@ -277,7 +301,7 @@ describe('GroupListItem', () => {
       fakeGroup.scopes.enforced = enforced;
       fakeGroup.isScopedToUri = isScopedToUri;
       const wrapper = createGroupListItem(fakeGroup, {
-        defaultSubmenuOpen: true,
+        isExpanded: true,
       });
       assert.equal(
         wrapper
@@ -316,7 +340,7 @@ describe('GroupListItem', () => {
       fakeGroup.type = groupType;
       fakeGroup.links.html = hasLink ? 'https://anno.co/groups/1' : null;
       const wrapper = createGroupListItem(fakeGroup, {
-        defaultSubmenuOpen: true,
+        isExpanded: true,
       });
       const copyAction = wrapper
         .find('MenuItem')
@@ -332,7 +356,7 @@ describe('GroupListItem', () => {
 
   it('copies activity URL if "Copy link" action is clicked', () => {
     const wrapper = createGroupListItem(fakeGroup, {
-      defaultSubmenuOpen: true,
+      isExpanded: true,
     });
     clickMenuItem(wrapper, 'Copy invite link');
     assert.calledWith(fakeCopyText, 'https://annotate.com/groups/groupid');
@@ -342,7 +366,7 @@ describe('GroupListItem', () => {
   it('reports an error if "Copy link" action fails', () => {
     fakeCopyText.throws(new Error('Something went wrong'));
     const wrapper = createGroupListItem(fakeGroup, {
-      defaultSubmenuOpen: true,
+      isExpanded: true,
     });
     clickMenuItem(wrapper, 'Copy invite link');
     assert.calledWith(fakeCopyText, 'https://annotate.com/groups/groupid');

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

@@ -22,8 +22,11 @@ describe('GroupListSection', () => {
   const createGroupListSection = ({
     groups = testGroups,
     heading = 'Test section',
+    ...props
   } = {}) => {
-    return shallow(<GroupListSection groups={groups} heading={heading} />);
+    return shallow(
+      <GroupListSection groups={groups} heading={heading} {...props} />
+    );
   };
 
   it('renders heading', () => {
@@ -35,4 +38,39 @@ describe('GroupListSection', () => {
     const wrapper = createGroupListSection();
     assert.equal(wrapper.find(GroupListItem).length, testGroups.length);
   });
+
+  it('expands group specified by `expandedGroup` prop', () => {
+    const wrapper = createGroupListSection();
+    for (let i = 0; i < testGroups.length; i++) {
+      wrapper.setProps({ expandedGroup: testGroups[i] });
+      wrapper.find(GroupListItem).forEach((n, idx) => {
+        assert.equal(n.prop('isExpanded'), idx === i);
+      });
+    }
+  });
+
+  it("sets expanded group when a group's submenu is expanded", () => {
+    const onExpandGroup = sinon.stub();
+    const wrapper = createGroupListSection({ onExpandGroup });
+    wrapper
+      .find(GroupListItem)
+      .first()
+      .props()
+      .onExpand(true);
+    assert.calledWith(onExpandGroup, testGroups[0]);
+  });
+
+  it("resets expanded group when group's submenu is collapsed", () => {
+    const onExpandGroup = sinon.stub();
+    const wrapper = createGroupListSection({
+      expandedGroup: testGroups[0],
+      onExpandGroup,
+    });
+    wrapper
+      .find(GroupListItem)
+      .first()
+      .props()
+      .onExpand(false);
+    assert.calledWith(onExpandGroup, null);
+  });
 });

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

@@ -2,6 +2,7 @@
 
 const { shallow } = require('enzyme');
 const { createElement } = require('preact');
+const { act } = require('preact/test-utils');
 
 const GroupList = require('../group-list');
 
@@ -23,6 +24,27 @@ describe('GroupList', () => {
     ).dive();
   }
 
+  /**
+   * Configure the store to populate all of the group sections.
+   * Must be called before group list is rendered.
+   */
+  function populateGroupSections() {
+    const testGroups = [
+      {
+        ...testGroup,
+        id: 'zzz',
+      },
+      {
+        ...testGroup,
+        id: 'aaa',
+      },
+    ];
+    fakeStore.getMyGroups.returns(testGroups);
+    fakeStore.getCurrentlyViewingGroups.returns(testGroups);
+    fakeStore.getFeaturedGroups.returns(testGroups);
+    return testGroups;
+  }
+
   beforeEach(() => {
     fakeServiceUrl = sinon.stub();
     fakeSettings = {
@@ -75,20 +97,7 @@ describe('GroupList', () => {
   });
 
   it('sorts groups within each section by organization', () => {
-    const testGroups = [
-      {
-        ...testGroup,
-        id: 'zzz',
-      },
-      {
-        ...testGroup,
-        id: 'aaa',
-      },
-    ];
-    fakeStore.getMyGroups.returns(testGroups);
-    fakeStore.getCurrentlyViewingGroups.returns(testGroups);
-    fakeStore.getFeaturedGroups.returns(testGroups);
-
+    const testGroups = populateGroupSections();
     const fakeGroupOrganizations = groups =>
       groups.sort((a, b) => a.id.localeCompare(b.id));
     GroupList.$imports.$mock({
@@ -163,4 +172,36 @@ describe('GroupList', () => {
     const img = shallow(label).find('img');
     assert.equal(img.prop('src'), 'test-icon');
   });
+
+  it('sets or resets expanded group item', () => {
+    const testGroups = populateGroupSections();
+
+    // Render group list. Initially no submenu should be expanded.
+    const wrapper = createGroupList();
+    const verifyGroupIsExpanded = group =>
+      wrapper.find('GroupListSection').forEach(section => {
+        assert.equal(section.prop('expandedGroup'), group);
+      });
+    verifyGroupIsExpanded(null);
+
+    // Expand a group in one of the sections.
+    act(() => {
+      wrapper
+        .find('GroupListSection')
+        .first()
+        .prop('onExpandGroup')(testGroups[0]);
+    });
+    wrapper.update();
+    verifyGroupIsExpanded(testGroups[0]);
+
+    // Reset expanded group.
+    act(() => {
+      wrapper
+        .find('GroupListSection')
+        .first()
+        .prop('onExpandGroup')(null);
+    });
+    wrapper.update();
+    verifyGroupIsExpanded(null);
+  });
 });