From c2a3b67cccb964a28dc7c6fdcfac387a44cf3066 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Sat, 8 Feb 2020 16:20:14 -0800 Subject: [PATCH] Fix scrollbar (#735) The computation was not correct. I'm not really sure how it used to work but it was not taking into account the dimensions of the scene so it was wrong. The new algorithm is computing the scrollbar such that it's the position of the viewport in relationship to the bounding box of the viewport and all the elements. Fixes #680 --- src/index.tsx | 40 +++++--------- src/scene/scrollbars.ts | 114 +++++++++++++++++++++------------------- 2 files changed, 73 insertions(+), 81 deletions(-) diff --git a/src/index.tsx b/src/index.tsx index 065e3d17..96e46046 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -102,9 +102,6 @@ import { copyToAppClipboard, getClipboardContent } from "./clipboard"; let { elements } = createScene(); const { history } = createHistory(); -const CANVAS_WINDOW_OFFSET_LEFT = 0; -const CANVAS_WINDOW_OFFSET_TOP = 0; - function resetCursor() { document.documentElement.style.cursor = ""; } @@ -142,8 +139,8 @@ export function viewportCoordsToSceneCoords( { clientX, clientY }: { clientX: number; clientY: number }, { scrollX, scrollY }: { scrollX: number; scrollY: number }, ) { - const x = clientX - CANVAS_WINDOW_OFFSET_LEFT - scrollX; - const y = clientY - CANVAS_WINDOW_OFFSET_TOP - scrollY; + const x = clientX - scrollX; + const y = clientY - scrollY; return { x, y }; } @@ -740,8 +737,8 @@ export class App extends React.Component { }; public render() { - const canvasWidth = window.innerWidth - CANVAS_WINDOW_OFFSET_LEFT; - const canvasHeight = window.innerHeight - CANVAS_WINDOW_OFFSET_TOP; + const canvasWidth = window.innerWidth; + const canvasHeight = window.innerHeight; return (
@@ -910,10 +907,10 @@ export class App extends React.Component { isOverVerticalScrollBar, } = isOverScrollBars( elements, - e.clientX - CANVAS_WINDOW_OFFSET_LEFT, - e.clientY - CANVAS_WINDOW_OFFSET_TOP, - canvasWidth, - canvasHeight, + e.clientX / window.devicePixelRatio, + e.clientY / window.devicePixelRatio, + canvasWidth / window.devicePixelRatio, + canvasHeight / window.devicePixelRatio, this.state.scrollX, this.state.scrollY, ); @@ -1096,8 +1093,8 @@ export class App extends React.Component { let lastY = y; if (isOverHorizontalScrollBar || isOverVerticalScrollBar) { - lastX = e.clientX - CANVAS_WINDOW_OFFSET_LEFT; - lastY = e.clientY - CANVAS_WINDOW_OFFSET_TOP; + lastX = e.clientX; + lastY = e.clientY; } let resizeArrowFn: @@ -1175,7 +1172,7 @@ export class App extends React.Component { } if (isOverHorizontalScrollBar) { - const x = e.clientX - CANVAS_WINDOW_OFFSET_LEFT; + const x = e.clientX; const dx = x - lastX; this.setState({ scrollX: this.state.scrollX - dx }); lastX = x; @@ -1183,7 +1180,7 @@ export class App extends React.Component { } if (isOverVerticalScrollBar) { - const y = e.clientY - CANVAS_WINDOW_OFFSET_TOP; + const y = e.clientY; const dy = y - lastY; this.setState({ scrollY: this.state.scrollY - dy }); lastY = y; @@ -1690,12 +1687,10 @@ export class App extends React.Component { textX = this.state.scrollX + elementAtPosition.x + - CANVAS_WINDOW_OFFSET_LEFT + elementAtPosition.width / 2; textY = this.state.scrollY + elementAtPosition.y + - CANVAS_WINDOW_OFFSET_TOP + elementAtPosition.height / 2; // x and y will change after calling newTextElement function @@ -1833,13 +1828,8 @@ export class App extends React.Component { const elementsCenterX = distance(minX, maxX) / 2; const elementsCenterY = distance(minY, maxY) / 2; - const dx = - cursorX - - this.state.scrollX - - CANVAS_WINDOW_OFFSET_LEFT - - elementsCenterX; - const dy = - cursorY - this.state.scrollY - CANVAS_WINDOW_OFFSET_TOP - elementsCenterY; + const dx = cursorX - this.state.scrollX - elementsCenterX; + const dy = cursorY - this.state.scrollY - elementsCenterY; elements = [ ...elements, @@ -1871,12 +1861,10 @@ export class App extends React.Component { const wysiwygX = this.state.scrollX + elementClickedInside.x + - CANVAS_WINDOW_OFFSET_LEFT + elementClickedInside.width / 2; const wysiwygY = this.state.scrollY + elementClickedInside.y + - CANVAS_WINDOW_OFFSET_TOP + elementClickedInside.height / 2; return { wysiwygX, wysiwygY, elementCenterX, elementCenterY }; } diff --git a/src/scene/scrollbars.ts b/src/scene/scrollbars.ts index fdcf609a..d234162e 100644 --- a/src/scene/scrollbars.ts +++ b/src/scene/scrollbars.ts @@ -1,67 +1,70 @@ import { ExcalidrawElement } from "../element/types"; import { getCommonBounds } from "../element"; -const SCROLLBAR_MIN_SIZE = 15; const SCROLLBAR_MARGIN = 4; export const SCROLLBAR_WIDTH = 6; export const SCROLLBAR_COLOR = "rgba(0,0,0,0.3)"; export function getScrollBars( elements: readonly ExcalidrawElement[], - canvasWidth: number, - canvasHeight: number, + viewportWidth: number, + viewportHeight: number, scrollX: number, scrollY: number, ) { - let [minX, minY, maxX, maxY] = getCommonBounds(elements); + // This is the bounding box of all the elements + const [ + elementsMinX, + elementsMinY, + elementsMaxX, + elementsMaxY, + ] = getCommonBounds(elements); - minX += scrollX; - maxX += scrollX; - minY += scrollY; - maxY += scrollY; + // The viewport is the rectangle currently visible for the user + const viewportMinX = -scrollX; + const viewportMinY = -scrollY; + const viewportMaxX = -scrollX + viewportWidth; + const viewportMaxY = -scrollY + viewportHeight; - const leftOverflow = Math.max(-minX, 0); - const rightOverflow = Math.max(-(canvasWidth - maxX), 0); - const topOverflow = Math.max(-minY, 0); - const bottomOverflow = Math.max(-(canvasHeight - maxY), 0); + // The scene is the bounding box of both the elements and viewport + const sceneMinX = Math.min(elementsMinX, viewportMinX); + const sceneMinY = Math.min(elementsMinY, viewportMinY); + const sceneMaxX = Math.max(elementsMaxX, viewportMaxX); + const sceneMaxY = Math.max(elementsMaxY, viewportMaxY); - // horizontal scrollbar - let horizontalScrollBar = null; - if (leftOverflow || rightOverflow) { - horizontalScrollBar = { - x: Math.min( - leftOverflow + SCROLLBAR_MARGIN, - canvasWidth - SCROLLBAR_MIN_SIZE - SCROLLBAR_MARGIN, - ), - y: canvasHeight - SCROLLBAR_WIDTH - SCROLLBAR_MARGIN, - width: Math.max( - canvasWidth - rightOverflow - leftOverflow - SCROLLBAR_MARGIN * 2, - SCROLLBAR_MIN_SIZE, - ), - height: SCROLLBAR_WIDTH, - }; - } - - // vertical scrollbar - let verticalScrollBar = null; - if (topOverflow || bottomOverflow) { - verticalScrollBar = { - x: canvasWidth - SCROLLBAR_WIDTH - SCROLLBAR_MARGIN, - y: Math.min( - topOverflow + SCROLLBAR_MARGIN, - canvasHeight - SCROLLBAR_MIN_SIZE - SCROLLBAR_MARGIN, - ), - width: SCROLLBAR_WIDTH, - height: Math.max( - canvasHeight - bottomOverflow - topOverflow - SCROLLBAR_WIDTH * 2, - SCROLLBAR_MIN_SIZE, - ), - }; - } + // The scrollbar represents where the viewport is in relationship to the scene return { - horizontal: horizontalScrollBar, - vertical: verticalScrollBar, + horizontal: + viewportMinX === sceneMinX && viewportMaxX === sceneMaxX + ? null + : { + x: + ((viewportMinX - sceneMinX) / (sceneMaxX - sceneMinX)) * + viewportWidth + + SCROLLBAR_MARGIN, + y: viewportHeight - SCROLLBAR_WIDTH - SCROLLBAR_MARGIN, + width: + ((viewportMaxX - viewportMinX) / (sceneMaxX - sceneMinX)) * + viewportWidth - + SCROLLBAR_MARGIN * 2, + height: SCROLLBAR_WIDTH, + }, + vertical: + viewportMinY === sceneMinY && viewportMaxY === sceneMaxY + ? null + : { + x: viewportWidth - SCROLLBAR_WIDTH - SCROLLBAR_MARGIN, + y: + ((viewportMinY - sceneMinY) / (sceneMaxY - sceneMinY)) * + viewportHeight + + SCROLLBAR_MARGIN, + width: SCROLLBAR_WIDTH, + height: + ((viewportMaxY - viewportMinY) / (sceneMaxY - sceneMinY)) * + viewportHeight - + SCROLLBAR_MARGIN * 2, + }, }; } @@ -69,15 +72,15 @@ export function isOverScrollBars( elements: readonly ExcalidrawElement[], x: number, y: number, - canvasWidth: number, - canvasHeight: number, + viewportWidth: number, + viewportHeight: number, scrollX: number, scrollY: number, ) { const scrollBars = getScrollBars( elements, - canvasWidth, - canvasHeight, + viewportWidth, + viewportHeight, scrollX, scrollY, ); @@ -85,14 +88,15 @@ export function isOverScrollBars( const [isOverHorizontalScrollBar, isOverVerticalScrollBar] = [ scrollBars.horizontal, scrollBars.vertical, - ].map( - scrollBar => + ].map(scrollBar => { + return ( scrollBar && scrollBar.x <= x && x <= scrollBar.x + scrollBar.width && scrollBar.y <= y && - y <= scrollBar.y + scrollBar.height, - ); + y <= scrollBar.y + scrollBar.height + ); + }); return { isOverHorizontalScrollBar,