From 206eb343b4aff4928623993a7935c587e7c1258b Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Wed, 20 Nov 2024 18:30:00 -0500 Subject: [PATCH 1/3] Clamp drag rectangle to chart area I believe this is a better UI, and it also fixes some issues with incorrect zoom ranges when dragging the rectangle outside of the chart area. To help implement this, I copied the clamp helper function from chartjs-plugin-annotation. Fixes #895 --- src/handlers.js | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/handlers.js b/src/handlers.js index df3d4ce1..57c0349f 100644 --- a/src/handlers.js +++ b/src/handlers.js @@ -3,6 +3,14 @@ import {zoom, zoomRect} from './core'; import {callback as call, getRelativePosition, _isPointInArea} from 'chart.js/helpers'; import {getState} from './state'; +/** + * @param {number} x + * @param {number} from + * @param {number} to + * @returns {number} + */ +const clamp = (x, from, to) => Math.min(to, Math.max(from, x)); + function removeHandler(chart, type) { const {handlers} = getState(chart); const handler = handlers[type]; @@ -111,12 +119,12 @@ function applyAspectRatio(endPoint, beginPoint, aspectRatio) { endPoint.y = beginPoint.y + height; } -function applyMinMaxProps(rect, beginPoint, endPoint, {min, max, prop}) { - rect[min] = Math.max(0, Math.min(beginPoint[prop], endPoint[prop])); - rect[max] = Math.max(beginPoint[prop], endPoint[prop]); +function applyMinMaxProps(rect, chartArea, beginPoint, endPoint, {min, max, prop}) { + rect[min] = clamp(Math.min(beginPoint[prop], endPoint[prop]), chartArea[min], chartArea[max]); + rect[max] = clamp(Math.max(beginPoint[prop], endPoint[prop]), chartArea[min], chartArea[max]); } -function getReplativePoints(chart, points, maintainAspectRatio) { +function getRelativePoints(chart, points, maintainAspectRatio) { const beginPoint = getPointPosition(points.dragStart, chart); const endPoint = getPointPosition(points.dragEnd, chart); @@ -134,14 +142,14 @@ export function computeDragRect(chart, mode, points, maintainAspectRatio) { const {top, left, right, bottom, width: chartWidth, height: chartHeight} = chart.chartArea; const rect = {top, left, right, bottom}; - const {beginPoint, endPoint} = getReplativePoints(chart, points, maintainAspectRatio && xEnabled && yEnabled); + const {beginPoint, endPoint} = getRelativePoints(chart, points, maintainAspectRatio && xEnabled && yEnabled); if (xEnabled) { - applyMinMaxProps(rect, beginPoint, endPoint, {min: 'left', max: 'right', prop: 'x'}); + applyMinMaxProps(rect, chart.chartArea, beginPoint, endPoint, {min: 'left', max: 'right', prop: 'x'}); } if (yEnabled) { - applyMinMaxProps(rect, beginPoint, endPoint, {min: 'top', max: 'bottom', prop: 'y'}); + applyMinMaxProps(rect, chart.chartArea, beginPoint, endPoint, {min: 'top', max: 'bottom', prop: 'y'}); } const width = rect.right - rect.left; From 0dadb11862c0be9314ee682e1dac9b692494637f Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Wed, 20 Nov 2024 18:52:01 -0500 Subject: [PATCH 2/3] Combine beginPoint and endPoint to fix ESLint max arguments --- src/handlers.js | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/handlers.js b/src/handlers.js index 57c0349f..25ea4415 100644 --- a/src/handlers.js +++ b/src/handlers.js @@ -104,9 +104,9 @@ export function mouseDown(chart, event) { addHandler(chart, window.document, 'keydown', keyDown); } -function applyAspectRatio(endPoint, beginPoint, aspectRatio) { - let width = endPoint.x - beginPoint.x; - let height = endPoint.y - beginPoint.y; +function applyAspectRatio({begin, end}, aspectRatio) { + let width = end.x - begin.x; + let height = end.y - begin.y; const ratio = Math.abs(width / height); if (ratio > aspectRatio) { @@ -115,41 +115,43 @@ function applyAspectRatio(endPoint, beginPoint, aspectRatio) { height = Math.sign(height) * Math.abs(width / aspectRatio); } - endPoint.x = beginPoint.x + width; - endPoint.y = beginPoint.y + height; + end.x = begin.x + width; + end.y = begin.y + height; } -function applyMinMaxProps(rect, chartArea, beginPoint, endPoint, {min, max, prop}) { - rect[min] = clamp(Math.min(beginPoint[prop], endPoint[prop]), chartArea[min], chartArea[max]); - rect[max] = clamp(Math.max(beginPoint[prop], endPoint[prop]), chartArea[min], chartArea[max]); +function applyMinMaxProps(rect, chartArea, points, {min, max, prop}) { + rect[min] = clamp(Math.min(points.begin[prop], points.end[prop]), chartArea[min], chartArea[max]); + rect[max] = clamp(Math.max(points.begin[prop], points.end[prop]), chartArea[min], chartArea[max]); } -function getRelativePoints(chart, points, maintainAspectRatio) { - const beginPoint = getPointPosition(points.dragStart, chart); - const endPoint = getPointPosition(points.dragEnd, chart); +function getRelativePoints(chart, pointEvents, maintainAspectRatio) { + const points = { + begin: getPointPosition(pointEvents.dragStart, chart), + end: getPointPosition(pointEvents.dragEnd, chart), + }; if (maintainAspectRatio) { const aspectRatio = chart.chartArea.width / chart.chartArea.height; - applyAspectRatio(endPoint, beginPoint, aspectRatio); + applyAspectRatio(points, aspectRatio); } - return {beginPoint, endPoint}; + return points; } -export function computeDragRect(chart, mode, points, maintainAspectRatio) { +export function computeDragRect(chart, mode, pointEvents, maintainAspectRatio) { const xEnabled = directionEnabled(mode, 'x', chart); const yEnabled = directionEnabled(mode, 'y', chart); const {top, left, right, bottom, width: chartWidth, height: chartHeight} = chart.chartArea; const rect = {top, left, right, bottom}; - const {beginPoint, endPoint} = getRelativePoints(chart, points, maintainAspectRatio && xEnabled && yEnabled); + const points = getRelativePoints(chart, pointEvents, maintainAspectRatio && xEnabled && yEnabled); if (xEnabled) { - applyMinMaxProps(rect, chart.chartArea, beginPoint, endPoint, {min: 'left', max: 'right', prop: 'x'}); + applyMinMaxProps(rect, chart.chartArea, points, {min: 'left', max: 'right', prop: 'x'}); } if (yEnabled) { - applyMinMaxProps(rect, chart.chartArea, beginPoint, endPoint, {min: 'top', max: 'bottom', prop: 'y'}); + applyMinMaxProps(rect, chart.chartArea, points, {min: 'top', max: 'bottom', prop: 'y'}); } const width = rect.right - rect.left; From 36fe2c23d979619d6b6167a8943445a0d584e056 Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Thu, 21 Nov 2024 13:02:42 -0500 Subject: [PATCH 3/3] Add a test --- test/specs/zoom.drag.spec.js | 56 ++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/test/specs/zoom.drag.spec.js b/test/specs/zoom.drag.spec.js index d9ff2602..712f2b6d 100644 --- a/test/specs/zoom.drag.spec.js +++ b/test/specs/zoom.drag.spec.js @@ -396,6 +396,62 @@ describe('zoom with drag', function() { } }); + describe('coordinate handling', function() { + let chart, scaleX, scaleY; + + it('handles dragging to the right edge of the chart', function() { + chart = window.acquireChart({ + type: 'line', + data, + options: { + scales: { + xScale0: { + id: 'xScale0', + type: 'linear', + min: 0, + max: 4 + }, + yScale0: { + id: 'yScale0', + type: 'linear', + min: 0, + max: 4, + } + }, + plugins: { + zoom: { + zoom: { + drag: { + enabled: true + }, + mode: 'xy' + } + } + } + } + }, { + wrapper: {style: 'position: absolute; left: 50px; top: 50px;'} + }); + + scaleX = chart.scales.xScale0; + scaleY = chart.scales.yScale0; + + jasmine.triggerMouseEvent(chart, 'mousedown', { + x: scaleX.getPixelForValue(1.5), + y: scaleY.getPixelForValue(1.2) + }); + jasmine.triggerMouseEvent(chart, 'mouseup', { + x: scaleX.getPixelForValue(4) + 5, + y: scaleY.getPixelForValue(1.7) + }); + + expect(scaleX.options.min).toBeCloseTo(1.5); + expect(scaleX.options.max).toBeCloseTo(4); + expect(scaleY.options.min).toBeCloseTo(1.2); + expect(scaleY.options.max).toBeCloseTo(1.7); + }); + }); + describe('events', function() { it('should call onZoomStart, onZoom and onZoomComplete', function(done) { const startSpy = jasmine.createSpy('start');