Pull callbacks and state up into higher-level components

lyzadanger · lyzadanger · commit 1cfb11c33d85 · 2022-03-23T07:40:25.000-04:00
Make `AnnotationPublishControl` more of a controlled component by
pulling callbacks and state up to `AnnotationEditor`.

Provide a `draft` prop from `Annotation` to `AnnotationEditor`.
diff --git a/src/sidebar/components/Annotation/Annotation.js b/src/sidebar/components/Annotation/Annotation.js
@@ -49,10 +49,12 @@ function Annotation({
 
   const store = useStoreProxy();
 
+  const draft = annotation && store.getDraft(annotation);
+
   const hasQuote = annotation && !!quote(annotation);
   const isFocused = annotation && store.isAnnotationFocused(annotation.$tag);
   const isSaving = annotation && store.isSavingAnnotation(annotation);
-  const isEditing = annotation && !!store.getDraft(annotation) && !isSaving;
+  const isEditing = annotation && !!draft && !isSaving;
 
   const userid = store.profile().userid;
   const showActions = !isSaving && !isEditing;
@@ -88,7 +90,9 @@ function Annotation({
             <AnnotationBody annotation={annotation} />
           )}
 
-          {isEditing && <AnnotationEditor annotation={annotation} />}
+          {isEditing && (
+            <AnnotationEditor annotation={annotation} draft={draft} />
+          )}
         </>
       )}
 
diff --git a/src/sidebar/components/Annotation/AnnotationEditor.js b/src/sidebar/components/Annotation/AnnotationEditor.js
@@ -1,7 +1,8 @@
 import { normalizeKeyName } from '@hypothesis/frontend-shared';
-import { useState } from 'preact/hooks';
+import { useCallback, useState } from 'preact/hooks';
 
 import { withServices } from '../../service-context';
+import { isReply, isSaved } from '../../helpers/annotation-metadata';
 import { applyTheme } from '../../helpers/theme';
 import { useStoreProxy } from '../../store/use-store';
 
