Merge pull request #13997 from overleaf/td-review-panel-mini-hover

React review panel: make hovering an entry in the mini review panel display correctly

GitOrigin-RevId: 41a40e4047b89d2a8db15b2e12baf04c11b78e21
This commit is contained in:
Tim Down 2023-07-24 11:21:34 +01:00 committed by Copybot
parent 8623b4d0f4
commit f2b5ac23f8
13 changed files with 249 additions and 127 deletions

View file

@ -1,4 +1,4 @@
import { useCallback, useMemo } from 'react'
import { useMemo } from 'react'
import Container from './container'
import Toolbar from './toolbar/toolbar'
import Nav from './nav'
@ -9,10 +9,7 @@ import CommentEntry from './entries/comment-entry'
import AddCommentEntry from './entries/add-comment-entry'
import BulkActionsEntry from './entries/bulk-actions-entry/bulk-actions-entry'
import PositionedEntries from './positioned-entries'
import {
useReviewPanelUpdaterFnsContext,
useReviewPanelValueContext,
} from '../../context/review-panel/review-panel-context'
import { useReviewPanelValueContext } from '../../context/review-panel/review-panel-context'
import useCodeMirrorContentHeight from '../../hooks/use-codemirror-content-height'
import { ReviewPanelEntry } from '../../../../../../types/review-panel/entry'
import {
@ -35,7 +32,6 @@ function CurrentFileContainer() {
entryHover,
nVisibleSelectedChanges: nChanges,
} = useReviewPanelValueContext()
const { setEntryHover, toggleReviewPanel } = useReviewPanelUpdaterFnsContext()
const contentHeight = useCodeMirrorContentHeight()
const currentDocEntries =
@ -47,14 +43,6 @@ function CurrentFileContainer() {
>
}, [currentDocEntries])
const onMouseEnter = useCallback(() => {
setEntryHover(true)
}, [setEntryHover])
const onMouseLeave = useCallback(() => {
setEntryHover(false)
}, [setEntryHover])
return (
<Container
classNames={{
@ -100,9 +88,6 @@ function CurrentFileContainer() {
focused={entry.focused}
entryIds={entry.entry_ids}
timestamp={entry.metadata.ts}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
onIndicatorClick={toggleReviewPanel}
/>
)
}
@ -121,9 +106,6 @@ function CurrentFileContainer() {
focused={entry.focused}
entryIds={entry.entry_ids}
timestamp={entry.metadata.ts}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
onIndicatorClick={toggleReviewPanel}
/>
)
}
@ -143,9 +125,6 @@ function CurrentFileContainer() {
offset={entry.offset}
focused={entry.focused}
permissions={permissions}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
onIndicatorClick={toggleReviewPanel}
/>
)
}

View file

