Update group forms header and footer

Align the header and footer of the group forms more closely with the mocks in https://www.figma.com/design/jon1U01LGSLcx7PWtZ9TPZ/Hypothesis---Group-Management?node-id=2942-1928&node-type=frame&t=NPDV4Y62CUMkfEpq-0. - Add "View group activity" link at the top of the page and remove the "Back to group overview" link from the bottom - Vertically align the page title with the Settings / Members tabs on the right - Add a divider below the title / tabs line, mirroring the one on the bottom of the form
hypothesis · Dec 12, 2024 · fb09be7 · fb09be7
1 parent 9bfe5e5
commit fb09be7
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 40 deletions.
diff --git a/h/static/scripts/group-forms/components/CreateEditGroupForm.tsx b/h/static/scripts/group-forms/components/CreateEditGroupForm.tsx
@@ -235,10 +235,12 @@ export default function CreateEditGroupForm({
   };
 
   return (
-    <FormContainer title={heading} classes="max-w-[530px] mx-auto">
-      {group && config.features.group_members && (
-        <GroupFormHeader group={group} />
-      )}
+    <FormContainer classes="max-w-[530px] mx-auto">
+      <GroupFormHeader
+        group={group}
+        title={heading}
+        enableMembers={config.features.group_members}
+      />
       <form onSubmit={onSubmit} data-testid="form">
         <TextField
           type="input"
@@ -291,7 +293,12 @@ export default function CreateEditGroupForm({
           </>
         )}
 
-        <div className="flex items-center gap-x-4 mt-2">
+        <div className="mt-2 pt-2 border-t border-t-text-grey-6 flex items-center gap-x-4">
+          <span>
+            {/* These are in a child span to avoid a gap between them. */}
+            <Star />
+            &nbsp;Required
+          </span>
           <ErrorNotice message={errorMessage} />
           <div className="grow" />
           <SaveStateIcon
@@ -321,19 +328,6 @@ export default function CreateEditGroupForm({
           }}
         />
       )}
-
-      <footer className="mt-14 pt-4 border-t border-t-text-grey-6">
-        <div className="flex">
-          {group && (
-            <a href={group.link} data-testid="back-link">
-              ← Back to group overview page
-            </a>
-          )}
-          <div className="grow" />
-          <Star />
-          &nbsp;Required
-        </div>
-      </footer>
     </FormContainer>
   );
 }
