chore: Remove focused comment state from router (#9962)

* chore: Refactor comment state from router

* Handle edge cases

* refactor

* feedback

app/actions/definitions/comments.tsx

@@ -3,7 +3,6 @@ import { toast } from "sonner";
 import Comment from "~/models/Comment";
 import CommentDeleteDialog from "~/components/CommentDeleteDialog";
 import ViewReactionsDialog from "~/components/Reactions/ViewReactionsDialog";
-import history from "~/utils/history";
 import { createActionV2 } from "..";
 import { ActiveDocumentSection } from "../sections";
 
@@ -50,16 +49,6 @@ export const resolveCommentFactory = ({
       stores.policies.abilities(comment.documentId).update,
     perform: async ({ t }) => {
       await comment.resolve();
-
-      const locationState = history.location.state as Record<string, unknown>;
-      history.replace({
-        ...history.location,
-        state: {
-          sidebarContext: locationState["sidebarContext"],
-          commentId: undefined,
-        },
-      });
-
       onResolve();
       toast.success(t("Thread resolved"));
     },
@@ -82,16 +71,6 @@ export const unresolveCommentFactory = ({
       stores.policies.abilities(comment.documentId).update,
     perform: async () => {
       await comment.unresolve();
-
-      const locationState = history.location.state as Record<string, unknown>;
-      history.replace({
-        ...history.location,
-        state: {
-          sidebarContext: locationState["sidebarContext"],
-          commentId: undefined,
-        },
-      });
-
       onUnresolve();
     },
   });

app/components/DocumentContext.tsx

@@ -11,9 +11,15 @@ class DocumentContext {
   /** The editor instance for this document */
   editor?: Editor;
 
+  /** The ID of the currently focused comment, or null if no comment is focused */
+  @observable
+  focusedCommentId: string | null = null;
+
+  /** Whether the editor has been initialized */
   @observable
   isEditorInitialized: boolean = false;
 
+  /** The headings in the document */
   @observable
   headings: Heading[] = [];
 
@@ -39,6 +45,11 @@ class DocumentContext {
     this.isEditorInitialized = initialized;
   };
 
+  @action
+  setFocusedCommentId = (commentId: string | null) => {
+    this.focusedCommentId = commentId;
+  };
+
   @action
   updateState = () => {
     this.updateHeadings();

app/hooks/useFocusedComment.ts

@@ -1,13 +1,44 @@
-import { useLocation } from "react-router-dom";
 import useQuery from "~/hooks/useQuery";
 import useStores from "./useStores";
+import { useDocumentContext } from "~/components/DocumentContext";
+import { useEffect } from "react";
+import { useHistory } from "react-router-dom";
 
-export default function useFocusedComment() {
+/**
+ * Custom hook to retrieve the currently focused comment in a document.
+ * It checks both the document context and the query string for the comment ID.
+ * If a comment is focused, it returns the comment itself or the parent thread if it exists
+ */
+export function useFocusedComment() {
   const { comments } = useStores();
-  const location = useLocation<{ commentId?: string }>();
+  const context = useDocumentContext();
   const query = useQuery();
-  const focusedCommentId = location.state?.commentId || query.get("commentId");
+  const focusedCommentId = context.focusedCommentId || query.get("commentId");
   const comment = focusedCommentId ? comments.get(focusedCommentId) : undefined;
+  const history = useHistory();
+
+  // Move the query string into context
+  useEffect(() => {
+    if (focusedCommentId && context.focusedCommentId !== focusedCommentId) {
+      context.setFocusedCommentId(focusedCommentId);
+    }
+  }, [focusedCommentId, context]);
+
+  // Clear query string from location
+  useEffect(() => {
+    if (focusedCommentId) {
+      const params = new URLSearchParams(history.location.search);
+
+      if (params.get("commentId") === focusedCommentId) {
+        params.delete("commentId");
+        history.replace({
+          pathname: history.location.pathname,
+          search: params.toString(),
+          state: history.location.state,
+        });
+      }
+    }
+  }, [focusedCommentId, history]);
 
   return comment?.parentCommentId
     ? comments.get(comment.parentCommentId)

app/scenes/Document/components/CommentSortMenu.tsx

@@ -1,23 +1,22 @@
-import queryString from "query-string";
 import React from "react";
 import { useTranslation } from "react-i18next";
-import { useHistory, useLocation } from "react-router-dom";
 import styled from "styled-components";
 import { s } from "@shared/styles";
 import { UserPreference } from "@shared/types";
 import { InputSelect, Option } from "~/components/InputSelect";
 import useCurrentUser from "~/hooks/useCurrentUser";
-import { useLocationSidebarContext } from "~/hooks/useLocationSidebarContext";
-import useQuery from "~/hooks/useQuery";
 import { CommentSortType } from "~/types";
 
-const CommentSortMenu = () => {
+type Props = {
+  /** Callback when the sort type changes */
+  onChange?: (sortType: CommentSortType | "resolved") => void;
+  /** Whether resolved comments are being viewed */
+  viewingResolved?: boolean;
+};
+
+const CommentSortMenu = ({ viewingResolved, onChange }: Props) => {
   const { t } = useTranslation();
-  const location = useLocation();
-  const sidebarContext = useLocationSidebarContext();
-  const history = useHistory();
   const user = useCurrentUser();
-  const params = useQuery();
 
   const preferredSortType = user.getPreference(
     UserPreference.SortCommentsByOrderInDocument
@@ -25,42 +24,23 @@ const CommentSortMenu = () => {
     ? CommentSortType.OrderInDocument
     : CommentSortType.MostRecent;
 
-  const viewingResolved = params.get("resolved") === "";
   const value = viewingResolved ? "resolved" : preferredSortType;
 
   const handleChange = React.useCallback(
-    (val: string) => {
-      if (val === "resolved") {
-        history.push({
-          search: queryString.stringify({
-            ...queryString.parse(location.search),
-            resolved: "",
-          }),
-          pathname: location.pathname,
-          state: { sidebarContext },
-        });
-        return;
+    (val: CommentSortType | "resolved") => {
+      if (val !== "resolved") {
+        if (val !== preferredSortType) {
+          user.setPreference(
+            UserPreference.SortCommentsByOrderInDocument,
+            val === CommentSortType.OrderInDocument
+          );
+          void user.save();
+        }
       }
 
-      const sortType = val as CommentSortType;
-      if (sortType !== preferredSortType) {
-        user.setPreference(
-          UserPreference.SortCommentsByOrderInDocument,
-          sortType === CommentSortType.OrderInDocument
-        );
-        void user.save();
-      }
-
-      history.push({
-        search: queryString.stringify({
-          ...queryString.parse(location.search),
-          resolved: undefined,
-        }),
-        pathname: location.pathname,
-        state: { sidebarContext },
-      });
+      onChange?.(val);
     },
-    [history, location, sidebarContext, user, preferredSortType]
+    [user, onChange, preferredSortType]
   );
 
   const options: Option[] = React.useMemo(

app/scenes/Document/components/CommentThread.tsx

@@ -2,7 +2,6 @@ import { observer } from "mobx-react";
 import { darken } from "polished";
 import * as React from "react";
 import { useTranslation } from "react-i18next";
-import { useHistory, useLocation } from "react-router-dom";
 import scrollIntoView from "scroll-into-view-if-needed";
 import styled, { css } from "styled-components";
 import breakpoint from "styled-components-breakpoint";
@@ -17,7 +16,6 @@ import Facepile from "~/components/Facepile";
 import Fade from "~/components/Fade";
 import { ResizingHeightContainer } from "~/components/ResizingHeightContainer";
 import useBoolean from "~/hooks/useBoolean";
-import { useLocationSidebarContext } from "~/hooks/useLocationSidebarContext";
 import useOnClickOutside from "~/hooks/useOnClickOutside";
 import usePersistedState from "~/hooks/usePersistedState";
 import usePolicy from "~/hooks/usePolicy";
@@ -51,14 +49,11 @@ function CommentThread({
   collapseNumDisplayed = 3,
 }: Props) {
   const [scrollOnMount] = React.useState(focused && !window.location.hash);
-  const { editor } = useDocumentContext();
+  const { editor, setFocusedCommentId } = useDocumentContext();
   const { comments } = useStores();
   const topRef = React.useRef<HTMLDivElement>(null);
   const replyRef = React.useRef<HTMLDivElement>(null);
   const { t } = useTranslation();
-  const history = useHistory();
-  const location = useLocation();
-  const sidebarContext = useLocationSidebarContext();
   const [autoFocus, setAutoFocusOn, setAutoFocusOff] = useBoolean(thread.isNew);
   const user = useCurrentUser();
 
@@ -102,14 +97,7 @@ function CommentThread({
       !(event.target as HTMLElement).classList.contains("comment") &&
       event.defaultPrevented === false
     ) {
-      history.replace({
-        search: location.search,
-        pathname: location.pathname,
-        state: {
-          commentId: undefined,
-          sidebarContext,
-        },
-      });
+      setFocusedCommentId(null);
     }
   });
 
@@ -118,15 +106,7 @@ function CommentThread({
   }, [editor, thread.id]);
 
   const handleClickThread = () => {
-    history.replace({
-      // Clear any commentId from the URL when explicitly focusing a thread
-      search: thread.isResolved ? "resolved=" : "",
-      pathname: location.pathname.replace(/\/history$/, ""),
-      state: {
-        commentId: thread.id,
-        sidebarContext,
-      },
-    });
+    setFocusedCommentId(thread.id);
   };
 
   const handleClickExpand = (ev: React.SyntheticEvent) => {

app/scenes/Document/components/CommentThreadItem.tsx

@@ -30,6 +30,7 @@ import useCurrentUser from "~/hooks/useCurrentUser";
 import CommentMenu from "~/menus/CommentMenu";
 import CommentEditor from "./CommentEditor";
 import { HighlightedText } from "./HighlightText";
+import { useDocumentContext } from "~/components/DocumentContext";
 
 /**
  * Hook to calculate if we should display a timestamp on a comment
@@ -111,6 +112,7 @@ function CommentThreadItem({
   onEditStart,
   onEditEnd,
 }: Props) {
+  const { setFocusedCommentId } = useDocumentContext();
   const { t } = useTranslation();
   const user = useCurrentUser();
   const [data, setData] = React.useState(comment.data);
@@ -154,6 +156,9 @@ function CommentThreadItem({
   const handleUpdate = React.useCallback(
     (attrs: { resolved: boolean }) => {
       onUpdate?.(comment.id, attrs);
+      if ("resolved" in attrs) {
+        setFocusedCommentId(null);
+      }
     },
     [comment.id, onUpdate]
   );

app/scenes/Document/components/Comments.tsx

@@ -13,7 +13,7 @@ import Fade from "~/components/Fade";
 import Flex from "~/components/Flex";
 import Scrollable from "~/components/Scrollable";
 import useCurrentUser from "~/hooks/useCurrentUser";
-import useFocusedComment from "~/hooks/useFocusedComment";
+import { useFocusedComment } from "~/hooks/useFocusedComment";
 import useKeyDown from "~/hooks/useKeyDown";
 import usePersistedState from "~/hooks/usePersistedState";
 import usePolicy from "~/hooks/usePolicy";
@@ -31,11 +31,13 @@ function Comments() {
   const { editor, isEditorInitialized } = useDocumentContext();
   const { t } = useTranslation();
   const match = useRouteMatch<{ documentSlug: string }>();
-  const params = useQuery();
   const document = documents.get(match.params.documentSlug);
   const focusedComment = useFocusedComment();
   const can = usePolicy(document);
-
+  const query = useQuery();
+  const [viewingResolved, setViewingResolved] = useState(
+    query.get("resolved") !== null || focusedComment?.isResolved || false
+  );
   const scrollableRef = useRef<HTMLDivElement | null>(null);
   const prevThreadCount = useRef(0);
   const isAtBottom = useRef(true);
@@ -43,6 +45,13 @@ function Comments() {
 
   useKeyDown("Escape", () => document && ui.set({ commentsExpanded: false }));
 
+  // Account for the resolved status of the comment changing
+  useEffect(() => {
+    if (focusedComment && focusedComment.isResolved !== viewingResolved) {
+      setViewingResolved(focusedComment.isResolved);
+    }
+  }, [focusedComment, viewingResolved]);
+
   const [draft, onSaveDraft] = usePersistedState<ProsemirrorData | undefined>(
     `draft-${document?.id}-new`,
     undefined
@@ -57,7 +66,6 @@ function Comments() {
       }
     : { type: CommentSortType.MostRecent };
 
-  const viewingResolved = params.get("resolved") === "";
   const threads = !document
     ? []
     : viewingResolved
@@ -124,7 +132,12 @@ function Comments() {
       title={
         <Flex align="center" justify="space-between" auto>
           <span>{t("Comments")}</span>
-          <CommentSortMenu />
+          <CommentSortMenu
+            viewingResolved={viewingResolved}
+            onChange={(val) => {
+              setViewingResolved(val === "resolved");
+            }}
+          />
         </Flex>
       }
       onClose={() => ui.set({ commentsExpanded: false })}

app/scenes/Document/components/Editor.tsx

@@ -3,7 +3,7 @@ import { observer } from "mobx-react";
 import * as React from "react";
 import { useTranslation } from "react-i18next";
 import { mergeRefs } from "react-merge-refs";
-import { useHistory, useRouteMatch } from "react-router-dom";
+import { useRouteMatch } from "react-router-dom";
 import styled from "styled-components";
 import Text from "@shared/components/Text";
 import { richExtensions, withComments } from "@shared/editor/nodes";
@@ -19,7 +19,7 @@ import Time from "~/components/Time";
 import { withUIExtensions } from "~/editor/extensions";
 import useCurrentTeam from "~/hooks/useCurrentTeam";
 import useCurrentUser from "~/hooks/useCurrentUser";
-import useFocusedComment from "~/hooks/useFocusedComment";
+import { useFocusedComment } from "~/hooks/useFocusedComment";
 import { useLocationSidebarContext } from "~/hooks/useLocationSidebarContext";
 import usePolicy from "~/hooks/usePolicy";
 import useQuery from "~/hooks/useQuery";
@@ -59,11 +59,11 @@ function DocumentEditor(props: Props, ref: React.RefObject<any>) {
   const titleRef = React.useRef<RefHandle>(null);
   const { t } = useTranslation();
   const match = useRouteMatch();
+  const { setFocusedCommentId } = useDocumentContext();
   const focusedComment = useFocusedComment();
   const { ui, comments } = useStores();
   const user = useCurrentUser({ rejectOnEmpty: false });
   const team = useCurrentTeam({ rejectOnEmpty: false });
-  const history = useHistory();
   const sidebarContext = useLocationSidebarContext();
   const params = useQuery();
   const {
@@ -95,18 +95,11 @@ function DocumentEditor(props: Props, ref: React.RefObject<any>) {
         (focusedComment.isResolved && !viewingResolved) ||
         (!focusedComment.isResolved && viewingResolved)
       ) {
-        history.replace({
-          search: focusedComment.isResolved ? "resolved=" : "",
-          pathname: location.pathname,
-          state: {
-            commentId: focusedComment.id,
-            sidebarContext,
-          },
-        });
+        setFocusedCommentId(focusedComment.id);
       }
       ui.set({ commentsExpanded: true });
     }
-  }, [focusedComment, ui, document.id, history, params, sidebarContext]);
+  }, [focusedComment, ui, document.id, params]);
 
   // Save document when blurring title, but delay so that if clicking on a
   // button this is allowed to execute first.
@@ -127,16 +120,6 @@ function DocumentEditor(props: Props, ref: React.RefObject<any>) {
     [focusAtStart, ref]
   );
 
-  const handleClickComment = React.useCallback(
-    (commentId: string) => {
-      history.replace({
-        pathname: window.location.pathname.replace(/\/history$/, ""),
-        state: { commentId, sidebarContext },
-      });
-    },
-    [history, sidebarContext]
-  );
-
   // Create a Comment model in local store when a comment mark is created, this
   // acts as a local draft before submission.
   const handleDraftComment = React.useCallback(
@@ -156,13 +139,9 @@ function DocumentEditor(props: Props, ref: React.RefObject<any>) {
       );
       comment.id = commentId;
       comments.add(comment);
-
-      history.replace({
-        pathname: window.location.pathname.replace(/\/history$/, ""),
-        state: { commentId, sidebarContext },
-      });
+      setFocusedCommentId(commentId);
     },
-    [comments, user?.id, props.id, history, sidebarContext]
+    [comments, user?.id, props.id]
   );
 
   // Soft delete the Comment model when associated mark is totally removed.
@@ -258,7 +237,7 @@ function DocumentEditor(props: Props, ref: React.RefObject<any>) {
         userId={user?.id}
         focusedCommentId={focusedComment?.id}
         onClickCommentMark={
-          commentingEnabled && can.comment ? handleClickComment : undefined
+          commentingEnabled && can.comment ? setFocusedCommentId : undefined
         }
         onCreateCommentMark={
           commentingEnabled && can.comment ? handleDraftComment : undefined

app/utils/routeHelpers.ts

@@ -33,7 +33,7 @@ export function settingsPath(...args: string[]): string {
 
 export function commentPath(document: Document, comment: Comment): string {
   return `${documentPath(document)}?commentId=${comment.id}${
-    comment.isResolved ? "&resolved=" : ""
+    comment.isResolved ? "&resolved=1" : ""
   }`;
 }