From fff0812659bf6cb31a1f4ea25dcd54e383053289 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 27 Feb 2023 19:50:35 -0500 Subject: [PATCH] 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 | 8 +- .../Document/components/CommentForm.tsx | 38 +++++++--- app/scenes/Document/components/Comments.tsx | 74 ++++++++++--------- app/scenes/Document/components/Editor.tsx | 20 ++--- .../Document/components/SidebarLayout.tsx | 18 +++-- shared/editor/components/Styles.ts | 4 - shared/editor/marks/Comment.ts | 1 - 7 files changed, 91 insertions(+), 72 deletions(-) diff --git a/app/editor/components/FloatingToolbar.tsx b/app/editor/components/FloatingToolbar.tsx index b903f3f96..f831e5a57 100644 --- a/app/editor/components/FloatingToolbar.tsx +++ b/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, diff --git a/app/scenes/Document/components/CommentForm.tsx b/app/scenes/Document/components/CommentForm.tsx index 5e83a81b8..7e9c46230 100644 --- a/app/scenes/Document/components/CommentForm.tsx +++ b/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 | undefined>( - `draft-${documentId}-${thread?.id ?? "new"}`, + `draft-${documentId}-${!thread ? "new" : thread?.id}`, undefined ); const formRef = React.useRef(null); const editorRef = React.useRef(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 @@ -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({ 0; return ( - - - {hasComments ? ( - threads.map((thread) => ( - - )) - ) : ( - - {t("No comments yet")} - - )} - - - {!focusedComment && ( - + + + + {hasComments ? ( + threads.map((thread) => ( + + )) + ) : ( + + {t("No comments yet")} + )} - - + + + + {!focusedComment && ( + + )} + ); } @@ -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); diff --git a/app/scenes/Document/components/Editor.tsx b/app/scenes/Document/components/Editor.tsx index da15c68a8..422ec2dcb 100644 --- a/app/scenes/Document/components/Editor.tsx +++ b/app/scenes/Document/components/Editor.tsx @@ -86,20 +86,14 @@ function DocumentEditor(props: Props, ref: React.RefObject) { ); 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 diff --git a/app/scenes/Document/components/SidebarLayout.tsx b/app/scenes/Document/components/SidebarLayout.tsx index a237577e0..66642da0b 100644 --- a/app/scenes/Document/components/SidebarLayout.tsx +++ b/app/scenes/Document/components/SidebarLayout.tsx @@ -9,13 +9,17 @@ import Scrollable from "~/components/Scrollable"; import Tooltip from "~/components/Tooltip"; type Props = React.HTMLAttributes & { + /* 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) { /> - - {children} - + {scrollable ? ( + + {children} + + ) : ( + children + )} ); } diff --git a/shared/editor/components/Styles.ts b/shared/editor/components/Styles.ts index 71f28bed3..6fe93455a 100644 --- a/shared/editor/components/Styles.ts +++ b/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; } diff --git a/shared/editor/marks/Comment.ts b/shared/editor/marks/Comment.ts index 78a15baed..b99f3e2e8 100644 --- a/shared/editor/marks/Comment.ts +++ b/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; }