diff --git a/h/static/scripts/group-forms/components/EditGroupMembersForm.tsx b/h/static/scripts/group-forms/components/EditGroupMembersForm.tsx
@@ -380,8 +380,8 @@ export default function EditGroupMembersForm({
 
   return (
     <>
-      <FormContainer title="Edit group members">
-        <GroupFormHeader group={group} />
+      <FormContainer>
+        <GroupFormHeader title="Edit group members" group={group} />
         <ErrorNotice message={errorMessage} />
         <div className="w-full">
           <Scroll>

diff --git a/h/static/scripts/group-forms/components/GroupFormHeader.tsx b/h/static/scripts/group-forms/components/GroupFormHeader.tsx
@@ -1,3 +1,4 @@
+import { Link } from '@hypothesis/frontend-shared';
 import classnames from 'classnames';
 import type { ComponentChildren } from 'preact';
 import { Link as RouterLink, useRoute } from 'wouter-preact';
@@ -8,9 +9,10 @@ import { routes } from '../routes';
 type TabLinkProps = {
   href: string;
   children: ComponentChildren;
+  testId?: string;
 };
 
-function TabLink({ children, href }: TabLinkProps) {
+function TabLink({ children, href, testId }: TabLinkProps) {
   const [selected] = useRoute(href);
   return (
     <RouterLink
@@ -20,29 +22,63 @@ function TabLink({ children, href }: TabLinkProps) {
           true,
         'text-grey-1 bg-grey-7': selected,
       })}
+      data-testid={testId}
     >
       {children}
     </RouterLink>
   );
 }
 
 export type GroupFormHeaderProps = {
-  group: Group;
+  group: Group | null;
+  title: string;
+  enableMembers?: boolean;
 };
 
-export default function GroupFormHeader({ group }: GroupFormHeaderProps) {
+export default function GroupFormHeader({
+  enableMembers = true,
+  group,
+  title,
+}: GroupFormHeaderProps) {
   // This should be replaced with a proper URL generation function that handles
   // escaping etc.
-  const editLink = routes.groups.edit.replace(':pubid', group.pubid);
-  const editMembersLinks = routes.groups.editMembers.replace(
-    ':pubid',
-    group.pubid,
-  );
+  const editLink = group
+    ? routes.groups.edit.replace(':pubid', group.pubid)
+    : null;
+  const editMembersLinks = group
+    ? routes.groups.editMembers.replace(':pubid', group.pubid)
+    : null;
 
   return (
-    <div className="flex gap-x-2 justify-end py-2">
-      <TabLink href={editLink}>Settings</TabLink>
-      <TabLink href={editMembersLinks}>Members</TabLink>
+    <div className="mb-4 pb-1 border-b border-b-text-grey-6">
+      {group && (
+        <div className="mb-4">
+          <Link
+            classes="text-grey-7"
+            href={group.link}
+            underline="always"
+            data-testid="activity-link"
+          >
+            View group activity
+          </Link>
+        </div>
+      )}
+      <div className="flex gap-x-2 py-2 items-center">
+        <h1 className="text-grey-7 text-xl/none" data-testid="header">
+          {title}
+        </h1>
+        <div className="grow" />
+        {enableMembers && editLink && (
+          <TabLink testId="settings-link" href={editLink}>
+            Settings
+          </TabLink>
+        )}
+        {enableMembers && editMembersLinks && (
+          <TabLink testId="members-link" href={editMembersLinks}>
+            Members
+          </TabLink>
+        )}
+      </div>
     </div>
   );
 }
diff --git a/h/static/scripts/group-forms/components/forms/FormContainer.tsx b/h/static/scripts/group-forms/components/forms/FormContainer.tsx
@@ -2,7 +2,6 @@ import classnames from 'classnames';
 import type { ComponentChildren } from 'preact';
 
 export type FormContainerProps = {
-  title: string;
   children: ComponentChildren;
   classes?: string;
 };
@@ -11,13 +10,9 @@ export type FormContainerProps = {
 export default function FormContainer({
   children,
   classes,
-  title,
 }: FormContainerProps) {
   return (
     <div className={classnames('text-grey-6 text-sm/relaxed', classes)}>
-      <h1 className="mt-14 mb-8 text-grey-7 text-xl/none" data-testid="header">
-        {title}
-      </h1>
       {children}
     </div>
   );

diff --git a/h/static/scripts/group-forms/components/test/CreateEditGroupForm-test.js b/h/static/scripts/group-forms/components/test/CreateEditGroupForm-test.js
@@ -78,7 +78,7 @@ describe('CreateEditGroupForm', () => {
 
   const getElements = wrapper => {
     return {
-      header: wrapper.find('[data-testid="header"]'),
+      header: wrapper.find('GroupFormHeader'),
       nameField: wrapper.find('TextField[label="Name"]'),
       descriptionField: wrapper.find('TextField[label="Description"]'),
       submitButton: wrapper.find('button[data-testid="button"]'),
@@ -111,7 +111,7 @@ describe('CreateEditGroupForm', () => {
       const { wrapper, elements } = createWrapper();
       const { header, nameField, descriptionField, submitButton } = elements;
 
-      assert.equal(header.text(), heading);
+      assert.equal(header.prop('title'), heading);
       assert.equal(nameField.prop('value'), '');
       assert.equal(descriptionField.prop('value'), '');
       assert.equal(submitButton.text(), 'Create group');
@@ -245,12 +245,11 @@ describe('CreateEditGroupForm', () => {
       const { wrapper, elements } = createWrapper({ group });
       const { header, nameField, descriptionField, submitButton } = elements;
 
-      assert.equal(header.text(), 'Edit group');
+      assert.equal(header.prop('title'), 'Edit group');
       assert.equal(nameField.prop('value'), group.name);
       assert.equal(descriptionField.prop('value'), group.description);
       assert.equal(getSelectedGroupType(wrapper), group.type);
       assert.equal(submitButton.text(), 'Save changes');
-      assert.isTrue(wrapper.exists('[data-testid="back-link"]'));
       assert.isFalse(wrapper.exists('[data-testid="error-message"]'));
       await assertInLoadingState(wrapper, false);
       assert.isFalse(savedConfirmationShowing(wrapper));

diff --git a/h/static/scripts/group-forms/components/test/GroupFormHeader-test.js b/h/static/scripts/group-forms/components/test/GroupFormHeader-test.js
@@ -0,0 +1,46 @@
+import { mount } from '@hypothesis/frontend-testing';
+
+import GroupFormHeader from '../GroupFormHeader';
+
+describe('GroupFormHeader', () => {
+  const createHeader = (props = {}) => mount(<GroupFormHeader {...props} />);
+
+  const group = {
+    pubid: 'abc123',
+    link: 'https://anno.co/groups/abc123',
+  };
+
+  const getLink = (wrapper, name) => wrapper.find(`a[data-testid="${name}"]`);
+
+  it('renders title', () => {
+    const header = createHeader({ title: 'Create a new group' });
+    assert.equal(header.find('h1').text(), 'Create a new group');
+  });
+
+  it('does not show activity link or tabs if no group', () => {
+    const header = createHeader();
+    assert.isFalse(getLink(header, 'activity-link').exists());
+    assert.isFalse(getLink(header, 'settings-link').exists());
+    assert.isFalse(getLink(header, 'members-link').exists());
+  });
+
+  it('renders group activity link and tabs if there is a group', () => {
+    const header = createHeader({ group });
+
+    const activityLink = getLink(header, 'activity-link');
+    assert.equal(activityLink.prop('href'), group.link);
+
+    const settingsLink = getLink(header, 'settings-link');
+    assert.equal(settingsLink.prop('href'), '/groups/abc123/edit');
+
+    const membersLink = getLink(header, 'members-link');
+    assert.equal(membersLink.prop('href'), '/groups/abc123/edit/members');
+  });
+
+  it('does not show tabs if the members flag is disabled', () => {
+    const header = createHeader({ group, enableMembers: false });
+    assert.isTrue(getLink(header, 'activity-link').exists());
+    assert.isFalse(getLink(header, 'settings-link').exists());
+    assert.isFalse(getLink(header, 'members-link').exists());
+  });
+});