From 4f134703451abb5ffa3075ac6abb0f0d7d8131e9 Mon Sep 17 00:00:00 2001 From: Tim Down <158919+timdown@users.noreply.github.com> Date: Tue, 21 Nov 2023 14:29:01 +0000 Subject: [PATCH] Merge pull request #15819 from overleaf/td-review-panel-comment-selection Prevent selection within review panel entry being immediately cleared GitOrigin-RevId: cca7206a6620bed8cae1bb55a064102204a00751 --- .../entries/aggregate-change-entry.tsx | 19 +++---------- .../review-panel/entries/change-entry.tsx | 19 +++---------- .../review-panel/entries/comment-entry.tsx | 20 +++----------- .../review-panel/hooks/use-entry-click.ts | 27 +++++++++++++++++++ 4 files changed, 36 insertions(+), 49 deletions(-) create mode 100644 services/web/frontend/js/features/source-editor/components/review-panel/hooks/use-entry-click.ts diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/entries/aggregate-change-entry.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/entries/aggregate-change-entry.tsx index be186dd2af..60d4c89b5f 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/entries/aggregate-change-entry.tsx +++ b/services/web/frontend/js/features/source-editor/components/review-panel/entries/aggregate-change-entry.tsx @@ -11,6 +11,7 @@ import comparePropsWithShallowArrayCompare from '../utils/compare-props-with-sha import { BaseChangeEntryProps } from '../types/base-change-entry-props' import useIndicatorHover from '../hooks/use-indicator-hover' import EntryIndicator from './entry-indicator' +import { useEntryClick } from '@/features/source-editor/components/review-panel/hooks/use-entry-click' interface AggregateChangeEntryProps extends BaseChangeEntryProps { replacedContent: string @@ -30,7 +31,7 @@ function AggregateChangeEntry({ contentLimit = 17, }: AggregateChangeEntryProps) { const { t } = useTranslation() - const { acceptChanges, rejectChanges, gotoEntry, handleLayoutChange } = + const { acceptChanges, rejectChanges, handleLayoutChange } = useReviewPanelUpdaterFnsContext() const [isDeletionCollapsed, setIsDeletionCollapsed] = useState(true) const [isInsertionCollapsed, setIsInsertionCollapsed] = useState(true) @@ -53,21 +54,7 @@ function AggregateChangeEntry({ ? content.substring(0, contentLimit) : content - const handleEntryClick = (e: React.MouseEvent) => { - const target = e.target as Element - - for (const selector of [ - '.rp-entry', - '.rp-entry-description', - '.rp-entry-body', - '.rp-entry-action-icon i', - ]) { - if (target.matches(selector)) { - gotoEntry(docId, offset) - break - } - } - } + const handleEntryClick = useEntryClick(docId, offset) const handleDeletionToggleCollapse = () => { setIsDeletionCollapsed(value => !value) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/entries/change-entry.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/entries/change-entry.tsx index dac1eb1727..0e533cbdd4 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/entries/change-entry.tsx +++ b/services/web/frontend/js/features/source-editor/components/review-panel/entries/change-entry.tsx @@ -12,6 +12,7 @@ import { BaseChangeEntryProps } from '../types/base-change-entry-props' import comparePropsWithShallowArrayCompare from '../utils/compare-props-with-shallow-array-compare' import useIndicatorHover from '../hooks/use-indicator-hover' import EntryIndicator from './entry-indicator' +import { useEntryClick } from '@/features/source-editor/components/review-panel/hooks/use-entry-click' interface ChangeEntryProps extends BaseChangeEntryProps { type: ReviewPanelChangeEntry['type'] @@ -31,7 +32,7 @@ function ChangeEntry({ contentLimit = 40, }: ChangeEntryProps) { const { t } = useTranslation() - const { handleLayoutChange, acceptChanges, rejectChanges, gotoEntry } = + const { handleLayoutChange, acceptChanges, rejectChanges } = useReviewPanelUpdaterFnsContext() const [isCollapsed, setIsCollapsed] = useState(true) const { @@ -49,21 +50,7 @@ function ChangeEntry({ const needsCollapsing = content.length > contentLimit const isInsert = type === 'insert' - const handleEntryClick = (e: React.MouseEvent) => { - const target = e.target as Element - - for (const selector of [ - '.rp-entry', - '.rp-entry-description', - '.rp-entry-body', - '.rp-entry-action-icon i', - ]) { - if (target.matches(selector)) { - gotoEntry(docId, offset) - break - } - } - } + const handleEntryClick = useEntryClick(docId, offset) const handleToggleCollapse = () => { setIsCollapsed(value => !value) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/entries/comment-entry.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/entries/comment-entry.tsx index fbf8388a80..9990ccfa7d 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/entries/comment-entry.tsx +++ b/services/web/frontend/js/features/source-editor/components/review-panel/entries/comment-entry.tsx @@ -17,6 +17,7 @@ import { ReviewPanelCommentThread } from '../../../../../../../types/review-pane import { ReviewPanelCommentEntry } from '../../../../../../../types/review-panel/entry' import useIndicatorHover from '../hooks/use-indicator-hover' import EntryIndicator from './entry-indicator' +import { useEntryClick } from '@/features/source-editor/components/review-panel/hooks/use-entry-click' type CommentEntryProps = { docId: DocId @@ -36,7 +37,7 @@ function CommentEntry({ permissions, }: CommentEntryProps) { const { t } = useTranslation() - const { gotoEntry, resolveComment, submitReply, handleLayoutChange } = + const { resolveComment, submitReply, handleLayoutChange } = useReviewPanelUpdaterFnsContext() const [replyContent, setReplyContent] = useState('') const [animating, setAnimating] = useState(false) @@ -50,22 +51,7 @@ function CommentEntry({ handleIndicatorClick, } = useIndicatorHover() - const handleEntryClick = (e: React.MouseEvent) => { - const target = e.target as Element - - for (const selector of [ - '.rp-entry', - '.rp-comment-loaded', - '.rp-comment-content', - '.rp-comment-reply', - '.rp-entry-metadata', - ]) { - if (target.matches(selector)) { - gotoEntry(docId, offset) - break - } - } - } + const handleEntryClick = useEntryClick(docId, offset) const handleAnimateAndCallOnResolve = () => { setAnimating(true) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/hooks/use-entry-click.ts b/services/web/frontend/js/features/source-editor/components/review-panel/hooks/use-entry-click.ts new file mode 100644 index 0000000000..b82ba38f09 --- /dev/null +++ b/services/web/frontend/js/features/source-editor/components/review-panel/hooks/use-entry-click.ts @@ -0,0 +1,27 @@ +import { useReviewPanelUpdaterFnsContext } from '@/features/source-editor/context/review-panel/review-panel-context' +import { DocId } from '../../../../../../../types/project-settings' + +export function useEntryClick(docId: DocId, offset: number) { + const { gotoEntry } = useReviewPanelUpdaterFnsContext() + + return (e: React.MouseEvent) => { + const target = e.target as Element + + // Ignore clicks inside interactive elements + if (!target.closest('textarea, button, a')) { + // If the user was making a selection within the entry rather than + // clicking it, ignore the click. Do this by checking whether there is a + // selection that intersects with the target, in which case we assume + // the user was making a selection + const selection = window.getSelection() + if ( + !selection || + selection.isCollapsed || + selection.rangeCount === 0 || + !selection.getRangeAt(0).intersectsNode(target) + ) { + gotoEntry(docId, offset) + } + } + } +}