@@ -13,12 +14,14 @@ import AnnotationPublishControl from './AnnotationPublishControl';
 
 /**
  * @typedef {import("../../../types/api").Annotation} Annotation
+ * @typedef {import("../../store/modules/drafts").Draft} Draft
  * @typedef {import("../../../types/config").SidebarSettings} SidebarSettings
  */
 
 /**
  * @typedef AnnotationEditorProps
  * @prop {Annotation} annotation - The annotation under edit
+ * @prop {Draft} draft - The annotation's draft
  * @prop {import('../../services/annotations').AnnotationsService} annotationsService
  * @prop {SidebarSettings} settings - Injected service
  * @prop {import('../../services/toast-messenger').ToastMessengerService} toastMessenger
@@ -32,6 +35,7 @@ import AnnotationPublishControl from './AnnotationPublishControl';
  */
 function AnnotationEditor({
   annotation,
+  draft,
   annotationsService,
   settings,
   tags: tagsService,
@@ -43,63 +47,83 @@ function AnnotationEditor({
   );
 
   const store = useStoreProxy();
-  const draft = store.getDraft(annotation);
   const group = store.getGroup(annotation.group);
 
-  if (!draft) {
-    // If there's no draft, we can't be in editing mode
-    return null;
-  }
-
   const shouldShowLicense =
     !draft.isPrivate && group && group.type !== 'private';
 
   const tags = draft.tags;
   const text = draft.text;
   const isEmpty = !text && !tags.length;
 
-  const onEditTags = ({ tags }) => {
-    store.createDraft(draft.annotation, { ...draft, tags });
-  };
+  const onEditTags = useCallback(
+    ({ tags }) => {
+      store.createDraft(draft.annotation, { ...draft, tags });
+    },
+    [draft, store]
+  );
 
   /**
    * Verify `newTag` has content and is not a duplicate; add the tag
    *
    * @param {string} newTag
    * @return {boolean} - `true` if tag is added
    */
-  const onAddTag = newTag => {
-    if (!newTag || tags.indexOf(newTag) >= 0) {
-      // don't add empty or duplicate tags
-      return false;
-    }
-    const tagList = [...tags, newTag];
-    // Update the tag locally for the suggested-tag list
-    tagsService.store(tagList);
-    onEditTags({ tags: tagList });
-    return true;
-  };
+  const onAddTag = useCallback(
+    newTag => {
+      if (!newTag || tags.indexOf(newTag) >= 0) {
+        // don't add empty or duplicate tags
+        return false;
+      }
+      const tagList = [...tags, newTag];
+      // Update the tag locally for the suggested-tag list
+      tagsService.store(tagList);
+      onEditTags({ tags: tagList });
+      return true;
+    },
+    [onEditTags, tags, tagsService]
+  );
 
   /**
    * Remove a tag from the annotation.
    *
    * @param {string} tag
    * @return {boolean} - `true` if tag extant and removed
    */
-  const onRemoveTag = tag => {
-    const newTagList = [...tags]; // make a copy
-    const index = newTagList.indexOf(tag);
-    if (index >= 0) {
-      newTagList.splice(index, 1);
-      onEditTags({ tags: newTagList });
-      return true;
-    }
-    return false;
-  };
+  const onRemoveTag = useCallback(
+    tag => {
+      const newTagList = [...tags]; // make a copy
+      const index = newTagList.indexOf(tag);
+      if (index >= 0) {
+        newTagList.splice(index, 1);
+        onEditTags({ tags: newTagList });
+        return true;
+      }
+      return false;
+    },
+    [onEditTags, tags]
+  );
 
-  const onEditText = ({ text }) => {
-    store.createDraft(draft.annotation, { ...draft, text });
-  };
+  const onEditText = useCallback(
+    ({ text }) => {
+      store.createDraft(draft.annotation, { ...draft, text });
+    },
+    [draft, store]
+  );
+
+  /**
+   * @param {boolean} isPrivate
+   */
+  const onSetPrivacy = useCallback(
+    isPrivate => {
+      store.createDraft(annotation, { ...draft, isPrivate });
+      // Persist this as privacy default for future annotations unless this is a reply
+      if (!isReply(annotation)) {
+        store.setDefault('annotationPrivacy', isPrivate ? 'private' : 'shared');
+      }
+    },
+    [annotation, draft, store]
+  );
 
   const onSave = async () => {
     // If there is any content in the tag editor input field that has
@@ -115,6 +139,14 @@ function AnnotationEditor({
     }
   };
 
+  // Revert changes to this annotation
+  const onCancel = useCallback(() => {
+    store.removeDraft(annotation);
+    if (!isSaved(annotation)) {
+      store.removeAnnotations([annotation]);
+    }
+  }, [annotation, store]);
+
   // Allow saving of annotation by pressing CMD/CTRL-Enter
   /** @param {KeyboardEvent} event */
   const onKeyDown = event => {
@@ -150,11 +182,16 @@ function AnnotationEditor({
         tagList={tags}
       />
       <div className="hyp-u-layout-row annotation__form-actions">
-        <AnnotationPublishControl
-          annotation={annotation}
-          isDisabled={isEmpty}
-          onSave={onSave}
-        />
+        {group && (
+          <AnnotationPublishControl
+            group={group}
+            isDisabled={isEmpty}
+            isPrivate={draft.isPrivate}
+            onCancel={onCancel}
+            onSave={onSave}
+            onSetPrivacy={onSetPrivacy}
+          />
+        )}
       </div>
       {shouldShowLicense && <AnnotationLicense />}
     </div>
diff --git a/src/sidebar/components/Annotation/AnnotationPublishControl.js b/src/sidebar/components/Annotation/AnnotationPublishControl.js
@@ -1,26 +1,26 @@
 import { Icon, LabeledButton } from '@hypothesis/frontend-shared';
 import classnames from 'classnames';
 
-import { useStoreProxy } from '../../store/use-store';
-import { isNew, isReply } from '../../helpers/annotation-metadata';
-import { isShared } from '../../helpers/permissions';
 import { withServices } from '../../service-context';
 import { applyTheme } from '../../helpers/theme';
 
 import Menu from '../Menu';
 import MenuItem from '../MenuItem';
 
 /**
- * @typedef {import('../../../types/api').Annotation} Annotation
+ * @typedef {import('../../../types/api').Group} Group
  * @typedef {import('../../../types/config').SidebarSettings} SidebarSettings
  */
 
 /**
  * @typedef AnnotationPublishControlProps
- * @prop {Annotation} annotation
+ * @prop {Group} group - The group this annotation or draft would publish to
  * @prop {boolean} [isDisabled]
  *  - Should the save button be disabled? Hint: it will be if the annotation has no content
- * @prop {() => any} onSave - Callback for save button click
+ * @prop {boolean} isPrivate - Annotation or draft is "Only Me"
+ * @prop {() => void} onCancel - Callback for cancel button click
+ * @prop {() => void} onSave - Callback for save button click
+ * @prop {(isPrivate: boolean) => void} onSetPrivacy - Callback for save button click
  * @prop {SidebarSettings} settings - Injected service
  */
 
@@ -32,41 +32,14 @@ import MenuItem from '../MenuItem';
  * @param {AnnotationPublishControlProps} props
  */
 function AnnotationPublishControl({
-  annotation,
+  group,
   isDisabled,
+  isPrivate,
+  onCancel,
   onSave,
+  onSetPrivacy,
   settings,
 }) {
-  const store = useStoreProxy();
-  const draft = store.getDraft(annotation);
-  const group = store.getGroup(annotation.group);
-
-  if (!group) {
-    // If there is no group, then don't render anything as a missing group
-    // may mean the group is not loaded yet.
-    return null;
-  }
-
-  const isPrivate = draft ? draft.isPrivate : !isShared(annotation.permissions);
-
-  const publishDestination = isPrivate ? 'Only Me' : group.name;
-
-  // Revert changes to this annotation
-  const onCancel = () => {
-    store.removeDraft(annotation);
-    if (isNew(annotation)) {
-      store.removeAnnotations([annotation]);
-    }
-  };
-
-  const onSetPrivacy = level => {
-    store.createDraft(annotation, { ...draft, isPrivate: level === 'private' });
-    // Persist this as privacy default for future annotations unless this is a reply
-    if (!isReply(annotation)) {
-      store.setDefault('annotationPrivacy', level);
-    }
-  };
-
   const buttonStyle = applyTheme(
     ['ctaTextColor', 'ctaBackgroundColor'],
     settings
@@ -93,7 +66,7 @@ function AnnotationPublishControl({
           size="large"
           variant="primary"
         >
-          Post to {publishDestination}
+          Post to {isPrivate ? 'Only Me' : group.name}
         </LabeledButton>
         {/* This wrapper div is necessary because of peculiarities with
              Safari: see https://github.com/hypothesis/client/issues/2302 */}
@@ -125,13 +98,13 @@ function AnnotationPublishControl({
               icon={group.type === 'open' ? 'public' : 'groups'}
               label={group.name}
               isSelected={!isPrivate}
-              onClick={() => onSetPrivacy('shared')}
+              onClick={() => onSetPrivacy(false)}
             />
             <MenuItem
               icon="lock"
               label="Only Me"
               isSelected={isPrivate}
-              onClick={() => onSetPrivacy('private')}
+              onClick={() => onSetPrivacy(true)}
             />
           </Menu>
         </div>
