From 5a20c6fef277c413c99a9a252e632f3b81947965 Mon Sep 17 00:00:00 2001 From: Federico Ruggi <1081051+ruggi@users.noreply.github.com> Date: Tue, 23 Jan 2024 14:41:13 +0100 Subject: [PATCH] Fix comment indicators in follow mode (#4781) * disable pan on follow mode * canPan * use current style * smooth out cursors * use canvas wrapper * fix click * update expect --------- Co-authored-by: Federico Ruggi --- .../canvas/controls/comment-indicator.tsx | 191 ++++++++---------- editor/src/templates/editor-canvas.tsx | 4 +- .../src/comments/commenting.spec.tsx | 4 +- 3 files changed, 86 insertions(+), 113 deletions(-) diff --git a/editor/src/components/canvas/controls/comment-indicator.tsx b/editor/src/components/canvas/controls/comment-indicator.tsx index 11f4842eea9b..b27509e3b60a 100644 --- a/editor/src/components/canvas/controls/comment-indicator.tsx +++ b/editor/src/components/canvas/controls/comment-indicator.tsx @@ -1,10 +1,7 @@ -/** @jsxRuntime classic */ -/** @jsx jsx */ -import type { Interpolation } from '@emotion/react' -import { jsx } from '@emotion/react' import type { ThreadData } from '@liveblocks/client' import { Comment } from '@liveblocks/react-comments' import { AnimatePresence, motion, useAnimate } from 'framer-motion' +import type { CSSProperties } from 'react' import React from 'react' import type { ThreadMetadata } from '../../../../liveblocks.config' import { useEditThreadMetadata, useStorage } from '../../../../liveblocks.config' @@ -20,7 +17,6 @@ import type { CanvasRectangle, LocalPoint, MaybeInfinityCanvasRectangle, - WindowPoint, } from '../../../core/shared/math-utils' import { canvasPoint, @@ -44,7 +40,6 @@ import { optionalMap } from '../../../core/shared/optional-utils' import type { CommentWrapperProps } from '../../../utils/multiplayer-wrapper' import { MultiplayerWrapper, baseMultiplayerAvatarStyle } from '../../../utils/multiplayer-wrapper' import { when } from '../../../utils/react-conditionals' -import type { Theme } from '../../../uuiui' import { UtopiaStyles, colorTheme } from '../../../uuiui' import { setProp_UNSAFE, @@ -56,7 +51,7 @@ import { useDispatch } from '../../editor/store/dispatch-context' import { RightMenuTab } from '../../editor/store/editor-state' import { Substores, useEditorState, useRefEditorState } from '../../editor/store/store-hook' import { MultiplayerAvatar } from '../../user-bar' -import { canvasPointToWindowPoint, windowToCanvasCoordinates } from '../dom-lookup' +import { windowToCanvasCoordinates } from '../dom-lookup' import { RemixNavigationAtom, useRemixNavigationContext, @@ -68,6 +63,7 @@ import * as EP from '../../../core/shared/element-path' import { useRefAtom } from '../../editor/hook-utils' import { emptyComments, jsExpressionValue } from '../../../core/shared/element-template' import * as PP from '../../../core/shared/property-path' +import { CanvasOffsetWrapper } from './canvas-offset-wrapper' export const CommentIndicators = React.memo(() => { const projectId = useEditorState( @@ -95,7 +91,7 @@ export const CommentIndicators = React.memo(() => { CommentIndicators.displayName = 'CommentIndicators' interface TemporaryCommentIndicatorProps { - position: WindowPoint + position: CanvasPoint bgColor: string fgColor: string avatarUrl: string | null @@ -114,25 +110,9 @@ function useCommentBeingComposed(): TemporaryCommentIndicatorProps | null { 'CommentIndicatorsInner commentBeingComposed', ) - const canvasScale = useEditorState( - Substores.canvasOffset, - (store) => store.editor.canvas.scale, - 'MultiplayerPresence canvasScale', - ) - - const canvasOffset = useEditorState( - Substores.canvasOffset, - (store) => store.editor.canvas.roundedCanvasOffset, - 'MultiplayerPresence canvasOffset', - ) - const { location } = useCanvasCommentThreadAndLocation( commentBeingComposed ?? { type: 'existing', threadId: 'dummy-thread-id' }, // this is as a placeholder for nulls ) - const position = React.useMemo( - () => (location == null ? null : canvasPointToWindowPoint(location, canvasScale, canvasOffset)), - [location, canvasScale, canvasOffset], - ) const collabs = useStorage((storage) => storage.collaborators) @@ -155,12 +135,12 @@ function useCommentBeingComposed(): TemporaryCommentIndicatorProps | null { } }, [collabs, myUserId]) - if (position == null) { + if (location == null) { return null } return { - position: position, + position: location, bgColor: collaboratorInfo.color.background, fgColor: collaboratorInfo.color.foreground, avatarUrl: collaboratorInfo.avatar, @@ -173,7 +153,7 @@ const CommentIndicatorsInner = React.memo(() => { const temporaryIndicatorData = useCommentBeingComposed() return ( - + {threads.map((thread) => ( ))} @@ -189,13 +169,13 @@ const CommentIndicatorsInner = React.memo(() => { isRead={false} /> ) : null} - + ) }) CommentIndicatorsInner.displayName = 'CommentIndicatorInner' interface CommentIndicatorUIProps { - position: WindowPoint + position: CanvasPoint resolved: boolean bgColor: string fgColor: string @@ -205,8 +185,8 @@ interface CommentIndicatorUIProps { isRead: boolean } -function getIndicatorStyle( - position: WindowPoint, +function useIndicatorStyle( + position: CanvasPoint, params: { isRead: boolean resolved: boolean @@ -214,19 +194,23 @@ function getIndicatorStyle( expanded: boolean dragging: boolean }, -) { +): CSSProperties { const { isRead, resolved, isActive, expanded, dragging } = params - const canvasHeight = getCanvasHeight() - const positionAdjust = 3 // px + const canvasScale = useEditorState( + Substores.canvasOffset, + (store) => store.editor.canvas.scale, + 'useIndicatorStyle canvasScale', + ) - const base: Interpolation = { + const style: CSSProperties = { pointerEvents: dragging ? 'none' : undefined, cursor: 'auto', padding: 2, - position: 'fixed', - bottom: canvasHeight - position.y - positionAdjust, + position: 'absolute', left: position.x, + top: position.y, + // transform: 'translateY(-100%)', background: isRead ? colorTheme.bg1.value : colorTheme.primary.value, borderRadius: '24px 24px 24px 0px', display: 'flex', @@ -236,36 +220,31 @@ function getIndicatorStyle( border: '.4px solid #a3a3a340', opacity: resolved ? 0.6 : undefined, zIndex: expanded ? 1 : 'auto', + transform: `translateY(-100%) scale(${1 / canvasScale})`, + transformOrigin: 'bottom left', } - const whenActive: Interpolation = { - border: `1px solid ${colorTheme.primary.value}`, - } - - const whenInactive: Interpolation = { - ':hover': {}, + if (isActive) { + style.border = `1px solid ${colorTheme.primary.value}` } - return { - ...base, - ...(isActive ? whenActive : whenInactive), - } + return style } export const CommentIndicatorUI = React.memo((props) => { const { position, bgColor, fgColor, avatarUrl, avatarInitials, resolved, isActive, isRead } = props + const style = useIndicatorStyle(position, { + isRead: isRead ?? true, + resolved: resolved, + isActive: isActive, + expanded: true, + dragging: false, + }) + return ( -
+
{ const [dragging, setDragging] = React.useState(false) const draggingCallback = React.useCallback((isDragging: boolean) => setDragging(isDragging), []) - const { onMouseDown, didDrag, dragPosition } = useDragging(thread, location, draggingCallback) + const { onMouseDown: onMouseDownDrag, dragPosition } = useDragging( + thread, + location, + draggingCallback, + ) const remixLocationRoute = React.useMemo(() => { return thread.metadata.remixLocationRoute ?? null @@ -315,17 +298,6 @@ const CommentIndicator = React.memo(({ thread }: CommentIndicatorProps) => { return remixLocationRoute != null && remixLocationRoute !== remixState?.location.pathname }, [remixLocationRoute, remixState]) - const canvasScale = useEditorState( - Substores.canvasOffset, - (store) => store.editor.canvas.scale, - 'CommentIndicator canvasScale', - ) - const canvasOffset = useEditorState( - Substores.canvasOffset, - (store) => store.editor.canvas.roundedCanvasOffset, - 'CommentIndicator canvasOffset', - ) - const isActive = useEditorState( Substores.restOfEditor, (store) => @@ -340,11 +312,6 @@ const CommentIndicator = React.memo(({ thread }: CommentIndicatorProps) => { return !isActive && (hovered || dragging) }, [hovered, isActive, dragging]) - const position = React.useMemo( - () => canvasPointToWindowPoint(dragPosition ?? location, canvasScale, canvasOffset), - [location, canvasScale, canvasOffset, dragPosition], - ) - const onMouseOut = React.useCallback(() => { if (dragging) { return @@ -352,47 +319,53 @@ const CommentIndicator = React.memo(({ thread }: CommentIndicatorProps) => { onHoverMouseOut() }, [dragging, onHoverMouseOut]) - const onClick = React.useCallback(() => { - if (didDrag) { - return - } - if (isOnAnotherRoute && remixLocationRoute != null && remixState != null) { - remixState.navigate(remixLocationRoute) - } - cancelHover() - dispatch([ - ...openCommentThreadActions(thread.id, commentScene), - setRightMenuTab(RightMenuTab.Comments), - ]) - }, [ - dispatch, - thread.id, - remixState, - remixLocationRoute, - isOnAnotherRoute, - commentScene, - didDrag, - cancelHover, - ]) + const onMouseDown = React.useCallback( + (e: React.MouseEvent) => { + onMouseDownDrag(e, (dragged) => { + if (dragged) { + return + } + if (isOnAnotherRoute && remixLocationRoute != null && remixState != null) { + remixState.navigate(remixLocationRoute) + } + cancelHover() + dispatch([ + ...openCommentThreadActions(thread.id, commentScene), + setRightMenuTab(RightMenuTab.Comments), + ]) + }) + }, + [ + dispatch, + thread.id, + remixState, + remixLocationRoute, + isOnAnotherRoute, + commentScene, + cancelHover, + onMouseDownDrag, + ], + ) // This is a hack: when the comment is unread, we show a dark background, so we need light foreground colors. // So we trick the Liveblocks Comment component and lie to it that the theme is dark mode. const dataThemeProp = isRead ? {} : { 'data-theme': 'dark' } + const style = useIndicatorStyle(dragPosition ?? location, { + isRead: isRead, + resolved: thread.metadata.resolved, + isActive: isActive, + expanded: preview, + dragging: dragging, + }) + return (
(null) - const [didDrag, setDidDrag] = React.useState(false) const canvasScaleRef = useRefEditorState((store) => store.editor.canvas.scale) const canvasOffsetRef = useRefEditorState((store) => store.editor.canvas.roundedCanvasOffset) @@ -463,9 +435,7 @@ function useDragging( const remixSceneRoutesRef = useRefAtom(RemixNavigationAtom) const onMouseDown = React.useCallback( - (event: React.MouseEvent) => { - setDidDrag(false) - + (event: React.MouseEvent, onHandled: (dragged: boolean) => void) => { const mouseDownPoint = windowPoint({ x: event.clientX, y: event.clientY }) const mouseDownCanvasPoint = windowToCanvasCoordinates( canvasScaleRef.current, @@ -494,7 +464,6 @@ function useDragging( draggingCallback(true) dispatch([switchEditorMode(EditorModes.commentMode(null, 'dragging'))]) - setDidDrag(true) const dragVectorWindow = pointDifference(mouseDownPoint, mouseMovePoint) const dragVectorCanvas = canvasPoint({ x: dragVectorWindow.x / canvasScaleRef.current, @@ -508,10 +477,12 @@ function useDragging( upEvent.stopPropagation() window.removeEventListener('mousemove', onMouseMove) window.removeEventListener('mouseup', onMouseUp) - draggingCallback(false) const mouseUpPoint = windowPoint({ x: upEvent.clientX, y: upEvent.clientY }) + draggingCallback(false) + onHandled(draggedPastThreshold) + if (!draggedPastThreshold) { return } @@ -588,7 +559,7 @@ function useDragging( ], ) - return { onMouseDown, dragPosition, didDrag } + return { onMouseDown, dragPosition } } function useHover() { diff --git a/editor/src/templates/editor-canvas.tsx b/editor/src/templates/editor-canvas.tsx index 18eb910cd2e3..b6fc8fb300d8 100644 --- a/editor/src/templates/editor-canvas.tsx +++ b/editor/src/templates/editor-canvas.tsx @@ -299,7 +299,9 @@ function on( } if (isDragToPan(canvas.editorState.canvas.interactionSession, canvas.keysPressed['space'])) { - if (event.event === 'MOVE' && event.nativeEvent.buttons === 1) { + const canPan = + !isFollowMode(canvas.mode) && event.event === 'MOVE' && event.nativeEvent.buttons === 1 + if (canPan) { return [ CanvasActions.scrollCanvas( canvasPoint({ diff --git a/puppeteer-tests/src/comments/commenting.spec.tsx b/puppeteer-tests/src/comments/commenting.spec.tsx index fe1af6f3dd14..3a4991c17713 100644 --- a/puppeteer-tests/src/comments/commenting.spec.tsx +++ b/puppeteer-tests/src/comments/commenting.spec.tsx @@ -236,8 +236,8 @@ describe('Comments test', () => { expect(originalCommentIndicatorBoundingBox).toEqual({ height: 26, width: 26, - x: 485, - y: 67, + x: 486, + y: 63, }) // drag the comment on the scene