Merge pull request #13720 from overleaf/td-review-panel-entry-pos

React review panel entry positioning

GitOrigin-RevId: c22617b1d3243b7d54b093426358aeb291421b9e
This commit is contained in:
Tim Down 2023-07-18 09:35:04 +01:00 committed by Copybot
parent b24d209453
commit 38c673d057
23 changed files with 717 additions and 167 deletions

View file

@ -1,4 +1,4 @@
import { useMemo } from 'react' import { useCallback, useMemo } from 'react'
import Container from './container' import Container from './container'
import Toolbar from './toolbar/toolbar' import Toolbar from './toolbar/toolbar'
import Nav from './nav' import Nav from './nav'
@ -8,13 +8,21 @@ import AggregateChangeEntry from './entries/aggregate-change-entry'
import CommentEntry from './entries/comment-entry' import CommentEntry from './entries/comment-entry'
import AddCommentEntry from './entries/add-comment-entry' import AddCommentEntry from './entries/add-comment-entry'
import BulkActionsEntry from './entries/bulk-actions-entry/bulk-actions-entry' import BulkActionsEntry from './entries/bulk-actions-entry/bulk-actions-entry'
import PositionedEntries from './positioned-entries'
import { import {
useReviewPanelUpdaterFnsContext, useReviewPanelUpdaterFnsContext,
useReviewPanelValueContext, useReviewPanelValueContext,
} from '../../context/review-panel/review-panel-context' } from '../../context/review-panel/review-panel-context'
import useCodeMirrorContentHeight from '../../hooks/use-codemirror-content-height' import useCodeMirrorContentHeight from '../../hooks/use-codemirror-content-height'
import { ReviewPanelEntry } from '../../../../../../types/review-panel/entry' import { ReviewPanelEntry } from '../../../../../../types/review-panel/entry'
import { ThreadId } from '../../../../../../types/review-panel/review-panel' import {
ReviewPanelDocEntries,
ThreadId,
} from '../../../../../../types/review-panel/review-panel'
const isEntryAThreadId = (
entry: keyof ReviewPanelDocEntries
): entry is ThreadId => entry !== 'add-comment' && entry !== 'bulk-actions'
function CurrentFileContainer() { function CurrentFileContainer() {
const { const {
@ -36,10 +44,18 @@ function CurrentFileContainer() {
const objectEntries = useMemo(() => { const objectEntries = useMemo(() => {
return Object.entries(currentDocEntries || {}) as Array< return Object.entries(currentDocEntries || {}) as Array<
[ThreadId, ReviewPanelEntry] [keyof ReviewPanelDocEntries, ReviewPanelEntry]
> >
}, [currentDocEntries]) }, [currentDocEntries])
const onMouseEnter = useCallback(() => {
setEntryHover(true)
}, [setEntryHover])
const onMouseLeave = useCallback(() => {
setEntryHover(false)
}, [setEntryHover])
return ( return (
<Container classNames={{ 'rp-collapsed-displaying-entry': entryHover }}> <Container classNames={{ 'rp-collapsed-displaying-entry': entryHover }}>
<div className="review-panel-tools"> <div className="review-panel-tools">
@ -53,9 +69,9 @@ function CurrentFileContainer() {
tabIndex={0} tabIndex={0}
aria-labelledby="review-panel-tab-current-file" aria-labelledby="review-panel-tab-current-file"
> >
<div <PositionedEntries
className="rp-entry-list-inner" entries={objectEntries}
style={{ height: `${contentHeight}px` }} contentHeight={contentHeight}
> >
{openDocId && {openDocId &&
objectEntries.map(([id, entry]) => { objectEntries.map(([id, entry]) => {
@ -63,37 +79,46 @@ function CurrentFileContainer() {
return null return null
} }
if (entry.type === 'insert' || entry.type === 'delete') { if (
isEntryAThreadId(id) &&
(entry.type === 'insert' || entry.type === 'delete')
) {
return ( return (
<ChangeEntry <ChangeEntry
key={id} key={id}
docId={openDocId} docId={openDocId}
entry={entry} entry={entry}
entryId={id}
permissions={permissions} permissions={permissions}
user={users[entry.metadata.user_id]} user={users[entry.metadata.user_id]}
onMouseEnter={setEntryHover.bind(null, true)} onMouseEnter={onMouseEnter}
onMouseLeave={setEntryHover.bind(null, false)} onMouseLeave={onMouseLeave}
onIndicatorClick={toggleReviewPanel} onIndicatorClick={toggleReviewPanel}
/> />
) )
} }
if (entry.type === 'aggregate-change') { if (isEntryAThreadId(id) && entry.type === 'aggregate-change') {
return ( return (
<AggregateChangeEntry <AggregateChangeEntry
key={id} key={id}
docId={openDocId} docId={openDocId}
entry={entry} entry={entry}
entryId={id}
permissions={permissions} permissions={permissions}
user={users[entry.metadata.user_id]} user={users[entry.metadata.user_id]}
onMouseEnter={setEntryHover.bind(null, true)} onMouseEnter={onMouseEnter}
onMouseLeave={setEntryHover.bind(null, false)} onMouseLeave={onMouseLeave}
onIndicatorClick={toggleReviewPanel} onIndicatorClick={toggleReviewPanel}
/> />
) )
} }
if (entry.type === 'comment' && !loadingThreads) { if (
isEntryAThreadId(id) &&
entry.type === 'comment' &&
!loadingThreads
) {
return ( return (
<CommentEntry <CommentEntry
key={id} key={id}
@ -102,15 +127,15 @@ function CurrentFileContainer() {
entryId={id} entryId={id}
permissions={permissions} permissions={permissions}
threads={commentThreads} threads={commentThreads}
onMouseEnter={setEntryHover.bind(null, true)} onMouseEnter={onMouseEnter}
onMouseLeave={setEntryHover.bind(null, false)} onMouseLeave={onMouseLeave}
onIndicatorClick={toggleReviewPanel} onIndicatorClick={toggleReviewPanel}
/> />
) )
} }
if (entry.type === 'add-comment' && permissions.comment) { if (entry.type === 'add-comment' && permissions.comment) {
return <AddCommentEntry key={id} entry={entry} /> return <AddCommentEntry key={id} entryId={entry.type} />
} }
if (entry.type === 'bulk-actions') { if (entry.type === 'bulk-actions') {
@ -118,6 +143,7 @@ function CurrentFileContainer() {
<BulkActionsEntry <BulkActionsEntry
key={id} key={id}
entry={entry} entry={entry}
entryId={entry.type}
nChanges={nChanges} nChanges={nChanges}
/> />
) )
@ -125,7 +151,7 @@ function CurrentFileContainer() {
return null return null
})} })}
</div> </PositionedEntries>
</div> </div>
</Container> </Container>
) )

View file

@ -12,8 +12,6 @@ import { useCodeMirrorViewContext } from '../../codemirror-editor'
import Modal, { useBulkActionsModal } from '../entries/bulk-actions-entry/modal' import Modal, { useBulkActionsModal } from '../entries/bulk-actions-entry/modal'
import getMeta from '../../../../../utils/meta' import getMeta from '../../../../../utils/meta'
import useScopeValue from '../../../../../shared/hooks/use-scope-value' import useScopeValue from '../../../../../shared/hooks/use-scope-value'
import { MergeAndOverride } from '../../../../../../../types/utils'
import { ReviewPanelBulkActionsEntry } from '../../../../../../../types/review-panel/entry'
function EditorWidgets() { function EditorWidgets() {
const { t } = useTranslation() const { t } = useTranslation()
@ -32,30 +30,13 @@ function EditorWidgets() {
) )
const view = useCodeMirrorViewContext() const view = useCodeMirrorViewContext()
type UseReviewPanelValueContextReturnType = ReturnType<
typeof useReviewPanelValueContext
>
const { const {
entries, entries,
openDocId, openDocId,
nVisibleSelectedChanges: nChanges, nVisibleSelectedChanges: nChanges,
wantTrackChanges, wantTrackChanges,
permissions, permissions,
// Remapping entries as they may contain `add-comment` and `bulk-actions` props along with DocIds } = useReviewPanelValueContext()
// Ideally the `add-comment` and `bulk-actions` objects should not be within the entries object
// as the doc data, but this is what currently angular returns.
} = useReviewPanelValueContext() as MergeAndOverride<
UseReviewPanelValueContextReturnType,
{
entries: {
// eslint-disable-next-line no-use-before-define
[Entry in UseReviewPanelValueContextReturnType['entries'] as keyof Entry]: Entry & {
'add-comment': ReviewPanelBulkActionsEntry
'bulk-actions': ReviewPanelBulkActionsEntry
}
}
}
>
const hasTrackChangesFeature = getMeta('ol-hasTrackChangesFeature') const hasTrackChangesFeature = getMeta('ol-hasTrackChangesFeature')

View file

@ -1,5 +1,5 @@
import { useTranslation } from 'react-i18next' import { useTranslation } from 'react-i18next'
import { useState } from 'react' import { useEffect, useState } from 'react'
import EntryContainer from './entry-container' import EntryContainer from './entry-container'
import EntryCallout from './entry-callout' import EntryCallout from './entry-callout'
import EntryActions from './entry-actions' import EntryActions from './entry-actions'
@ -14,34 +14,41 @@ import classnames from 'classnames'
import { ReviewPanelAddCommentEntry } from '../../../../../../../types/review-panel/entry' import { ReviewPanelAddCommentEntry } from '../../../../../../../types/review-panel/entry'
type AddCommentEntryProps = { type AddCommentEntryProps = {
entry: ReviewPanelAddCommentEntry entryId: ReviewPanelAddCommentEntry['type']
} }
function AddCommentEntry({ entry }: AddCommentEntryProps) { function AddCommentEntry({ entryId }: AddCommentEntryProps) {
const { t } = useTranslation() const { t } = useTranslation()
const { isAddingComment, submitNewComment, handleLayoutChange } = const { isAddingComment, submitNewComment } = useReviewPanelValueContext()
useReviewPanelValueContext() const { setIsAddingComment, handleLayoutChange } =
const { setIsAddingComment } = useReviewPanelUpdaterFnsContext() useReviewPanelUpdaterFnsContext()
const [content, setContent] = useState('') const [content, setContent] = useState('')
const handleStartNewComment = () => { const handleStartNewComment = () => {
setIsAddingComment(true) setIsAddingComment(true)
handleLayoutChange() window.setTimeout(handleLayoutChange, 0)
} }
const handleSubmitNewComment = () => { const handleSubmitNewComment = () => {
submitNewComment(content) submitNewComment(content)
setIsAddingComment(false) setIsAddingComment(false)
setContent('') setContent('')
window.setTimeout(handleLayoutChange, 0)
} }
const handleCancelNewComment = () => { const handleCancelNewComment = () => {
setIsAddingComment(false) setIsAddingComment(false)
setContent('') setContent('')
handleLayoutChange() window.setTimeout(handleLayoutChange, 0)
} }
useEffect(() => {
return () => {
setIsAddingComment(false)
}
}, [setIsAddingComment])
const handleCommentKeyPress = ( const handleCommentKeyPress = (
e: React.KeyboardEvent<HTMLTextAreaElement> e: React.KeyboardEvent<HTMLTextAreaElement>
) => { ) => {
@ -62,16 +69,12 @@ function AddCommentEntry({ entry }: AddCommentEntryProps) {
} }
return ( return (
<EntryContainer> <EntryContainer id={entryId}>
<EntryCallout className="rp-entry-callout-add-comment" /> <EntryCallout className="rp-entry-callout-add-comment" />
<div <div
className={classnames('rp-entry', 'rp-entry-add-comment', { className={classnames('rp-entry', 'rp-entry-add-comment', {
'rp-entry-adding-comment': isAddingComment, 'rp-entry-adding-comment': isAddingComment,
})} })}
style={{
top: entry.screenPos.y + 'px',
visibility: entry.visible ? 'visible' : 'hidden',
}}
> >
{isAddingComment ? ( {isAddingComment ? (
<> <>

View file

@ -4,19 +4,24 @@ import EntryContainer from './entry-container'
import EntryCallout from './entry-callout' import EntryCallout from './entry-callout'
import EntryActions from './entry-actions' import EntryActions from './entry-actions'
import Icon from '../../../../../shared/components/icon' import Icon from '../../../../../shared/components/icon'
import { useReviewPanelValueContext } from '../../../context/review-panel/review-panel-context' import {
useReviewPanelUpdaterFnsContext,
useReviewPanelValueContext,
} from '../../../context/review-panel/review-panel-context'
import { formatTime } from '../../../../utils/format-date' import { formatTime } from '../../../../utils/format-date'
import classnames from 'classnames' import classnames from 'classnames'
import { ReviewPanelAggregateChangeEntry } from '../../../../../../../types/review-panel/entry' import { ReviewPanelAggregateChangeEntry } from '../../../../../../../types/review-panel/entry'
import { import {
ReviewPanelPermissions, ReviewPanelPermissions,
ReviewPanelUser, ReviewPanelUser,
ThreadId,
} from '../../../../../../../types/review-panel/review-panel' } from '../../../../../../../types/review-panel/review-panel'
import { DocId } from '../../../../../../../types/project-settings' import { DocId } from '../../../../../../../types/project-settings'
type AggregateChangeEntryProps = { type AggregateChangeEntryProps = {
docId: DocId docId: DocId
entry: ReviewPanelAggregateChangeEntry entry: ReviewPanelAggregateChangeEntry
entryId: ThreadId
permissions: ReviewPanelPermissions permissions: ReviewPanelPermissions
user: ReviewPanelUser | undefined user: ReviewPanelUser | undefined
contentLimit?: number contentLimit?: number
@ -28,6 +33,7 @@ type AggregateChangeEntryProps = {
function AggregateChangeEntry({ function AggregateChangeEntry({
docId, docId,
entry, entry,
entryId,
permissions, permissions,
user, user,
contentLimit = 17, contentLimit = 17,
@ -36,8 +42,9 @@ function AggregateChangeEntry({
onIndicatorClick, onIndicatorClick,
}: AggregateChangeEntryProps) { }: AggregateChangeEntryProps) {
const { t } = useTranslation() const { t } = useTranslation()
const { acceptChanges, rejectChanges, handleLayoutChange, gotoEntry } = const { acceptChanges, rejectChanges, gotoEntry } =
useReviewPanelValueContext() useReviewPanelValueContext()
const { handleLayoutChange } = useReviewPanelUpdaterFnsContext()
const [isDeletionCollapsed, setIsDeletionCollapsed] = useState(true) const [isDeletionCollapsed, setIsDeletionCollapsed] = useState(true)
const [isInsertionCollapsed, setIsInsertionCollapsed] = useState(true) const [isInsertionCollapsed, setIsInsertionCollapsed] = useState(true)
@ -82,26 +89,17 @@ function AggregateChangeEntry({
return ( return (
<EntryContainer <EntryContainer
id={entryId}
onClick={handleEntryClick} onClick={handleEntryClick}
onMouseEnter={onMouseEnter} onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave} onMouseLeave={onMouseLeave}
> >
<EntryCallout <EntryCallout className="rp-entry-callout-aggregate" />
className="rp-entry-callout-aggregate"
style={{
top: entry.screenPos
? entry.screenPos.y + entry.screenPos.height - 1 + 'px'
: undefined,
}}
/>
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */} {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */}
<div <div
className={classnames('rp-entry-indicator', { className={classnames('rp-entry-indicator', {
'rp-entry-indicator-focused': entry.focused, 'rp-entry-indicator-focused': entry.focused,
})} })}
style={{
top: entry.screenPos ? entry.screenPos.y + 'px' : undefined,
}}
onClick={onIndicatorClick} onClick={onIndicatorClick}
> >
<Icon type="pencil" /> <Icon type="pencil" />
@ -110,10 +108,6 @@ function AggregateChangeEntry({
className={classnames('rp-entry', 'rp-entry-aggregate', { className={classnames('rp-entry', 'rp-entry-aggregate', {
'rp-entry-focused': entry.focused, 'rp-entry-focused': entry.focused,
})} })}
style={{
top: entry.screenPos ? entry.screenPos.y + 'px' : undefined,
visibility: entry.visible ? 'visible' : 'hidden',
}}
> >
<div className="rp-entry-body"> <div className="rp-entry-body">
<div className="rp-entry-action-icon"> <div className="rp-entry-action-icon">

View file

@ -8,10 +8,11 @@ import { ReviewPanelBulkActionsEntry } from '../../../../../../../../types/revie
type BulkActionsEntryProps = { type BulkActionsEntryProps = {
entry: ReviewPanelBulkActionsEntry entry: ReviewPanelBulkActionsEntry
entryId: ReviewPanelBulkActionsEntry['type']
nChanges: number nChanges: number
} }
function BulkActionsEntry({ entry, nChanges }: BulkActionsEntryProps) { function BulkActionsEntry({ entry, entryId, nChanges }: BulkActionsEntryProps) {
const { t } = useTranslation() const { t } = useTranslation()
const { const {
show, show,
@ -24,7 +25,7 @@ function BulkActionsEntry({ entry, nChanges }: BulkActionsEntryProps) {
return ( return (
<> <>
<EntryContainer> <EntryContainer id={entryId}>
{nChanges > 1 && ( {nChanges > 1 && (
<> <>
<EntryCallout <EntryCallout

View file

@ -4,7 +4,10 @@ import EntryContainer from './entry-container'
import EntryCallout from './entry-callout' import EntryCallout from './entry-callout'
import EntryActions from './entry-actions' import EntryActions from './entry-actions'
import Icon from '../../../../../shared/components/icon' import Icon from '../../../../../shared/components/icon'
import { useReviewPanelValueContext } from '../../../context/review-panel/review-panel-context' import {
useReviewPanelUpdaterFnsContext,
useReviewPanelValueContext,
} from '../../../context/review-panel/review-panel-context'
import { formatTime } from '../../../../utils/format-date' import { formatTime } from '../../../../utils/format-date'
import classnames from 'classnames' import classnames from 'classnames'
import { import {
@ -14,12 +17,14 @@ import {
import { import {
ReviewPanelPermissions, ReviewPanelPermissions,
ReviewPanelUser, ReviewPanelUser,
ThreadId,
} from '../../../../../../../types/review-panel/review-panel' } from '../../../../../../../types/review-panel/review-panel'
import { DocId } from '../../../../../../../types/project-settings' import { DocId } from '../../../../../../../types/project-settings'
type ChangeEntryProps = { type ChangeEntryProps = {
docId: DocId docId: DocId
entry: ReviewPanelInsertEntry | ReviewPanelDeleteEntry entry: ReviewPanelInsertEntry | ReviewPanelDeleteEntry
entryId: ThreadId
permissions: ReviewPanelPermissions permissions: ReviewPanelPermissions
user: ReviewPanelUser | undefined user: ReviewPanelUser | undefined
contentLimit?: number contentLimit?: number
@ -31,6 +36,7 @@ type ChangeEntryProps = {
function ChangeEntry({ function ChangeEntry({
docId, docId,
entry, entry,
entryId,
permissions, permissions,
user, user,
contentLimit = 40, contentLimit = 40,
@ -39,8 +45,9 @@ function ChangeEntry({
onIndicatorClick, onIndicatorClick,
}: ChangeEntryProps) { }: ChangeEntryProps) {
const { t } = useTranslation() const { t } = useTranslation()
const { acceptChanges, rejectChanges, handleLayoutChange, gotoEntry } = const { acceptChanges, rejectChanges, gotoEntry } =
useReviewPanelValueContext() useReviewPanelValueContext()
const { handleLayoutChange } = useReviewPanelUpdaterFnsContext()
const [isCollapsed, setIsCollapsed] = useState(true) const [isCollapsed, setIsCollapsed] = useState(true)
const content = isCollapsed const content = isCollapsed
@ -75,23 +82,14 @@ function ChangeEntry({
onClick={handleEntryClick} onClick={handleEntryClick}
onMouseEnter={onMouseEnter} onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave} onMouseLeave={onMouseLeave}
id={entryId}
> >
<EntryCallout <EntryCallout className={`rp-entry-callout-${entry.type}`} />
className={`rp-entry-callout-${entry.type}`}
style={{
top: entry.screenPos
? entry.screenPos.y + entry.screenPos.height - 1 + 'px'
: undefined,
}}
/>
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
<div <div
className={classnames('rp-entry-indicator', { className={classnames('rp-entry-indicator', {
'rp-entry-indicator-focused': entry.focused, 'rp-entry-indicator-focused': entry.focused,
})} })}
style={{
top: entry.screenPos ? entry.screenPos.y + 'px' : undefined,
}}
onClick={onIndicatorClick} onClick={onIndicatorClick}
> >
{entry.type === 'insert' ? ( {entry.type === 'insert' ? (
@ -104,10 +102,6 @@ function ChangeEntry({
className={classnames('rp-entry', `rp-entry-${entry.type}`, { className={classnames('rp-entry', `rp-entry-${entry.type}`, {
'rp-entry-focused': entry.focused, 'rp-entry-focused': entry.focused,
})} })}
style={{
top: entry.screenPos ? entry.screenPos.y + 'px' : undefined,
visibility: entry.visible ? 'visible' : 'hidden',
}}
> >
<div className="rp-entry-body"> <div className="rp-entry-body">
<div className="rp-entry-action-icon"> <div className="rp-entry-action-icon">

View file

@ -1,4 +1,4 @@
import { useState, useRef } from 'react' import { useState, useRef, useEffect } from 'react'
import { useTranslation } from 'react-i18next' import { useTranslation } from 'react-i18next'
import EntryContainer from './entry-container' import EntryContainer from './entry-container'
import EntryCallout from './entry-callout' import EntryCallout from './entry-callout'
@ -8,7 +8,10 @@ import AutoExpandingTextArea, {
resetHeight, resetHeight,
} from '../../../../../shared/components/auto-expanding-text-area' } from '../../../../../shared/components/auto-expanding-text-area'
import Icon from '../../../../../shared/components/icon' import Icon from '../../../../../shared/components/icon'
import { useReviewPanelValueContext } from '../../../context/review-panel/review-panel-context' import {
useReviewPanelUpdaterFnsContext,
useReviewPanelValueContext,
} from '../../../context/review-panel/review-panel-context'
import classnames from 'classnames' import classnames from 'classnames'
import { ReviewPanelCommentEntry } from '../../../../../../../types/review-panel/entry' import { ReviewPanelCommentEntry } from '../../../../../../../types/review-panel/entry'
import { import {
@ -40,8 +43,9 @@ function CommentEntry({
onIndicatorClick, onIndicatorClick,
}: CommentEntryProps) { }: CommentEntryProps) {
const { t } = useTranslation() const { t } = useTranslation()
const { gotoEntry, resolveComment, submitReply, handleLayoutChange } = const { gotoEntry, resolveComment, submitReply } =
useReviewPanelValueContext() useReviewPanelValueContext()
const { handleLayoutChange } = useReviewPanelUpdaterFnsContext()
const [replyContent, setReplyContent] = useState('') const [replyContent, setReplyContent] = useState('')
const [animating, setAnimating] = useState(false) const [animating, setAnimating] = useState(false)
const [resolved, setResolved] = useState(false) const [resolved, setResolved] = useState(false)
@ -103,12 +107,24 @@ function CommentEntry({
} }
} }
const submitting = Boolean(thread?.submitting)
// Update the layout when loading finishes
useEffect(() => {
if (!submitting) {
// Ensure everything is rendered in the DOM before updating the layout.
// Having to use a timeout seems less than ideal.
window.setTimeout(handleLayoutChange, 0)
}
}, [submitting, handleLayoutChange])
if (!thread || resolved) { if (!thread || resolved) {
return null return null
} }
return ( return (
<EntryContainer <EntryContainer
id={entryId}
onClick={handleEntryClick} onClick={handleEntryClick}
onMouseEnter={onMouseEnter} onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave} onMouseLeave={onMouseLeave}
@ -119,22 +135,12 @@ function CommentEntry({
'rp-comment-wrapper-resolving': animating, 'rp-comment-wrapper-resolving': animating,
})} })}
> >
<EntryCallout <EntryCallout className="rp-entry-callout-comment" />
className="rp-entry-callout-comment"
style={{
top: entry.screenPos
? entry.screenPos.y + entry.screenPos.height - 1 + 'px'
: undefined,
}}
/>
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
<div <div
className={classnames('rp-entry-indicator', { className={classnames('rp-entry-indicator', {
'rp-entry-indicator-focused': entry.focused, 'rp-entry-indicator-focused': entry.focused,
})} })}
style={{
top: entry.screenPos ? `${entry.screenPos.y}px` : undefined,
}}
onClick={onIndicatorClick} onClick={onIndicatorClick}
> >
<Icon type="comment" /> <Icon type="comment" />
@ -144,13 +150,9 @@ function CommentEntry({
'rp-entry-focused': entry.focused, 'rp-entry-focused': entry.focused,
'rp-entry-comment-resolving': animating, 'rp-entry-comment-resolving': animating,
})} })}
style={{
top: entry.screenPos ? `${entry.screenPos.y}px` : undefined,
visibility: entry.visible ? 'visible' : 'hidden',
}}
ref={entryDivRef} ref={entryDivRef}
> >
{!thread.submitting && (!thread || thread.messages.length === 0) && ( {!submitting && (!thread || thread.messages.length === 0) && (
<div className="rp-loading">{t('no_comments')}</div> <div className="rp-loading">{t('no_comments')}</div>
)} )}
<div className="rp-comment-loaded"> <div className="rp-comment-loaded">
@ -163,7 +165,7 @@ function CommentEntry({
/> />
))} ))}
</div> </div>
{thread.submitting && ( {submitting && (
<div className="rp-loading"> <div className="rp-loading">
<Icon type="spinner" spin /> <Icon type="spinner" spin />
</div> </div>

View file

@ -2,7 +2,10 @@ import { useTranslation } from 'react-i18next'
import { useState } from 'react' import { useState } from 'react'
import AutoExpandingTextArea from '../../../../../shared/components/auto-expanding-text-area' import AutoExpandingTextArea from '../../../../../shared/components/auto-expanding-text-area'
import { formatTime } from '../../../../utils/format-date' import { formatTime } from '../../../../utils/format-date'
import { useReviewPanelValueContext } from '../../../context/review-panel/review-panel-context' import {
useReviewPanelUpdaterFnsContext,
useReviewPanelValueContext,
} from '../../../context/review-panel/review-panel-context'
import { ReviewPanelCommentThread } from '../../../../../../../types/review-panel/comment-thread' import { ReviewPanelCommentThread } from '../../../../../../../types/review-panel/comment-thread'
import { import {
ReviewPanelCommentThreadMessage, ReviewPanelCommentThreadMessage,
@ -17,8 +20,8 @@ type CommentProps = {
function Comment({ thread, threadId, comment }: CommentProps) { function Comment({ thread, threadId, comment }: CommentProps) {
const { t } = useTranslation() const { t } = useTranslation()
const { deleteComment, handleLayoutChange, saveEdit } = const { deleteComment, saveEdit } = useReviewPanelValueContext()
useReviewPanelValueContext() const { handleLayoutChange } = useReviewPanelUpdaterFnsContext()
const [deleting, setDeleting] = useState(false) const [deleting, setDeleting] = useState(false)
const [editing, setEditing] = useState(false) const [editing, setEditing] = useState(false)

View file

@ -1,7 +1,17 @@
import classnames from 'classnames' import classnames from 'classnames'
function EntryContainer({ className, ...rest }: React.ComponentProps<'div'>) { function EntryContainer({
return <div className={classnames('rp-entry-wrapper', className)} {...rest} /> id,
className,
...rest
}: React.ComponentProps<'div'>) {
return (
<div
className={classnames('rp-entry-wrapper', className)}
data-entry-id={id}
{...rest}
/>
)
} }
export default EntryContainer export default EntryContainer

View file

@ -6,14 +6,28 @@ import {
useReviewPanelUpdaterFnsContext, useReviewPanelUpdaterFnsContext,
} from '../../context/review-panel/review-panel-context' } from '../../context/review-panel/review-panel-context'
import { isCurrentFileView, isOverviewView } from '../../utils/sub-view' import { isCurrentFileView, isOverviewView } from '../../utils/sub-view'
import { useCallback } from 'react'
import { useResizeObserver } from '../../../../shared/hooks/use-resize-observer'
function Nav() { function Nav() {
const { t } = useTranslation() const { t } = useTranslation()
const { subView } = useReviewPanelValueContext() const { subView } = useReviewPanelValueContext()
const { handleSetSubview } = useReviewPanelUpdaterFnsContext() const { handleSetSubview, setNavHeight } = useReviewPanelUpdaterFnsContext()
const handleResize = useCallback(
el => {
// Use requestAnimationFrame to prevent errors like "ResizeObserver loop
// completed with undelivered notifications" that occur if onResize does
// something complicated. The cost of this is that onResize lags one frame
// behind, but it's unlikely to matter.
const height = el.offsetHeight
window.requestAnimationFrame(() => setNavHeight(height))
},
[setNavHeight]
)
const resizeRef = useResizeObserver(handleResize)
return ( return (
<div className="rp-nav" role="tablist"> <div ref={resizeRef} className="rp-nav" role="tablist">
<button <button
type="button" type="button"
id="review-panel-tab-current-file" id="review-panel-tab-current-file"

View file

@ -82,6 +82,7 @@ function OverviewFile({ docId, docPath }: OverviewFileProps) {
key={id} key={id}
docId={docId} docId={docId}
entry={entry} entry={entry}
entryId={id}
permissions={permissions} permissions={permissions}
user={users[entry.metadata.user_id]} user={users[entry.metadata.user_id]}
/> />
@ -94,6 +95,7 @@ function OverviewFile({ docId, docPath }: OverviewFileProps) {
key={id} key={id}
docId={docId} docId={docId}
entry={entry} entry={entry}
entryId={id}
permissions={permissions} permissions={permissions}
user={users[entry.metadata.user_id]} user={users[entry.metadata.user_id]}
/> />

View file

@ -0,0 +1,409 @@
import { useEffect, useRef } from 'react'
import { useLayoutContext } from '../../../../shared/context/layout-context'
import {
ReviewPanelEntry,
ReviewPanelEntryScreenPos,
} from '../../../../../../types/review-panel/entry'
import useScopeValue from '../../../../shared/hooks/use-scope-value'
import { debugConsole } from '../../../../utils/debugging'
import { useReviewPanelValueContext } from '../../context/review-panel/review-panel-context'
import useEventListener from '../../../../shared/hooks/use-event-listener'
import { ReviewPanelDocEntries } from '../../../../../../types/review-panel/review-panel'
import { dispatchReviewPanelLayout } from '../../extensions/changes/change-manager'
type EntryView = {
wrapper: HTMLElement
indicator: HTMLElement | null
box: HTMLElement
callout: HTMLElement
layout: HTMLElement
height: number
previousEntryTop: number | null
previousCalloutTop: number | null
entry: ReviewPanelEntry
visible: boolean
positions?: {
entryTop: number
callout: { top: number; height: number; inverted: boolean }
}
}
function css(el: HTMLElement, props: React.CSSProperties) {
Object.assign(el.style, props)
}
function applyEntryVisibility(entryView: EntryView) {
const visible = !!entryView.entry.screenPos
entryView.wrapper.classList.toggle('rp-entry-hidden', !visible)
return visible
}
function calculateCalloutPosition(
screenPos: ReviewPanelEntryScreenPos,
entryTop: number,
lineHeight: number
) {
const height = screenPos.height ?? lineHeight
const originalTop = screenPos.y
const inverted = entryTop <= originalTop
return {
top: inverted ? entryTop + height : originalTop + height - 1,
height: Math.abs(entryTop - originalTop),
inverted,
}
}
const calculateEntryViewPositions = (
entryViews: EntryView[],
lineHeight: number,
calculateTop: (originalTop: number, height: number) => number
) => {
for (const entryView of entryViews) {
const entryVisible = applyEntryVisibility(entryView)
if (entryVisible) {
const entryTop = calculateTop(
entryView.entry.screenPos.y,
entryView.height
)
const callout = calculateCalloutPosition(
entryView.entry.screenPos,
entryTop,
lineHeight
)
entryView.positions = {
entryTop,
callout,
}
}
debugConsole.log('ENTRY', { entry: entryView.entry, top })
}
}
type PositionedEntriesProps = {
entries: Array<[keyof ReviewPanelDocEntries, ReviewPanelEntry]>
contentHeight: number
children: React.ReactNode
}
function PositionedEntries({
entries,
contentHeight,
children,
}: PositionedEntriesProps) {
const { navHeight, toolbarHeight } = useReviewPanelValueContext()
const containerRef = useRef<HTMLDivElement | null>(null)
const { reviewPanelOpen } = useLayoutContext()
const [lineHeight] = useScopeValue<number>(
'reviewPanel.rendererData.lineHeight'
)
const animationTimerRef = useRef<number | null>(null)
const previousFocusedEntryIndexRef = useRef(0)
const layout = () => {
const container = containerRef.current
if (!container) {
return
}
const padding = reviewPanelOpen ? 8 : 4
const toolbarPaddedHeight = reviewPanelOpen ? toolbarHeight + 6 : 0
const navPaddedHeight = reviewPanelOpen ? navHeight + 4 : 0
// Create a list of entry views, typing together DOM elements and model.
// No measuring or style change is done at this point.
const entryViews: EntryView[] = []
// TODO: Look into tying the entry to the DOM element without going via a DOM data attribute
for (const wrapper of container.querySelectorAll<HTMLElement>(
'.rp-entry-wrapper'
)) {
const entryId = wrapper.dataset.entryId
if (!entryId) {
throw new Error('Could not find an entry ID')
}
const entry = entries.find(value => value[0] === entryId)?.[1]
if (!entry) {
throw new Error(`Could not find an entry for ID ${entryId}`)
}
const indicator = wrapper.querySelector<HTMLElement>(
'.rp-entry-indicator'
)
const box = wrapper.querySelector<HTMLElement>('.rp-entry')
const callout = wrapper.querySelector<HTMLElement>('.rp-entry-callout')
const layoutElement = reviewPanelOpen ? box : indicator
if (box && callout && layoutElement) {
const previousEntryTopData = box.dataset.previousEntryTop
const previousCalloutTopData = callout.dataset.previousEntryTop
entryViews.push({
wrapper,
indicator,
box,
callout,
layout: layoutElement,
visible: !!entry.screenPos,
height: 0,
previousEntryTop:
previousEntryTopData !== undefined
? parseInt(previousEntryTopData)
: null,
previousCalloutTop:
previousCalloutTopData !== undefined
? parseInt(previousCalloutTopData)
: null,
entry,
})
} else {
debugConsole.log(
'Entry wrapper is missing indicator, box or callout, so ignoring',
wrapper
)
}
}
if (entryViews.length === 0) {
return
}
entryViews.sort((a, b) => a.entry.offset - b.entry.offset)
// Do the DOM interaction in three phases:
//
// - Apply the `display` property to all elements whose visibility has
// changed. This needs to happen first in order to measure heights.
// - Measure the height of each entry
// - Move each entry without animation to their original position
// relative to the editor content
// - In the next animation frame, re-enable animation and position each
// entry
//
// The idea is to batch DOM reads and writes to avoid layout thrashing. In
// this case, the best we can do is a write phase, a read phase then a
// final write phase.
// See https://web.dev/avoid-large-complex-layouts-and-layout-thrashing/
// First, update visibility for each entry that needs it
for (const entryView of entryViews) {
entryView.wrapper.classList.toggle('rp-entry-hidden', !entryView.visible)
}
// Next, measure the height of each entry
for (const entryView of entryViews) {
if (entryView.visible) {
entryView.height = entryView.layout.offsetHeight
}
}
// Calculate positions for all visible entries
let focusedEntryIndex = entryViews.findIndex(view => view.entry.focused)
if (focusedEntryIndex === -1) {
focusedEntryIndex = Math.min(
previousFocusedEntryIndexRef.current,
entryViews.length - 1
)
}
previousFocusedEntryIndexRef.current = focusedEntryIndex
const focusedEntryView = entryViews[focusedEntryIndex]
if (!focusedEntryView.entry.screenPos) {
return
}
// If the focused entry has no screenPos, we can't position other
// entryViews relative to it, so we position all other entryViews as
// though the focused entry is at the top and the rest follow it
const entryViewsAfter = focusedEntryView.visible
? entryViews.slice(focusedEntryIndex + 1)
: [...entryViews]
const entryViewsBefore = focusedEntryView.visible
? entryViews.slice(0, focusedEntryIndex).reverse() // Work through backwards, starting with the one just above
: []
debugConsole.log('focusedEntryIndex', focusedEntryIndex)
let lastEntryBottom = 0
let firstEntryTop = 0
// Put the focused entry as close to where it wants to be as possible
if (focusedEntryView.visible) {
const focusedEntryScreenPos = focusedEntryView.entry.screenPos
const entryTop = Math.max(focusedEntryScreenPos.y, toolbarPaddedHeight)
const callout = calculateCalloutPosition(
focusedEntryScreenPos,
entryTop,
lineHeight
)
focusedEntryView.positions = {
entryTop,
callout,
}
lastEntryBottom = entryTop + focusedEntryView.height
firstEntryTop = entryTop
}
// Calculate positions for entries that are below the focused entry
calculateEntryViewPositions(
entryViewsAfter,
lineHeight,
(originalTop: number, height: number) => {
const top = Math.max(originalTop, lastEntryBottom + padding)
lastEntryBottom = top + height
return top
}
)
// Calculate positions for entries that are above the focused entry
calculateEntryViewPositions(
entryViewsBefore,
lineHeight,
(originalTop: number, height: number) => {
const originalBottom = originalTop + height
const bottom = Math.min(originalBottom, firstEntryTop - padding)
const top = bottom - height
firstEntryTop = top
return top
}
)
// Calculate the new top overflow
const overflowTop = Math.max(0, toolbarPaddedHeight - firstEntryTop)
// Position everything where it was before, taking into account the new top
// overflow
const moveEntriesToInitialPosition = () => {
// Prevent CSS animation of position for this phase
container.classList.add('no-animate')
for (const entryView of entryViews) {
const { callout: calloutEl, positions } = entryView
if (positions) {
const { entryTop, callout } = positions
// Position the main wrapper in its original position, if it had
// one, or its new position otherwise
const entryTopInitial =
entryView.previousEntryTop === null
? entryTop
: entryView.previousEntryTop
css(entryView.box, {
top: entryTopInitial + overflowTop + 'px',
// The entry element is invisible by default, to avoid flickering
// when positioning for the first time. Here we make sure it becomes
// visible after having a "top" value.
visibility: 'visible',
})
if (entryView.indicator) {
entryView.indicator.style.top = entryTopInitial + overflowTop + 'px'
}
entryView.box.dataset.previousEntryTop = entryTopInitial + ''
// Position the callout element in its original position, if it had
// one, or its new position otherwise
calloutEl.classList.toggle(
'rp-entry-callout-inverted',
callout.inverted
)
const calloutTopInitial =
entryView.previousCalloutTop === null
? callout.top
: entryView.previousCalloutTop
css(calloutEl, {
top: calloutTopInitial + overflowTop + 'px',
height: callout.height + 'px',
})
entryView.box.dataset.previousCalloutTop = calloutTopInitial + ''
}
}
}
const moveToFinalPositions = () => {
// Re-enable CSS animation of position for this phase
container.classList.remove('no-animate')
for (const entryView of entryViews) {
const { callout: calloutEl, positions } = entryView
if (positions) {
const { entryTop, callout } = positions
// Position the main wrapper, if it's moved
if (entryView.previousEntryTop !== entryTop) {
entryView.box.style.top = entryTop + overflowTop + 'px'
}
entryView.box.dataset.previousEntryTop = entryTop + ''
if (entryView.indicator) {
entryView.indicator.style.top = entryTop + overflowTop + 'px'
}
// Position the callout element
if (entryView.previousCalloutTop !== callout.top) {
calloutEl.style.top = callout.top + overflowTop + 'px'
entryView.callout.dataset.previousCalloutTop = callout.top + ''
}
}
}
animationTimerRef.current = null
}
moveEntriesToInitialPosition()
// Inform the editor of the new top overflow
window.dispatchEvent(
new CustomEvent('review-panel:event', {
detail: {
type: 'sizes',
payload: {
overflowTop,
height: lastEntryBottom + navPaddedHeight,
},
},
})
)
// Schedule the final, animated move
animationTimerRef.current = window.setTimeout(moveToFinalPositions, 60)
}
useEventListener('review-panel:layout', () => {
if (animationTimerRef.current) {
window.clearTimeout(animationTimerRef.current)
animationTimerRef.current = null
}
layout()
})
// Cancel scheduled move on unmount
useEffect(() => {
return () => {
if (animationTimerRef.current) {
window.clearTimeout(animationTimerRef.current)
animationTimerRef.current = null
}
}
}, [])
// Layout on first render. This is necessary to ensure layout happens when
// switching from overview to current file view
useEffect(() => {
dispatchReviewPanelLayout()
}, [])
return (
<div
ref={containerRef}
className="rp-entry-list-react"
style={{ height: `${contentHeight}px` }}
>
{children}
</div>
)
}
export default PositionedEntries

View file

@ -11,12 +11,13 @@ import {
} from '../../../../../../../types/review-panel/review-panel' } from '../../../../../../../types/review-panel/review-panel'
import { ReviewPanelResolvedCommentThread } from '../../../../../../../types/review-panel/comment-thread' import { ReviewPanelResolvedCommentThread } from '../../../../../../../types/review-panel/comment-thread'
import { DocId } from '../../../../../../../types/project-settings' import { DocId } from '../../../../../../../types/project-settings'
import { ReviewPanelEntry } from '../../../../../../../types/review-panel/entry'
export interface FilteredResolvedComments export interface FilteredResolvedComments
extends ReviewPanelResolvedCommentThread { extends ReviewPanelResolvedCommentThread {
content: string content: string
threadId: ThreadId threadId: ThreadId
entryId: string entryId: ThreadId
docId: DocId docId: DocId
docName: string | null docName: string | null
} }
@ -56,7 +57,9 @@ function ResolvedCommentsDropdown() {
for (const [docId, docEntries] of Object.entries(resolvedComments) as Array< for (const [docId, docEntries] of Object.entries(resolvedComments) as Array<
[DocId, ReviewPanelDocEntries] [DocId, ReviewPanelDocEntries]
>) { >) {
for (const [entryId, entry] of Object.entries(docEntries)) { for (const [entryId, entry] of Object.entries(docEntries) as Array<
[ThreadId, ReviewPanelEntry]
>) {
if (entry.type === 'comment') { if (entry.type === 'comment') {
const threadId = entry.thread_id const threadId = entry.thread_id
const thread = const thread =

View file

@ -1,9 +1,26 @@
import ResolvedCommentsDropdown from './resolved-comments-dropdown' import ResolvedCommentsDropdown from './resolved-comments-dropdown'
import ToggleMenu from './toggle-menu' import ToggleMenu from './toggle-menu'
import { useReviewPanelUpdaterFnsContext } from '../../../context/review-panel/review-panel-context'
import { useCallback } from 'react'
import { useResizeObserver } from '../../../../../shared/hooks/use-resize-observer'
function Toolbar() { function Toolbar() {
const { setToolbarHeight } = useReviewPanelUpdaterFnsContext()
const handleResize = useCallback(
el => {
// Use requestAnimationFrame to prevent errors like "ResizeObserver loop
// completed with undelivered notifications" that occur if onResize does
// something complicated. The cost of this is that onResize lags one frame
// behind, but it's unlikely to matter.
const height = el.offsetHeight
window.requestAnimationFrame(() => setToolbarHeight(height))
},
[setToolbarHeight]
)
const resizeRef = useResizeObserver(handleResize)
return ( return (
<div className="review-panel-toolbar"> <div ref={resizeRef} className="review-panel-toolbar">
<ResolvedCommentsDropdown /> <ResolvedCommentsDropdown />
<ToggleMenu /> <ToggleMenu />
</div> </div>

View file

@ -1,15 +1,13 @@
import { useState, useMemo, useCallback } from 'react' import { useState, useMemo, useCallback } from 'react'
import useScopeValue from '../../../../../shared/hooks/use-scope-value' import useScopeValue from '../../../../../shared/hooks/use-scope-value'
import useScopeEventEmitter from '../../../../../shared/hooks/use-scope-event-emitter'
import { sendMB } from '../../../../../infrastructure/event-tracking' import { sendMB } from '../../../../../infrastructure/event-tracking'
import { ReviewPanelState } from '../types/review-panel-state' import { ReviewPanelState } from '../types/review-panel-state'
import * as ReviewPanel from '../types/review-panel-state' import * as ReviewPanel from '../types/review-panel-state'
import { SubView } from '../../../../../../../types/review-panel/review-panel' import { SubView } from '../../../../../../../types/review-panel/review-panel'
import { ReviewPanelCommentEntry } from '../../../../../../../types/review-panel/entry' import { ReviewPanelCommentEntry } from '../../../../../../../types/review-panel/entry'
import { dispatchReviewPanelLayout as handleLayoutChange } from '../../../extensions/changes/change-manager'
function useAngularReviewPanelState(): ReviewPanelState { function useAngularReviewPanelState(): ReviewPanelState {
const emitLayoutChange = useScopeEventEmitter('review-panel:layout', false)
const [subView, setSubView] = useScopeValue<ReviewPanel.Value<'subView'>>( const [subView, setSubView] = useScopeValue<ReviewPanel.Value<'subView'>>(
'reviewPanel.subView' 'reviewPanel.subView'
) )
@ -113,12 +111,6 @@ function useAngularReviewPanelState(): ReviewPanelState {
[setSubView] [setSubView]
) )
const handleLayoutChange = useCallback(() => {
window.requestAnimationFrame(() => {
emitLayoutChange()
})
}, [emitLayoutChange])
const submitReply = useCallback( const submitReply = useCallback(
(entry: ReviewPanelCommentEntry, replyContent: string) => { (entry: ReviewPanelCommentEntry, replyContent: string) => {
submitReplyAngular({ ...entry, replyContent }) submitReplyAngular({ ...entry, replyContent })
@ -128,6 +120,8 @@ function useAngularReviewPanelState(): ReviewPanelState {
const [entryHover, setEntryHover] = useState(false) const [entryHover, setEntryHover] = useState(false)
const [isAddingComment, setIsAddingComment] = useState(false) const [isAddingComment, setIsAddingComment] = useState(false)
const [navHeight, setNavHeight] = useState(0)
const [toolbarHeight, setToolbarHeight] = useState(0)
const values = useMemo<ReviewPanelState['values']>( const values = useMemo<ReviewPanelState['values']>(
() => ({ () => ({
@ -139,7 +133,6 @@ function useAngularReviewPanelState(): ReviewPanelState {
entryHover, entryHover,
isAddingComment, isAddingComment,
gotoEntry, gotoEntry,
handleLayoutChange,
loadingThreads, loadingThreads,
nVisibleSelectedChanges, nVisibleSelectedChanges,
permissions, permissions,
@ -148,6 +141,8 @@ function useAngularReviewPanelState(): ReviewPanelState {
resolvedComments, resolvedComments,
saveEdit, saveEdit,
shouldCollapse, shouldCollapse,
navHeight,
toolbarHeight,
submitReply, submitReply,
subView, subView,
wantTrackChanges, wantTrackChanges,
@ -180,7 +175,6 @@ function useAngularReviewPanelState(): ReviewPanelState {
entryHover, entryHover,
isAddingComment, isAddingComment,
gotoEntry, gotoEntry,
handleLayoutChange,
loadingThreads, loadingThreads,
nVisibleSelectedChanges, nVisibleSelectedChanges,
permissions, permissions,
@ -189,6 +183,8 @@ function useAngularReviewPanelState(): ReviewPanelState {
resolvedComments, resolvedComments,
saveEdit, saveEdit,
shouldCollapse, shouldCollapse,
navHeight,
toolbarHeight,
submitReply, submitReply,
subView, subView,
wantTrackChanges, wantTrackChanges,
@ -217,10 +213,13 @@ function useAngularReviewPanelState(): ReviewPanelState {
const updaterFns = useMemo<ReviewPanelState['updaterFns']>( const updaterFns = useMemo<ReviewPanelState['updaterFns']>(
() => ({ () => ({
handleSetSubview, handleSetSubview,
handleLayoutChange,
setEntryHover, setEntryHover,
setCollapsed, setCollapsed,
setShouldCollapse, setShouldCollapse,
setIsAddingComment, setIsAddingComment,
setNavHeight,
setToolbarHeight,
}), }),
[ [
handleSetSubview, handleSetSubview,
@ -228,6 +227,8 @@ function useAngularReviewPanelState(): ReviewPanelState {
setEntryHover, setEntryHover,
setShouldCollapse, setShouldCollapse,
setIsAddingComment, setIsAddingComment,
setNavHeight,
setToolbarHeight,
] ]
) )

View file

@ -24,7 +24,6 @@ export interface ReviewPanelState {
entryHover: boolean entryHover: boolean
isAddingComment: boolean isAddingComment: boolean
gotoEntry: (docId: DocId, entryOffset: number) => void gotoEntry: (docId: DocId, entryOffset: number) => void
handleLayoutChange: () => void
loadingThreads: boolean loadingThreads: boolean
nVisibleSelectedChanges: number nVisibleSelectedChanges: number
permissions: ReviewPanelPermissions permissions: ReviewPanelPermissions
@ -37,6 +36,8 @@ export interface ReviewPanelState {
content: string content: string
) => void ) => void
shouldCollapse: boolean shouldCollapse: boolean
navHeight: number
toolbarHeight: number
submitReply: (entry: ReviewPanelCommentEntry, replyContent: string) => void submitReply: (entry: ReviewPanelCommentEntry, replyContent: string) => void
subView: SubView subView: SubView
wantTrackChanges: boolean wantTrackChanges: boolean
@ -68,6 +69,7 @@ export interface ReviewPanelState {
} }
updaterFns: { updaterFns: {
handleSetSubview: (subView: SubView) => void handleSetSubview: (subView: SubView) => void
handleLayoutChange: () => void
setEntryHover: React.Dispatch<React.SetStateAction<Value<'entryHover'>>> setEntryHover: React.Dispatch<React.SetStateAction<Value<'entryHover'>>>
setIsAddingComment: React.Dispatch< setIsAddingComment: React.Dispatch<
React.SetStateAction<Value<'isAddingComment'>> React.SetStateAction<Value<'isAddingComment'>>
@ -76,6 +78,10 @@ export interface ReviewPanelState {
setShouldCollapse: React.Dispatch< setShouldCollapse: React.Dispatch<
React.SetStateAction<Value<'shouldCollapse'>> React.SetStateAction<Value<'shouldCollapse'>>
> >
setNavHeight: React.Dispatch<React.SetStateAction<Value<'navHeight'>>>
setToolbarHeight: React.Dispatch<
React.SetStateAction<Value<'toolbarHeight'>>
>
} }
} }
/* eslint-enable no-use-before-define */ /* eslint-enable no-use-before-define */

View file

@ -25,6 +25,15 @@ export const dispatchEditorEvent = (type: string, payload?: unknown) => {
}, 0) }, 0)
} }
export const dispatchReviewPanelLayout = () => {
window.dispatchEvent(new CustomEvent('review-panel:layout'))
}
const scheduleDispatchReviewPanelLayout = debounce(
dispatchReviewPanelLayout,
50
)
export type ChangeManager = { export type ChangeManager = {
initialize: () => void initialize: () => void
handleUpdate: (update: ViewUpdate) => void handleUpdate: (update: ViewUpdate) => void
@ -66,6 +75,10 @@ export const createChangeManager = (
const y = Math.round(coords.top - contentRect.top - editorPaddingTop) const y = Math.round(coords.top - contentRect.top - editorPaddingTop)
const height = Math.round(coords.bottom - coords.top) const height = Math.round(coords.bottom - coords.top)
if (!entry.screenPos) {
visibilityChanged = true
}
entry.screenPos = { y, height, editorPaddingTop } entry.screenPos = { y, height, editorPaddingTop }
} }
@ -284,6 +297,12 @@ export const createChangeManager = (
if (changed) { if (changed) {
dispatchEditorEvent('track-changes:visibility_changed') dispatchEditorEvent('track-changes:visibility_changed')
} }
dispatchReviewPanelLayout()
// Ensure the layout is updated again once the review panel entries
// have updated in the React review panel. The use of a timeout is bad
// but the timings are a bit of a mess and will be improved when the
// review panel state is migrated away from Angular
scheduleDispatchReviewPanelLayout()
break break
} }
@ -348,7 +367,6 @@ export const createChangeManager = (
}) })
) )
} }
break break
} }
} }

View file

@ -128,17 +128,17 @@ export default App.controller(
}) })
$scope.$on('layout:pdf:linked', (event, state) => $scope.$on('layout:pdf:linked', (event, state) =>
$scope.$broadcast('review-panel:layout') ide.$scope.$broadcast('review-panel:layout')
) )
$scope.$on('layout:pdf:resize', (event, state) => { $scope.$on('layout:pdf:resize', (event, state) => {
ide.$scope.reviewPanel.layoutToLeft = ide.$scope.reviewPanel.layoutToLeft =
state.east?.size < 220 || state.east?.initClosed state.east?.size < 220 || state.east?.initClosed
$scope.$broadcast('review-panel:layout', false) ide.$scope.$broadcast('review-panel:layout', false)
}) })
$scope.$on('expandable-text-area:resize', event => $scope.$on('expandable-text-area:resize', event =>
$timeout(() => $scope.$broadcast('review-panel:layout')) $timeout(() => ide.$scope.$broadcast('review-panel:layout'))
) )
$scope.$on('review-panel:sizes', (e, sizes) => { $scope.$on('review-panel:sizes', (e, sizes) => {
@ -206,7 +206,7 @@ export default App.controller(
delete thread.submitting delete thread.submitting
thread.messages.push(formatComment(comment)) thread.messages.push(formatComment(comment))
$scope.$apply() $scope.$apply()
return $timeout(() => $scope.$broadcast('review-panel:layout')) return $timeout(() => ide.$scope.$broadcast('review-panel:layout'))
}) })
ide.socket.on('accept-changes', function (doc_id, change_ids) { ide.socket.on('accept-changes', function (doc_id, change_ids) {
@ -321,7 +321,7 @@ export default App.controller(
} }
return $timeout(function () { return $timeout(function () {
$scope.$broadcast('review-panel:toggle') $scope.$broadcast('review-panel:toggle')
return $scope.$broadcast('review-panel:layout', false) return ide.$scope.$broadcast('review-panel:layout', false)
}) })
}) })
@ -559,12 +559,12 @@ export default App.controller(
$scope.$broadcast('review-panel:recalculate-screen-positions') $scope.$broadcast('review-panel:recalculate-screen-positions')
dispatchReviewPanelEvent('recalculate-screen-positions', entries) dispatchReviewPanelEvent('recalculate-screen-positions', entries)
return $scope.$broadcast('review-panel:layout') return ide.$scope.$broadcast('review-panel:layout')
} }
}) })
$scope.$on('editor:track-changes:visibility_changed', () => $scope.$on('editor:track-changes:visibility_changed', () =>
$timeout(() => $scope.$broadcast('review-panel:layout', false)) $timeout(() => ide.$scope.$broadcast('review-panel:layout', false))
) )
$scope.$on( $scope.$on(
@ -576,21 +576,42 @@ export default App.controller(
ide.$scope.reviewPanel.selectedEntryIds = [] ide.$scope.reviewPanel.selectedEntryIds = []
// Count of user-visible changes, i.e. an aggregated change will count as one. // Count of user-visible changes, i.e. an aggregated change will count as one.
ide.$scope.reviewPanel.nVisibleSelectedChanges = 0 ide.$scope.reviewPanel.nVisibleSelectedChanges = 0
delete entries['add-comment']
delete entries['bulk-actions']
const offset = selection_offset_start
const length = selection_offset_end - selection_offset_start
// Recreate the add comment and bulk actions entries only when
// necessary. This is to avoid the UI thinking that these entries have
// changed and getting into an infinite loop.
if (selection) { if (selection) {
const existingAddComment = entries['add-comment']
if (
!existingAddComment ||
existingAddComment.offset !== offset ||
existingAddComment.length !== length
) {
entries['add-comment'] = { entries['add-comment'] = {
type: 'add-comment', type: 'add-comment',
offset: selection_offset_start, offset,
length: selection_offset_end - selection_offset_start, length,
} }
}
const existingBulkActions = entries['bulk-actions']
if (
!existingBulkActions ||
existingBulkActions.offset !== offset ||
existingBulkActions.length !== length
) {
entries['bulk-actions'] = { entries['bulk-actions'] = {
type: 'bulk-actions', type: 'bulk-actions',
offset: selection_offset_start, offset,
length: selection_offset_end - selection_offset_start, length,
} }
} }
} else {
delete entries['add-comment']
delete entries['bulk-actions']
}
for (const id in entries) { for (const id in entries) {
const entry = entries[id] const entry = entries[id]
@ -640,7 +661,11 @@ export default App.controller(
dispatchReviewPanelEvent('recalculate-screen-positions', entries) dispatchReviewPanelEvent('recalculate-screen-positions', entries)
return $scope.$broadcast('review-panel:layout') // Ensure that watchers, such as the React-based review panel component,
// are informed of the changes to entries
ide.$scope.$apply()
return ide.$scope.$broadcast('review-panel:layout')
} }
) )
@ -757,7 +782,7 @@ export default App.controller(
$scope.toggleReviewPanel() $scope.toggleReviewPanel()
} }
return $timeout(function () { return $timeout(function () {
$scope.$broadcast('review-panel:layout') ide.$scope.$broadcast('review-panel:layout')
return $scope.$broadcast('comment:start_adding') return $scope.$broadcast('comment:start_adding')
}) })
} }
@ -765,7 +790,7 @@ export default App.controller(
$scope.startNewComment = function () { $scope.startNewComment = function () {
$scope.$broadcast('comment:select_line') $scope.$broadcast('comment:select_line')
dispatchReviewPanelEvent('comment:select_line') dispatchReviewPanelEvent('comment:select_line')
return $timeout(() => $scope.$broadcast('review-panel:layout')) return $timeout(() => ide.$scope.$broadcast('review-panel:layout'))
} }
ide.$scope.submitNewComment = function (content) { ide.$scope.submitNewComment = function (content) {
@ -800,16 +825,16 @@ export default App.controller(
) )
// TODO: unused? // TODO: unused?
$scope.$broadcast('editor:clearSelection') $scope.$broadcast('editor:clearSelection')
$timeout(() => $scope.$broadcast('review-panel:layout')) $timeout(() => ide.$scope.$broadcast('review-panel:layout'))
eventTracking.sendMB('rp-new-comment', { size: content.length }) eventTracking.sendMB('rp-new-comment', { size: content.length })
} }
$scope.cancelNewComment = entry => $scope.cancelNewComment = entry =>
$timeout(() => $scope.$broadcast('review-panel:layout')) $timeout(() => ide.$scope.$broadcast('review-panel:layout'))
$scope.startReply = function (entry) { $scope.startReply = function (entry) {
entry.replying = true entry.replying = true
return $timeout(() => $scope.$broadcast('review-panel:layout')) return $timeout(() => ide.$scope.$broadcast('review-panel:layout'))
} }
ide.$scope.submitReply = function (entry, entry_id) { ide.$scope.submitReply = function (entry, entry_id) {
@ -839,14 +864,14 @@ export default App.controller(
thread.submitting = true thread.submitting = true
entry.replyContent = '' entry.replyContent = ''
entry.replying = false entry.replying = false
$timeout(() => $scope.$broadcast('review-panel:layout')) $timeout(() => ide.$scope.$broadcast('review-panel:layout'))
eventTracking.sendMB('rp-comment-reply', trackingMetadata) eventTracking.sendMB('rp-comment-reply', trackingMetadata)
} }
$scope.cancelReply = function (entry) { $scope.cancelReply = function (entry) {
entry.replying = false entry.replying = false
entry.replyContent = '' entry.replyContent = ''
return $scope.$broadcast('review-panel:layout') return ide.$scope.$broadcast('review-panel:layout')
} }
ide.$scope.resolveComment = function (doc_id, entry_id) { ide.$scope.resolveComment = function (doc_id, entry_id) {
@ -947,7 +972,7 @@ export default App.controller(
_csrf: window.csrfToken, _csrf: window.csrfToken,
} }
) )
return $timeout(() => $scope.$broadcast('review-panel:layout')) return $timeout(() => ide.$scope.$broadcast('review-panel:layout'))
} }
ide.$scope.deleteComment = function (thread_id, comment_id) { ide.$scope.deleteComment = function (thread_id, comment_id) {
@ -959,7 +984,7 @@ export default App.controller(
'X-CSRF-Token': window.csrfToken, 'X-CSRF-Token': window.csrfToken,
}, },
}) })
return $timeout(() => $scope.$broadcast('review-panel:layout')) return $timeout(() => ide.$scope.$broadcast('review-panel:layout'))
} }
$scope.setSubView = function (subView) { $scope.setSubView = function (subView) {
@ -1244,7 +1269,7 @@ export default App.controller(
} }
ide.$scope.reviewPanel.commentThreads = threads ide.$scope.reviewPanel.commentThreads = threads
dispatchReviewPanelEvent('loaded_threads') dispatchReviewPanelEvent('loaded_threads')
return $timeout(() => $scope.$broadcast('review-panel:layout')) return $timeout(() => ide.$scope.$broadcast('review-panel:layout'))
}) })
} }
@ -1310,7 +1335,7 @@ export default App.controller(
switch (type) { switch (type) {
case 'line-height': { case 'line-height': {
ide.$scope.reviewPanel.rendererData.lineHeight = payload ide.$scope.reviewPanel.rendererData.lineHeight = payload
$scope.$broadcast('review-panel:layout') ide.$scope.$broadcast('review-panel:layout')
break break
} }

View file

@ -50,7 +50,11 @@ function AutoExpandingTextArea({
if (isFirstResize) { if (isFirstResize) {
isFirstResize = false isFirstResize = false
} else { } else {
onResize() // Prevent errors like "ResizeObserver loop completed with undelivered
// notifications" that occur if onResize does something complicated.
// The cost of this is that onResize lags one frame behind, but it's
// unlikely to matter.
window.requestAnimationFrame(onResize)
} }
}) })

View file

@ -0,0 +1,7 @@
type DebugConsole = { log(...data: any[]): void }
export const debugging =
new URLSearchParams(window.location.search).get('debug') === 'true'
export const debugConsole: DebugConsole = debugging
? console
: { log: () => {} }

View file

@ -27,6 +27,8 @@
@rp-toolbar-height: 32px; @rp-toolbar-height: 32px;
@rp-entry-animation-speed: 0.3s; @rp-entry-animation-speed: 0.3s;
// Move a little faster in React to compensate for the delay before moving the vertical position
@rp-entry-animation-speed-react: 0.2s;
.rp-button() { .rp-button() {
display: block; // IE doesn't do flex with inline items. display: block; // IE doesn't do flex with inline items.
@ -65,6 +67,8 @@
} }
#review-panel { #review-panel {
--rp-animation-speed: @rp-entry-animation-speed;
display: block; display: block;
.rp-size-expanded & { .rp-size-expanded & {
@ -248,10 +252,10 @@
border-radius: 3px; border-radius: 3px;
color: #fff; color: #fff;
cursor: pointer; cursor: pointer;
transition: top @rp-entry-animation-speed, left 0.1s, right 0.1s; transition: top var(--rp-animation-speed), left 0.1s, right 0.1s;
.no-animate & { .no-animate & {
transition: none; transition: left 0.1s, right 0.1s;
} }
&-focused { &-focused {
@ -372,10 +376,10 @@
border-left: solid @rp-entry-ribbon-width transparent; border-left: solid @rp-entry-ribbon-width transparent;
border-radius: 3px; border-radius: 3px;
background-color: #fff; background-color: #fff;
transition: top @rp-entry-animation-speed, left 0.1s, right 0.1s; transition: top var(--rp-animation-speed), left 0.1s, right 0.1s;
.no-animate & { .no-animate & {
transition: none; transition: left 0.1s, right 0.1s;
} }
&-insert, &-insert,
@ -640,7 +644,7 @@
} }
.rp-entry-callout { .rp-entry-callout {
transition: top @rp-entry-animation-speed, height @rp-entry-animation-speed; transition: top var(--rp-animation-speed), height var(--rp-animation-speed);
.rp-state-current-file & { .rp-state-current-file & {
position: absolute; position: absolute;
@ -1193,6 +1197,8 @@ button when (@is-overleaf-light = true) {
// CM6-specific review panel rules // CM6-specific review panel rules
.ol-cm-review-panel { .ol-cm-review-panel {
--rp-animation-speed: @rp-entry-animation-speed-react;
position: relative; position: relative;
z-index: 6; z-index: 6;
display: block; display: block;
@ -1254,6 +1260,17 @@ button when (@is-overleaf-light = true) {
bottom: 0; bottom: 0;
} }
.rp-entry-list-react {
position: relative;
.rp-entry-list-react-inner {
position: absolute;
top: 0;
left: 0;
right: 0;
}
}
.rp-state-current-file & { .rp-state-current-file & {
.review-panel-tools { .review-panel-tools {
display: flex; display: flex;

View file

@ -1,6 +1,6 @@
import { ThreadId, UserId } from './review-panel' import { ThreadId, UserId } from './review-panel'
interface ReviewPanelEntryScreenPos { export interface ReviewPanelEntryScreenPos {
y: number y: number
height: number height: number
editorPaddingTop: number editorPaddingTop: number

View file

@ -1,6 +1,10 @@
import { Brand } from '../helpers/brand' import { Brand } from '../helpers/brand'
import { DocId } from '../project-settings' import { DocId } from '../project-settings'
import { ReviewPanelEntry } from './entry' import {
ReviewPanelAddCommentEntry,
ReviewPanelBulkActionsEntry,
ReviewPanelEntry,
} from './entry'
import { ReviewPanelCommentThread } from './comment-thread' import { ReviewPanelCommentThread } from './comment-thread'
export type SubView = 'cur_file' | 'overview' export type SubView = 'cur_file' | 'overview'
@ -13,7 +17,15 @@ export interface ReviewPanelPermissions {
} }
export type ThreadId = Brand<string, 'ThreadId'> export type ThreadId = Brand<string, 'ThreadId'>
export type ReviewPanelDocEntries = Record<ThreadId, ReviewPanelEntry> // Entries may contain `add-comment` and `bulk-actions` props along with DocIds
// Ideally the `add-comment` and `bulk-actions` objects should not be within the entries object
// as the doc data, but this is what currently angular returns.
export type ReviewPanelDocEntries = Record<
| ThreadId
| ReviewPanelAddCommentEntry['type']
| ReviewPanelBulkActionsEntry['type'],
ReviewPanelEntry
>
export type ReviewPanelEntries = Record<DocId, ReviewPanelDocEntries> export type ReviewPanelEntries = Record<DocId, ReviewPanelDocEntries>
@ -27,6 +39,7 @@ export interface ReviewPanelUser {
isSelf: boolean isSelf: boolean
name: string name: string
} }
export type ReviewPanelUsers = Record<UserId, ReviewPanelUser> export type ReviewPanelUsers = Record<UserId, ReviewPanelUser>
export type CommentId = Brand<string, 'CommentId'> export type CommentId = Brand<string, 'CommentId'>