@ -9,6 +9,8 @@ import { formatTime } from '../../../../utils/format-date'
import classnames from 'classnames'
import comparePropsWithShallowArrayCompare from '../utils/compare-props-with-shallow-array-compare'
import { BaseChangeEntryProps } from '../types/base-change-entry-props'
import useIndicatorHover from '../hooks/use-indicator-hover'
import EntryIndicator from './entry-indicator'
interface AggregateChangeEntryProps extends BaseChangeEntryProps {
replacedContent: string
@ -26,15 +28,19 @@ function AggregateChangeEntry({
entryIds,
timestamp,
contentLimit = 17,
onMouseEnter,
onMouseLeave,
onIndicatorClick,
}: AggregateChangeEntryProps) {
const { t } = useTranslation()
const { acceptChanges, rejectChanges, gotoEntry, handleLayoutChange } =
useReviewPanelUpdaterFnsContext()
const [isDeletionCollapsed, setIsDeletionCollapsed] = useState(true)
const [isInsertionCollapsed, setIsInsertionCollapsed] = useState(true)
const {
hoverCoords,
indicatorRef,
handleEntryMouseLeave,
handleIndicatorMouseEnter,
handleIndicatorClick,
} = useIndicatorHover()
const deletionNeedsCollapsing = replacedContent.length > contentLimit
const insertionNeedsCollapsing = content.length > contentLimit
@ -76,20 +82,19 @@ function AggregateChangeEntry({
return (
<EntryContainer
id={entryId}
hoverCoords={hoverCoords}
onClick={handleEntryClick}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
onMouseLeave={handleEntryMouseLeave}
>
<EntryCallout className="rp-entry-callout-aggregate" />
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */}
<div
className={classnames('rp-entry-indicator', {
'rp-entry-indicator-focused': focused,
})}
onClick={onIndicatorClick}
<EntryIndicator
ref={indicatorRef}
focused={focused}
onMouseEnter={handleIndicatorMouseEnter}
onClick={handleIndicatorClick}
>
<Icon type="pencil" />
</div>
</EntryIndicator>
<div
className={classnames('rp-entry', 'rp-entry-aggregate', {
'rp-entry-focused': focused,

View file

@ -10,6 +10,8 @@ import classnames from 'classnames'
import { ReviewPanelChangeEntry } from '../../../../../../../types/review-panel/entry'
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'
interface ChangeEntryProps extends BaseChangeEntryProps {
type: ReviewPanelChangeEntry['type']
@ -27,14 +29,18 @@ function ChangeEntry({
entryIds,
timestamp,
contentLimit = 40,
onMouseEnter,
onMouseLeave,
onIndicatorClick,
}: ChangeEntryProps) {
const { t } = useTranslation()
const { handleLayoutChange, acceptChanges, rejectChanges, gotoEntry } =
useReviewPanelUpdaterFnsContext()
const [isCollapsed, setIsCollapsed] = useState(true)
const {
hoverCoords,
indicatorRef,
handleEntryMouseLeave,
handleIndicatorMouseEnter,
handleIndicatorClick,
} = useIndicatorHover()
const contentToDisplay = isCollapsed
? content.substring(0, contentLimit)
@ -66,21 +72,20 @@ function ChangeEntry({
return (
<EntryContainer
onClick={handleEntryClick}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
id={entryId}
hoverCoords={hoverCoords}
onClick={handleEntryClick}
onMouseLeave={handleEntryMouseLeave}
>
<EntryCallout className={`rp-entry-callout-${type}`} />
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
<div
className={classnames('rp-entry-indicator', {
'rp-entry-indicator-focused': focused,
})}
onClick={onIndicatorClick}
<EntryIndicator
ref={indicatorRef}
focused={focused}
onMouseEnter={handleIndicatorMouseEnter}
onClick={handleIndicatorClick}
>
{isInsert ? <Icon type="pencil" /> : <i className="rp-icon-delete" />}
</div>
</EntryIndicator>
<div
className={classnames('rp-entry', `rp-entry-${type}`, {
'rp-entry-focused': focused,

View file

@ -17,6 +17,8 @@ import {
import { DocId } from '../../../../../../../types/project-settings'
import { ReviewPanelCommentThread } from '../../../../../../../types/review-panel/comment-thread'
import { ReviewPanelCommentEntry } from '../../../../../../../types/review-panel/entry'
import useIndicatorHover from '../hooks/use-indicator-hover'
import EntryIndicator from './entry-indicator'
type CommentEntryProps = {
docId: DocId
@ -24,9 +26,6 @@ type CommentEntryProps = {
thread: ReviewPanelCommentThread | undefined
threadId: ReviewPanelCommentEntry['thread_id']
permissions: ReviewPanelPermissions
onMouseEnter?: () => void
onMouseLeave?: () => void
onIndicatorClick?: () => void
} & Pick<ReviewPanelCommentEntry, 'offset' | 'focused'>
function CommentEntry({
@ -37,9 +36,6 @@ function CommentEntry({
offset,
focused,
permissions,
onMouseEnter,
onMouseLeave,
onIndicatorClick,
}: CommentEntryProps) {
const { t } = useTranslation()
const { gotoEntry, resolveComment, submitReply, handleLayoutChange } =
@ -48,6 +44,13 @@ function CommentEntry({
const [animating, setAnimating] = useState(false)
const [resolved, setResolved] = useState(false)
const entryDivRef = useRef<HTMLDivElement | null>(null)
const {
hoverCoords,
indicatorRef,
handleEntryMouseLeave,
handleIndicatorMouseEnter,
handleIndicatorClick,
} = useIndicatorHover()
const handleEntryClick = (e: React.MouseEvent<HTMLDivElement>) => {
const target = e.target as Element
@ -120,9 +123,9 @@ function CommentEntry({
return (
<EntryContainer
id={entryId}
hoverCoords={hoverCoords}
onClick={handleEntryClick}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
onMouseLeave={handleEntryMouseLeave}
>
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
<div
@ -131,15 +134,14 @@ function CommentEntry({
})}
>
<EntryCallout className="rp-entry-callout-comment" />
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
<div
className={classnames('rp-entry-indicator', {
'rp-entry-indicator-focused': focused,
})}
onClick={onIndicatorClick}
<EntryIndicator
ref={indicatorRef}
focused={focused}
onMouseEnter={handleIndicatorMouseEnter}
onClick={handleIndicatorClick}
>
<Icon type="comment" />
</div>
</EntryIndicator>
<div
className={classnames('rp-entry', 'rp-entry-comment', {
'rp-entry-focused': focused,

View file

@ -1,17 +1,37 @@
import classnames from 'classnames'
import { createPortal } from 'react-dom'
import { Coordinates } from '../hooks/use-indicator-hover'
function EntryContainer({
id,
className,
hoverCoords,
...rest
}: React.ComponentProps<'div'>) {
return (
}: React.ComponentProps<'div'> & {
hoverCoords?: Coordinates | null
}) {
const container = (
<div
className={classnames('rp-entry-wrapper', className)}
data-entry-id={id}
{...rest}
/>
)
if (hoverCoords) {
// Render in a floating positioned container
return createPortal(
<div
className="rp-floating-entry"
style={{ left: hoverCoords.x + 'px', top: hoverCoords.y + 'px' }}
>
{container}
</div>,
document.body
)
} else {
return container
}
}
export default EntryContainer

View file

@ -0,0 +1,30 @@
import classnames from 'classnames'
import { forwardRef } from 'react'
type EntryIndicatorProps = {
focused: boolean
onMouseEnter: () => void
onClick: () => void
children: React.ReactNode
}
const EntryIndicator = forwardRef<HTMLDivElement, EntryIndicatorProps>(
({ focused, onMouseEnter, onClick, children }, ref) => {
return (
/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */
<div
ref={ref}
className={classnames('rp-entry-indicator', {
'rp-entry-indicator-focused': focused,
})}
onClick={onClick}
onMouseEnter={onMouseEnter}
>
{children}
</div>
)
}
)
EntryIndicator.displayName = 'EntryIndicator'
export default EntryIndicator

View file

@ -0,0 +1,60 @@
import { useRef, useState } from 'react'
import { flushSync } from 'react-dom'
import { useReviewPanelUpdaterFnsContext } from '../../../context/review-panel/review-panel-context'
import { useLayoutContext } from '../../../../../shared/context/layout-context'
import useScopeValue from '../../../../../shared/hooks/use-scope-value'
export type Coordinates = {
x: number
y: number
}
function useIndicatorHover() {
const [hoverCoords, setHoverCoords] = useState<Coordinates | null>(null)
const [layoutToLeft] = useScopeValue<boolean>('reviewPanel.layoutToLeft')
const { toggleReviewPanel } = useReviewPanelUpdaterFnsContext()
const { reviewPanelOpen } = useLayoutContext()
const { setLayoutSuspended, handleLayoutChange } =
useReviewPanelUpdaterFnsContext()
const indicatorRef = useRef<HTMLDivElement | null>(null)
const handleEntryMouseLeave = () => {
if (!reviewPanelOpen && !layoutToLeft) {
// Use flushSync to ensure that React renders immediately. This is
// necessary to ensure that the subsequent layout update acts on the
// updated DOM.
flushSync(() => {
setHoverCoords(null)
setLayoutSuspended(false)
})
handleLayoutChange(true)
}
}
const handleIndicatorMouseEnter = () => {
if (!layoutToLeft) {
const rect = indicatorRef.current?.getBoundingClientRect()
setHoverCoords({
x: rect?.left || 0,
y: rect?.top || 0,
})
setLayoutSuspended(true)
}
}
const handleIndicatorClick = () => {
setHoverCoords(null)
setLayoutSuspended(false)
toggleReviewPanel()
}
return {
hoverCoords,
indicatorRef,
handleEntryMouseLeave,
handleIndicatorMouseEnter,
handleIndicatorClick,
}
}
export default useIndicatorHover

View file

@ -24,32 +24,18 @@ type EntryView = {
callout: HTMLElement
layout: HTMLElement
height: number
previousEntryTop: number | null
previousCalloutTop: number | null
entry: ReviewPanelEntry
visible: boolean
positions?: Positions
previousPositions?: Positions
}
type EntryPositions = Pick<EntryView, 'entryId' | 'positions'>
type LayoutInfo = {
focusedEntryIndex: number
overflowTop: number
height: number
positions: EntryPositions[]
}
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,
@ -78,8 +64,8 @@ const calculateEntryViewPositions = (
calculateTop: (originalTop: number, height: number) => number
) => {
for (const entryView of entryViews) {
const entryVisible = applyEntryVisibility(entryView)
if (entryVisible) {
entryView.wrapper.classList.toggle('rp-entry-hidden', !entryView.visible)
if (entryView.visible) {
const entryTop = calculateTop(
entryView.entry.screenPos.y,
entryView.height
@ -104,21 +90,24 @@ type PositionedEntriesProps = {
children: React.ReactNode
}
const initialLayoutInfo = {
focusedEntryIndex: 0,
overflowTop: 0,
height: 0,
positions: [] as EntryPositions[],
}
function PositionedEntries({
entries,
contentHeight,
children,
}: PositionedEntriesProps) {
const { navHeight, toolbarHeight, lineHeight } = useReviewPanelValueContext()
const { navHeight, toolbarHeight, lineHeight, layoutSuspended } =
useReviewPanelValueContext()
const containerRef = useRef<HTMLDivElement | null>(null)
const { reviewPanelOpen } = useLayoutContext()
const animationTimerRef = useRef<number | null>(null)
const previousLayoutInfoRef = useRef<LayoutInfo>({
focusedEntryIndex: 0,
overflowTop: 0,
height: 0,
positions: [],
})
const previousLayoutInfoRef = useRef(initialLayoutInfo)
const layout = () => {
const container = containerRef.current
@ -158,8 +147,10 @@ function PositionedEntries({
const layoutElement = reviewPanelOpen ? box : indicator
if (box && callout && layoutElement) {
const previousEntryTopData = box.dataset.previousEntryTop
const previousCalloutTopData = callout.dataset.previousEntryTop
const previousPositions = previousLayoutInfoRef.current?.positions.find(
pos => pos.entryId === entryId
)?.positions
const visible = Boolean(entry.screenPos)
entryViews.push({
entryId,
wrapper,
@ -167,17 +158,10 @@ function PositionedEntries({
box,
callout,
layout: layoutElement,
visible: !!entry.screenPos,
visible,
height: 0,
previousEntryTop:
previousEntryTopData !== undefined
? parseInt(previousEntryTopData)
: null,
previousCalloutTop:
previousCalloutTopData !== undefined
? parseInt(previousCalloutTopData)
: null,
entry,
previousPositions,
})
} else {
debugConsole.log(
@ -305,10 +289,9 @@ function PositionedEntries({
// 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
const entryTopInitial = entryView.previousPositions
? entryView.previousPositions.entryTop
: entryTop
css(entryView.box, {
top: entryTopInitial + overflowTop + 'px',
@ -322,25 +305,20 @@ function PositionedEntries({
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
const calloutTopInitial = entryView.previousPositions
? entryView.previousPositions.callout.top
: callout.top
css(calloutEl, {
top: calloutTopInitial + overflowTop + 'px',
height: callout.height + 'px',
})
entryView.box.dataset.previousCalloutTop = calloutTopInitial + ''
}
}
}
@ -355,19 +333,17 @@ function PositionedEntries({
const { entryTop, callout } = positions
// Position the main wrapper, if it's moved
if (entryView.previousEntryTop !== entryTop) {
if (entryView.previousPositions?.entryTop !== 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) {
if (entryView.previousPositions?.callout.top !== callout.top) {
calloutEl.style.top = callout.top + overflowTop + 'px'
entryView.callout.dataset.previousCalloutTop = callout.top + ''
}
}
}
@ -430,8 +406,14 @@ function PositionedEntries({
}
}
useEventListener('review-panel:layout', () => {
layout()
useEventListener('review-panel:layout', (event: CustomEvent) => {
if (!layoutSuspended) {
// Clear previous positions if forcing a layout
if (event.detail.force) {
previousLayoutInfoRef.current = initialLayoutInfo
}
layout()
}
})
// Layout on first render. This is necessary to ensure layout happens when
@ -440,6 +422,11 @@ function PositionedEntries({
dispatchReviewPanelLayout()
}, [])
// Ensure layout is performed after opening or closing the review panel
useEffect(() => {
previousLayoutInfoRef.current = initialLayoutInfo
}, [reviewPanelOpen])
return (
<div
ref={containerRef}

View file

@ -15,7 +15,4 @@ export interface BaseChangeEntryProps
timestamp: ReviewPanelChangeEntry['metadata']['ts']
contentLimit?: number
entryIds: ReviewPanelChangeEntry['entry_ids']
onMouseEnter?: () => void
onMouseLeave?: () => void
onIndicatorClick?: () => void
}

View file

@ -137,6 +137,7 @@ function useAngularReviewPanelState(): ReviewPanelState {
const [isAddingComment, setIsAddingComment] = useState(false)
const [navHeight, setNavHeight] = useState(0)
const [toolbarHeight, setToolbarHeight] = useState(0)
const [layoutSuspended, setLayoutSuspended] = useState(false)
const values = useMemo<ReviewPanelState['values']>(
() => ({
@ -164,6 +165,7 @@ function useAngularReviewPanelState(): ReviewPanelState {
trackChangesOnForGuests,
trackChangesForGuestsAvailable,
formattedProjectMembers,
layoutSuspended,
}),
[
collapsed,
@ -190,6 +192,7 @@ function useAngularReviewPanelState(): ReviewPanelState {
trackChangesOnForGuests,
trackChangesForGuestsAvailable,
formattedProjectMembers,
layoutSuspended,
]
)
@ -220,6 +223,7 @@ function useAngularReviewPanelState(): ReviewPanelState {
setIsAddingComment,
setNavHeight,
setToolbarHeight,
setLayoutSuspended,
}),
[
handleSetSubview,
@ -246,6 +250,7 @@ function useAngularReviewPanelState(): ReviewPanelState {
setIsAddingComment,
setNavHeight,
setToolbarHeight,
setLayoutSuspended,
]
)

View file

@ -45,10 +45,11 @@ export interface ReviewPanelState {
name: string
}
>
layoutSuspended: boolean
}
updaterFns: {
handleSetSubview: (subView: SubView) => void
handleLayoutChange: () => void
handleLayoutChange: (force?: boolean) => void
gotoEntry: (docId: DocId, entryOffset: number) => void
resolveComment: (docId: DocId, entryId: ThreadId) => void
deleteComment: (threadId: ThreadId, commentId: CommentId) => void
@ -82,6 +83,9 @@ export interface ReviewPanelState {
setToolbarHeight: React.Dispatch<
React.SetStateAction<Value<'toolbarHeight'>>
>
setLayoutSuspended: React.Dispatch<
React.SetStateAction<Value<'layoutSuspended'>>
>
}
}
/* eslint-enable no-use-before-define */

View file

@ -31,8 +31,10 @@ export const dispatchEditorEvent = (type: string, payload?: unknown) => {
}, 0)
}
export const dispatchReviewPanelLayout = () => {
window.dispatchEvent(new CustomEvent('review-panel:layout'))
export const dispatchReviewPanelLayout = (force = false) => {
window.dispatchEvent(
new CustomEvent('review-panel:layout', { detail: { force } })
)
}
const scheduleDispatchReviewPanelLayout = debounce(

View file

@ -232,10 +232,17 @@
.rp-entry-indicator {
display: none;
z-index: 2; // above .review-panel-toggler
.rp-size-mini & {
display: block;
}
.rp-floating-entry & {
display: block;
position: static;
width: @review-off-width - 4px;
}
.rp-size-mini &-add-comment {
display: none;
}
@ -280,8 +287,8 @@
width: @review-panel-width;
}
.rp-state-current-file-mini & {
display: none;
.rp-state-current-file-mini &,
.rp-floating-entry & {
left: @review-off-width + @rp-entry-arrow-width;
box-shadow: 0 0 10px 5px rgba(0, 0, 0, 0.2);
z-index: 1;
@ -309,6 +316,17 @@
}
}
.rp-state-current-file-mini & {
display: none;
}
.rp-floating-entry & {
position: absolute;
width: @review-panel-width;
left: @review-off-width + @rp-entry-arrow-width;
top: 0;
}
.rp-state-current-file-mini.rp-layout-left & {
left: auto;
right: @review-off-width + @rp-entry-arrow-width;
@ -1258,7 +1276,6 @@ button when (@is-overleaf-light = true) {
.rp-entry-list-react {
position: relative;
overflow-x: hidden;
}
.rp-state-current-file & {
@ -1299,7 +1316,16 @@ button when (@is-overleaf-light = true) {
height: 0;
transition: height 150ms;
}
}
.rp-floating-entry {
position: absolute;
font-size: @rp-base-font-size;
color: @rp-type-blue;
}
.ol-cm-review-panel,
.rp-floating-entry {
.rp-entry-metadata {
button {
padding: 0;