Various commenting improvements (#4941)

* fix: New threads attached to previous as replies

* fix: Cannot use floating toolbar properly in comments

* perf: Avoid re-writing history on click in editor

* fix: Comment on text selection

* fix: 'Copy link' on comments uses wrong hostname

* Show comment buttons on input focus rather than non-empty input
Increase maximum sidebar size

* Allow opening comments from document menu

* fix: Clicking comment menu should not focus thread

* fix: Selection color

* fix: Draft comments not restored

* Add border above document level comment input

* fix: Floating toolbar not constrainted by offset parent

* fix flash of no comment on saving

* fix: Clicking on editor does not remove draft mark

app/editor/components/FloatingToolbar.tsx

@@ -83,8 +83,8 @@ function usePosition({
   const offsetParent = menuRef.current.offsetParent
     ? menuRef.current.offsetParent.getBoundingClientRect()
     : ({
-        width: 0,
-        height: 0,
+        width: window.innerWidth,
+        height: window.innerHeight,
         top: 0,
         left: 0,
       } as DOMRect);
@@ -141,8 +141,8 @@ function usePosition({
     // instances leave a margin
     const margin = 12;
     const left = Math.min(
-      window.innerWidth - menuWidth - margin,
-      Math.max(margin, centerOfSelection - menuWidth / 2)
+      offsetParent.x + offsetParent.width - menuWidth - margin,
+      Math.max(offsetParent.x + margin, centerOfSelection - menuWidth / 2)
     );
     const top = Math.min(
       window.innerHeight - menuHeight - margin,

app/scenes/Document/components/CommentForm.tsx

@@ -3,6 +3,7 @@ import { action } from "mobx";
 import { observer } from "mobx-react";
 import * as React from "react";
 import { useTranslation } from "react-i18next";
+import { v4 as uuidv4 } from "uuid";
 import { CommentValidation } from "@shared/validations";
 import Comment from "~/models/Comment";
 import Avatar from "~/components/Avatar";
@@ -59,13 +60,13 @@ function CommentForm({
 }: Props) {
   const { editor } = useDocumentContext();
   const [data, setData] = usePersistedState<Record<string, any> | undefined>(
-    `draft-${documentId}-${thread?.id ?? "new"}`,
+    `draft-${documentId}-${!thread ? "new" : thread?.id}`,
     undefined
   );
   const formRef = React.useRef<HTMLFormElement>(null);
   const editorRef = React.useRef<SharedEditor>(null);
   const [forceRender, setForceRender] = React.useState(0);
-  const [inputFocused, setInputFocused] = React.useState(false);
+  const [inputFocused, setInputFocused] = React.useState(autoFocus);
   const { t } = useTranslation();
   const { showToast } = useToasts();
   const { comments } = useStores();
@@ -87,6 +88,7 @@ function CommentForm({
 
     setData(undefined);
     setForceRender((s) => ++s);
+    setInputFocused(false);
 
     const comment =
       thread ??
@@ -110,10 +112,11 @@ function CommentForm({
 
     // optimistically update the comment model
     comment.isNew = false;
+    comment.createdById = user.id;
     comment.createdBy = user;
   });
 
-  const handleCreateReply = async (event: React.FormEvent) => {
+  const handleCreateReply = action(async (event: React.FormEvent) => {
     event.preventDefault();
     if (!data) {
       return;
@@ -121,17 +124,31 @@ function CommentForm({
 
     setData(undefined);
     setForceRender((s) => ++s);
+    setInputFocused(false);
 
-    try {
-      await comments.save({
+    const comment = new Comment(
+      {
         parentCommentId: thread?.id,
         documentId,
         data,
-      });
-    } catch (error) {
+      },
+      comments
+    );
+
+    comment.id = uuidv4();
+    comments.add(comment);
+
+    comment.save().catch(() => {
+      comments.remove(comment.id);
+      comment.isNew = true;
       showToast(t("Error creating comment"), { type: "error" });
-    }
-  };
+    });
+
+    // optimistically update the comment model
+    comment.isNew = false;
+    comment.createdById = user.id;
+    comment.createdBy = user;
+  });
 
   const handleChange = (
     value: (asString: boolean, trim: boolean) => Record<string, any>
@@ -155,6 +172,7 @@ function CommentForm({
   const handleCancel = () => {
     setData(undefined);
     setForceRender((s) => ++s);
+    setInputFocused(false);
   };
 
   const handleFocus = () => {
@@ -164,7 +182,6 @@ function CommentForm({
 
   const handleBlur = () => {
     onBlur?.();
-    setInputFocused(false);
   };
 
   // Focus the editor when it's a new comment just mounted, after a delay as the
@@ -219,6 +236,7 @@ function CommentForm({
           <CommentEditor
             key={`${forceRender}`}
             ref={editorRef}
+            defaultValue={data}
             onChange={handleChange}
             onSave={handleSave}
             onFocus={handleFocus}

app/scenes/Document/components/Comments.tsx

@@ -6,8 +6,10 @@ import { useRouteMatch } from "react-router-dom";
 import styled from "styled-components";
 import Empty from "~/components/Empty";
 import Flex from "~/components/Flex";
+import Scrollable from "~/components/Scrollable";
 import useCurrentUser from "~/hooks/useCurrentUser";
 import useFocusedComment from "~/hooks/useFocusedComment";
+import useKeyDown from "~/hooks/useKeyDown";
 import useStores from "~/hooks/useStores";
 import CommentForm from "./CommentForm";
 import CommentThread from "./CommentThread";
@@ -21,6 +23,8 @@ function Comments() {
   const document = documents.getByUrl(match.params.documentSlug);
   const focusedComment = useFocusedComment();
 
+  useKeyDown("Escape", ui.collapseComments);
+
   if (!document) {
     return null;
   }
@@ -31,37 +35,42 @@ function Comments() {
   const hasComments = threads.length > 0;
 
   return (
-    <Sidebar title={t("Comments")} onClose={ui.collapseComments}>
-      <Wrapper $hasComments={hasComments}>
-        {hasComments ? (
-          threads.map((thread) => (
-            <CommentThread
-              key={thread.id}
-              comment={thread}
-              document={document}
-              recessed={!!focusedComment && focusedComment.id !== thread.id}
-              focused={focusedComment?.id === thread.id}
-            />
-          ))
-        ) : (
-          <NoComments align="center" justify="center" auto>
-            <Empty>{t("No comments yet")}</Empty>
-          </NoComments>
-        )}
-
-        <AnimatePresence initial={false}>
-          {!focusedComment && (
-            <NewCommentForm
-              documentId={document.id}
-              placeholder={`${t("Add a comment")}…`}
-              autoFocus={false}
-              dir={document.dir}
-              animatePresence
-              standalone
-            />
+    <Sidebar
+      title={t("Comments")}
+      onClose={ui.collapseComments}
+      scrollable={false}
+    >
+      <Scrollable hiddenScrollbars topShadow bottomShadow>
+        <Wrapper $hasComments={hasComments}>
+          {hasComments ? (
+            threads.map((thread) => (
+              <CommentThread
+                key={thread.id}
+                comment={thread}
+                document={document}
+                recessed={!!focusedComment && focusedComment.id !== thread.id}
+                focused={focusedComment?.id === thread.id}
+              />
+            ))
+          ) : (
+            <NoComments align="center" justify="center" auto>
+              <Empty>{t("No comments yet")}</Empty>
+            </NoComments>
           )}
-        </AnimatePresence>
-      </Wrapper>
+        </Wrapper>
+      </Scrollable>
+      <AnimatePresence initial={false}>
+        {!focusedComment && (
+          <NewCommentForm
+            documentId={document.id}
+            placeholder={`${t("Add a comment")}…`}
+            autoFocus={false}
+            dir={document.dir}
+            animatePresence
+            standalone
+          />
+        )}
+      </AnimatePresence>
     </Sidebar>
   );
 }
@@ -77,14 +86,9 @@ const Wrapper = styled.div<{ $hasComments: boolean }>`
 `;
 
 const NewCommentForm = styled(CommentForm)<{ dir?: "ltr" | "rtl" }>`
-  background: ${(props) => props.theme.background};
-  position: absolute;
   padding: 12px;
   padding-right: ${(props) => (props.dir !== "rtl" ? "18px" : "12px")};
   padding-left: ${(props) => (props.dir === "rtl" ? "18px" : "12px")};
-  left: 0;
-  right: 0;
-  bottom: 0;
 `;
 
 export default observer(Comments);

app/scenes/Document/components/Editor.tsx

@@ -86,20 +86,14 @@ function DocumentEditor(props: Props, ref: React.RefObject<any>) {
   );
 
   const handleClickComment = React.useCallback(
-    (commentId?: string) => {
-      if (commentId) {
-        ui.expandComments();
-        history.replace({
-          pathname: window.location.pathname.replace(/\/history$/, ""),
-          state: { commentId },
-        });
-      } else if (focusedComment) {
-        history.replace({
-          pathname: window.location.pathname,
-        });
-      }
+    (commentId: string) => {
+      ui.expandComments();
+      history.replace({
+        pathname: window.location.pathname.replace(/\/history$/, ""),
+        state: { commentId },
+      });
     },
-    [ui, focusedComment, history]
+    [ui, history]
   );
 
   // Create a Comment model in local store when a comment mark is created, this

app/scenes/Document/components/SidebarLayout.tsx

@@ -9,13 +9,17 @@ import Scrollable from "~/components/Scrollable";
 import Tooltip from "~/components/Tooltip";
 
 type Props = React.HTMLAttributes<HTMLDivElement> & {
+  /* The title of the sidebar */
   title: React.ReactNode;
+  /* The content of the sidebar */
   children: React.ReactNode;
+  /* Called when the sidebar is closed */
   onClose: React.MouseEventHandler;
-  border?: boolean;
+  /* Whether the sidebar should be scrollable */
+  scrollable?: boolean;
 };
 
-function SidebarLayout({ title, onClose, children }: Props) {
+function SidebarLayout({ title, onClose, children, scrollable = true }: Props) {
   const { t } = useTranslation();
 
   return (
@@ -31,9 +35,13 @@ function SidebarLayout({ title, onClose, children }: Props) {
           />
         </Tooltip>
       </Header>
-      <Scrollable hiddenScrollbars topShadow>
-        {children}
-      </Scrollable>
+      {scrollable ? (
+        <Scrollable hiddenScrollbars topShadow>
+          {children}
+        </Scrollable>
+      ) : (
+        children
+      )}
     </>
   );
 }

shared/editor/components/Styles.ts

@@ -97,10 +97,6 @@ math-block .katex-display {
   margin: 0;
 }
 
-p::selection, p > *::selection {
-  background-color: #c0c0c0;
-}
-
 .katex-html *::selection {
   background-color: none !important;
 }

shared/editor/marks/Comment.ts

@@ -90,7 +90,6 @@ export default class Comment extends Mark {
                 !(event.target instanceof HTMLSpanElement) ||
                 !event.target.classList.contains("comment")
               ) {
-                this.options?.onClickCommentMark?.();
                 return false;
               }