refactor: changes per PR feedback

app/assets/javascripts/components/Icon.tsx

@@ -11,42 +11,28 @@ import RestoreIcon from '../../icons/ic-restore.svg';
 import CloseIcon from '../../icons/ic-close.svg';
 import { toDirective } from './utils';
 
-export enum IconType {
-  PencilOff = 'pencil-off',
-  RichText = 'rich-text',
-  Trash = 'trash',
-  Pin = 'pin',
-  Unpin = 'unpin',
-  Archive = 'archive',
-  Unarchive = 'unarchive',
-  Hashtag = 'hashtag',
-  ChevronRight = 'chevron-right',
-  Restore = 'restore',
-  Close = 'close',
-}
-
 const ICONS = {
-  [IconType.PencilOff]: PencilOffIcon,
+  'pencil-off': PencilOffIcon,
-  [IconType.RichText]: RichTextIcon,
+  'rich-text': RichTextIcon,
-  [IconType.Trash]: TrashIcon,
+  'trash': TrashIcon,
-  [IconType.Pin]: PinIcon,
+  'pin': PinIcon,
-  [IconType.Unpin]: UnpinIcon,
+  'unpin': UnpinIcon,
-  [IconType.Archive]: ArchiveIcon,
+  'archive': ArchiveIcon,
-  [IconType.Unarchive]: UnarchiveIcon,
+  'unarchive': UnarchiveIcon,
-  [IconType.Hashtag]: HashtagIcon,
+  'hashtag': HashtagIcon,
-  [IconType.ChevronRight]: ChevronRightIcon,
+  'chevron-right': ChevronRightIcon,
-  [IconType.Restore]: RestoreIcon,
+  'restore': RestoreIcon,
-  [IconType.Close]: CloseIcon,
+  'close': CloseIcon,
 };
 
 type Props = {
-  type: IconType;
+  type: keyof (typeof ICONS);
   className: string;
 }
 
 export const Icon: React.FC<Props> = ({ type, className }) => {
   const IconComponent = ICONS[type];
-  return IconComponent ? <IconComponent className={className} /> : null;
+  return <IconComponent className={className} />;
 };
 
 export const IconDirective = toDirective<Props>(

app/assets/javascripts/components/NotesContextMenu.tsx

@@ -2,7 +2,7 @@ import { AppState } from '@/ui_models/app_state';
 import { toDirective, useCloseOnBlur } from './utils';
 import { observer } from 'mobx-react-lite';
 import { NotesOptions } from './NotesOptions';
-import { useEffect, useRef } from 'preact/hooks';
+import { useCallback, useEffect, useRef } from 'preact/hooks';
 
 type Props = {
   appState: AppState;
@@ -10,23 +10,23 @@ type Props = {
 
 const NotesContextMenu = observer(({ appState }: Props) => {
   const contextMenuRef = useRef<HTMLDivElement>();
-  const [closeOnBlur, setLockCloseOnBlur] = useCloseOnBlur(
+  const [closeOnBlur] = useCloseOnBlur(
     contextMenuRef,
     (open: boolean) => appState.notes.setContextMenuOpen(open)
   );
 
-  const closeOnClickOutside = (event: MouseEvent) => {
+  const closeOnClickOutside = useCallback((event: MouseEvent) => {
     if (!contextMenuRef.current?.contains(event.target as Node)) {
       appState.notes.setContextMenuOpen(false);
     }
-  };
+  }, [appState]);
 
   useEffect(() => {
     document.addEventListener('click', closeOnClickOutside);
     return () => {
       document.removeEventListener('click', closeOnClickOutside);
     };
-  });
+  }, [closeOnClickOutside]);
 
   return appState.notes.contextMenuOpen ? (
     <div

app/assets/javascripts/components/NotesOptions.tsx

@@ -1,5 +1,5 @@
 import { AppState } from '@/ui_models/app_state';
-import { Icon, IconType } from './Icon';
+import { Icon } from './Icon';
 import { Switch } from './Switch';
 import { observer } from 'mobx-react-lite';
 import { useRef, useState, useEffect } from 'preact/hooks';
@@ -15,7 +15,7 @@ type Props = {
   onSubmenuChange?: (submenuOpen: boolean) => void;
 };
 
-const MAX_TAGS_MENU_HEIGHT = 265;
+const MAX_TAGS_MENU_HEIGHT = 320;
 
 export const NotesOptions = observer(
   ({ appState, closeOnBlur, onSubmenuChange }: Props) => {
@@ -57,7 +57,7 @@ export const NotesOptions = observer(
           }}
         >
           <span className="flex items-center">
-            <Icon type={IconType.PencilOff} className={iconClass} />
+            <Icon type="pencil-off" className={iconClass} />
             Prevent editing
           </span>
         </Switch>
@@ -70,7 +70,7 @@ export const NotesOptions = observer(
           }}
         >
           <span className="flex items-center">
-            <Icon type={IconType.RichText} className={iconClass} />
+            <Icon type="rich-text" className={iconClass} />
             Show preview
           </span>
         </Switch>
@@ -102,11 +102,11 @@ export const NotesOptions = observer(
               className={`${buttonClass} py-1.5 justify-between`}
             >
               <div className="flex items-center">
-                <Icon type={IconType.Hashtag} className={iconClass} />
+                <Icon type="hashtag" className={iconClass} />
                 {'Add tag'}
               </div>
               <Icon
-                type={IconType.ChevronRight}
+                type="chevron-right"
                 className="fill-current color-neutral"
               />
             </DisclosureButton>
@@ -120,7 +120,7 @@ export const NotesOptions = observer(
               style={{
                 ...tagsMenuPosition,
               }}
-              className="sn-dropdown sn-dropdown-anchor-right flex flex-col py-2 max-w-265px max-h-80 overflow-y-scroll"
+              className="sn-dropdown sn-dropdown-anchor-right flex flex-col py-2 max-h-80 overflow-y-scroll"
             >
               {appState.tags.tags.map((tag) => (
                 <button
@@ -128,12 +128,12 @@ export const NotesOptions = observer(
                   className={`${buttonClass} py-2`}
                   onBlur={closeOnBlur}
                   onClick={() => {
-                    appState.tags.isTagInSelectedNotes(tag)
+                    appState.notes.isTagInSelectedNotes(tag)
-                      ? appState.tags.removeTagFromSelectedNotes(tag)
+                      ? appState.notes.removeTagFromSelectedNotes(tag)
-                      : appState.tags.addTagToSelectedNotes(tag);
+                      : appState.notes.addTagToSelectedNotes(tag);
                   }}
                 >
-                  <span className={appState.tags.isTagInSelectedNotes(tag) ? 'font-bold' : ''}>
+                  <span className={appState.notes.isTagInSelectedNotes(tag) ? 'font-bold' : ''}>
                     {tag.title}
                   </span>
                 </button>
@@ -149,7 +149,7 @@ export const NotesOptions = observer(
           }}
         >
           <Icon
-            type={pinned ? IconType.Unpin : IconType.Pin}
+            type={pinned ? 'unpin' : 'pin'}
             className={iconClass}
           />
           {appState.notes.selectedNotesCount > 1
@@ -168,7 +168,7 @@ export const NotesOptions = observer(
           }}
         >
           <Icon
-            type={archived ? IconType.Unarchive : IconType.Archive}
+            type={archived ? 'unarchive' : 'archive'}
             className={iconClass}
           />
           {archived ? 'Unarchive' : 'Archive'}
@@ -180,7 +180,7 @@ export const NotesOptions = observer(
             await appState.notes.setTrashSelectedNotes(!trashed);
           }}
         >
-          <Icon type={trashed ? IconType.Restore : IconType.Trash} className={iconClass} />
+          <Icon type={trashed ? 'restore' : 'trash'} className={iconClass} />
           {trashed ? 'Restore' : 'Move to Trash'}
         </button>
         {appState.selectedTag?.isTrashTag && (
@@ -191,7 +191,7 @@ export const NotesOptions = observer(
               await appState.notes.deleteNotesPermanently();
             }}
           >
-            <Icon type={IconType.Close} className="fill-current color-danger mr-2" />
+            <Icon type="close" className="fill-current color-danger mr-2" />
             <span className="color-danger">Delete Permanently</span>
           </button>
           )}

app/assets/javascripts/components/NotesOptionsPanel.tsx

@@ -23,7 +23,7 @@ export const NotesOptionsPanel = observer(({ appState }: Props) => {
   });
   const buttonRef = useRef<HTMLButtonElement>();
   const panelRef = useRef<HTMLDivElement>();
-  const [closeOnBlur, setLockCloseOnBlur] = useCloseOnBlur(panelRef, setOpen);
+  const [closeOnBlur] = useCloseOnBlur(panelRef, setOpen);
   const [submenuOpen, setSubmenuOpen] = useState(false);
 
   const onSubmenuChange = (open: boolean) => {
@@ -70,7 +70,7 @@ export const NotesOptionsPanel = observer(({ appState }: Props) => {
         style={{
           ...position,
         }}
-        className="sn-dropdown sn-dropdown-anchor-right flex flex-col py-2 max-w-265"
+        className="sn-dropdown sn-dropdown-anchor-right flex flex-col py-2"
       >
         {open && (
           <NotesOptions

app/assets/javascripts/ui_models/app_state/notes_state.ts

@@ -6,6 +6,7 @@ import {
   SNNote,
   NoteMutator,
   ContentType,
+  SNTag,
 } from '@standardnotes/snjs';
 import {
   makeObservable,
@@ -14,7 +15,6 @@ import {
   computed,
   runInAction,
 } from 'mobx';
-import { RefObject } from 'preact';
 import { WebApplication } from '../application';
 import { Editor } from '../editor';
 
@@ -46,6 +46,9 @@ export class NotesState {
       setPinSelectedNotes: action,
       setTrashSelectedNotes: action,
       unselectNotes: action,
+      addTagToSelectedNotes: action,
+      removeTagFromSelectedNotes: action,
+      isTagInSelectedNotes: action,
     });
 
     appEventListeners.push(
@@ -204,10 +207,8 @@ export class NotesState {
         },
         false
       );
-      runInAction(() => {
+      this.unselectNotes();
-        this.selectedNotes = {};
+      this.contextMenuOpen = false;
-        this.contextMenuOpen = false;
-      });
     }
   }
 
@@ -297,6 +298,42 @@ export class NotesState {
     this.selectedNotes = {};
   }
 
+  async addTagToSelectedNotes(tag: SNTag): Promise<void> {
+    const selectedNotes = Object.values(
+      this.application.getAppState().notes.selectedNotes
+    );
+    await this.application.changeItem(tag.uuid, (mutator) => {
+      for (const note of selectedNotes) {
+        mutator.addItemAsRelationship(note);
+      }
+    });
+    this.application.sync();
+  }
+
+  async removeTagFromSelectedNotes(tag: SNTag): Promise<void> {
+    const selectedNotes = Object.values(
+      this.application.getAppState().notes.selectedNotes
+    );
+    await this.application.changeItem(tag.uuid, (mutator) => {
+      for (const note of selectedNotes) {
+        mutator.removeItemAsRelationship(note);
+      }
+    });
+    this.application.sync();
+  }
+
+  isTagInSelectedNotes(tag: SNTag): boolean {
+    const selectedNotes = Object.values(
+      this.application.getAppState().notes.selectedNotes
+    );
+    return selectedNotes.every((note) =>
+      this.application
+        .getAppState()
+        .getNoteTags(note)
+        .find((noteTag) => noteTag.uuid === tag.uuid)
+    );
+  }
+
   private get io() {
     return this.application.io;
   }

app/assets/javascripts/ui_models/app_state/tags_state.ts

@@ -1,5 +1,5 @@
 import { ContentType, SNSmartTag, SNTag } from '@standardnotes/snjs';
-import { action, computed, makeObservable, observable } from 'mobx';
+import { computed, makeObservable, observable } from 'mobx';
 import { WebApplication } from '../application';
 
 export class TagsState {
@@ -15,10 +15,6 @@ export class TagsState {
       smartTags: observable,
 
       tagsCount: computed,
-
-      addTagToSelectedNotes: action,
-      removeTagFromSelectedNotes: action,
-      isTagInSelectedNotes: action,
     });
 
     appEventListeners.push(
@@ -34,45 +30,6 @@ export class TagsState {
     );
   }
 
-  async addTagToSelectedNotes(tag: SNTag): Promise<void> {
-    const selectedNotes = Object.values(
-      this.application.getAppState().notes.selectedNotes
-    );
-    await this.application.changeItem(tag.uuid, (mutator) => {
-      for (const note of selectedNotes) {
-        mutator.addItemAsRelationship(note);
-      }
-    });
-    this.application.sync();
-  }
-
-  async removeTagFromSelectedNotes(tag: SNTag): Promise<void> {
-    const selectedNotes = Object.values(
-      this.application.getAppState().notes.selectedNotes
-    );
-    await Promise.all(
-      selectedNotes.map(
-        async (note) =>
-          await this.application.changeItem(tag.uuid, (mutator) => {
-            mutator.removeItemAsRelationship(note);
-          })
-      )
-    );
-    this.application.sync();
-  }
-
-  isTagInSelectedNotes(tag: SNTag): boolean {
-    const selectedNotes = Object.values(
-      this.application.getAppState().notes.selectedNotes
-    );
-    return selectedNotes.every((note) =>
-      this.application
-        .getAppState()
-        .getNoteTags(note)
-        .find((noteTag) => noteTag.uuid === tag.uuid)
-    );
-  }
-
   get tagsCount(): number {
     return this.tags.length;
   }

app/assets/stylesheets/_sn.scss

@@ -107,10 +107,6 @@
   max-width: 15rem;
 }
 
-.max-w-265px {
-  max-width: 265px;
-}
-
 .h-32px {
   height: 32px;
 }