From 031f71838e1f351b0b33e303299be22a9db079da Mon Sep 17 00:00:00 2001 From: Jim O'Donnell Date: Thu, 13 Feb 2025 16:58:22 +0000 Subject: [PATCH] refactor(drawing-tools): drag handle sizing (#6685) * refactor(drawing-tools): drag handle sizing - Remove `defaultProps` from functional components. - Refactor transcription lines to use translation and scale transforms for the line handles. - Style drag handles and point marks with `non-scaling-size`. - Fixes a small issue where drag handles aren't rendered in the correct proportion with respect to marks. * Ignore CI errors when coveralls is down --- .github/workflows/coverage.yml | 1 + .../components/DragHandle/DragHandle.js | 22 ++--- .../drawingTools/components/Mark/Mark.js | 4 +- .../drawingTools/components/Point/Point.js | 33 ++++---- .../TranscriptionLine/TranscriptionLine.js | 4 - .../TranscriptionLine.spec.js | 8 +- .../TranscriptionLineMark.js | 84 +++++++++++-------- 7 files changed, 76 insertions(+), 80 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 2b01653893..6875e2a2e3 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -32,3 +32,4 @@ jobs: uses: coverallsapp/github-action@v2 with: github-token: ${{ secrets.GITHUB_TOKEN }} + fail-on-error: false diff --git a/packages/lib-classifier/src/plugins/drawingTools/components/DragHandle/DragHandle.js b/packages/lib-classifier/src/plugins/drawingTools/components/DragHandle/DragHandle.js index 465acf1c92..92ac25ffba 100644 --- a/packages/lib-classifier/src/plugins/drawingTools/components/DragHandle/DragHandle.js +++ b/packages/lib-classifier/src/plugins/drawingTools/components/DragHandle/DragHandle.js @@ -2,6 +2,7 @@ import { forwardRef } from 'react'; import PropTypes from 'prop-types' import styled from 'styled-components' import draggable from '../draggable' +import { STROKE_WIDTH } from '../Mark/Mark'; const StyledCircle = styled('circle')` stroke-width: 2; @@ -9,19 +10,18 @@ const StyledCircle = styled('circle')` cursor: move; } ` -const RADIUS = screen.width > 900 ? 4 : 10 +const RADIUS = screen.width > 900 ? 3 * STROKE_WIDTH : 5 * STROKE_WIDTH const DragHandle = forwardRef(function DragHandle( { - fill, - radius, - scale, + fill = 'currentColor', + radius = RADIUS, + scale = 1, x, y, dragging = false, invisibleWhenDragging = false, testid, - ...props }, ref ) { @@ -33,9 +33,9 @@ const DragHandle = forwardRef(function DragHandle( } return ( - - - + + + ) }) @@ -48,10 +48,4 @@ DragHandle.propTypes = { y: PropTypes.number.isRequired } -DragHandle.defaultProps = { - fill: 'currentColor', - radius: RADIUS, - scale: 1 -} - export default draggable(DragHandle) diff --git a/packages/lib-classifier/src/plugins/drawingTools/components/Mark/Mark.js b/packages/lib-classifier/src/plugins/drawingTools/components/Mark/Mark.js index 7691532733..b54393fab4 100644 --- a/packages/lib-classifier/src/plugins/drawingTools/components/Mark/Mark.js +++ b/packages/lib-classifier/src/plugins/drawingTools/components/Mark/Mark.js @@ -4,8 +4,8 @@ import { forwardRef, useEffect, useRef } from 'react'; import styled, { css, useTheme } from 'styled-components' import draggable from '../draggable' -const STROKE_WIDTH = 3 -const SELECTED_STROKE_WIDTH = 6 +export const STROKE_WIDTH = 2 +export const SELECTED_STROKE_WIDTH = 4 const StyledGroup = styled.g` stroke-width: ${STROKE_WIDTH}px; diff --git a/packages/lib-classifier/src/plugins/drawingTools/components/Point/Point.js b/packages/lib-classifier/src/plugins/drawingTools/components/Point/Point.js index 575d221b23..acbeab6657 100644 --- a/packages/lib-classifier/src/plugins/drawingTools/components/Point/Point.js +++ b/packages/lib-classifier/src/plugins/drawingTools/components/Point/Point.js @@ -14,22 +14,27 @@ const SELECTED_RADIUS = { const CROSSHAIR_SPACE = 0.2 const CROSSHAIR_WIDTH = 1 -function Point({ active, children, mark, onFinish, scale }) { +const DEFAULT_MARK = { + tool: { + size: 'large' + } +} +function Point({ active = false, children, mark = {DEFAULT_MARK}, onFinish, scale = 1 }) { const { size } = mark.tool const crosshairSpace = CROSSHAIR_SPACE const crosshairWidth = CROSSHAIR_WIDTH - const selectedRadius = SELECTED_RADIUS[size] / scale - const radius = RADIUS[size] / scale + const selectedRadius = SELECTED_RADIUS[size] + const radius = RADIUS[size] return ( - + - + ) } @@ -66,14 +71,4 @@ Point.propTypes = { scale: PropTypes.number } -Point.defaultProps = { - active: false, - mark: { - tool: { - size: 'large' - } - }, - scale: 1 -} - export default observer(Point) diff --git a/packages/lib-classifier/src/plugins/drawingTools/experimental/components/TranscriptionLine/TranscriptionLine.js b/packages/lib-classifier/src/plugins/drawingTools/experimental/components/TranscriptionLine/TranscriptionLine.js index 5a930f949a..1d103cd73f 100644 --- a/packages/lib-classifier/src/plugins/drawingTools/experimental/components/TranscriptionLine/TranscriptionLine.js +++ b/packages/lib-classifier/src/plugins/drawingTools/experimental/components/TranscriptionLine/TranscriptionLine.js @@ -6,7 +6,6 @@ import { Tooltip } from '@zooniverse/react-components' import { useTranslation } from '@translations/i18n' import TooltipIcon from './components/TooltipIcon' -import { HANDLE_RADIUS } from './helpers/constants' import TranscriptionLineMark from './components/TranscriptionLineMark' function storeMapper(stores) { @@ -35,7 +34,6 @@ function TranscriptionLine(props) { lineState = 'active' } const colorToRender = (usesTranscriptionTask) ? transcriptionTaskColors[lineState] : color - const handleRadius = HANDLE_RADIUS / scale function onHandleDrag(coords) { mark.setCoordinates(coords) @@ -66,7 +64,6 @@ function TranscriptionLine(props) { color={colorToRender} handlePointerDown={handlePointerDown} handleFinishClick={handleFinishClick} - handleRadius={handleRadius} mark={mark} onHandleDrag={onHandleDrag} scale={scale} @@ -81,7 +78,6 @@ function TranscriptionLine(props) { color={colorToRender} handlePointerDown={handlePointerDown} handleFinishClick={handleFinishClick} - handleRadius={handleRadius} mark={mark} onHandleDrag={onHandleDrag} scale={scale} diff --git a/packages/lib-classifier/src/plugins/drawingTools/experimental/components/TranscriptionLine/TranscriptionLine.spec.js b/packages/lib-classifier/src/plugins/drawingTools/experimental/components/TranscriptionLine/TranscriptionLine.spec.js index 2f6cc3dbd2..117a5d8bef 100644 --- a/packages/lib-classifier/src/plugins/drawingTools/experimental/components/TranscriptionLine/TranscriptionLine.spec.js +++ b/packages/lib-classifier/src/plugins/drawingTools/experimental/components/TranscriptionLine/TranscriptionLine.spec.js @@ -92,7 +92,7 @@ describe('Components > Drawing marks > Transcription line', function () { } } ) - expect(wrapper.find(DragHandle).find('[x=100]').at(0)).to.have.lengthOf(1) + expect(wrapper.find(DragHandle).find({ x: 100 }).at(0)).to.have.lengthOf(1) }) it('should have a transparent start point', function () { @@ -229,7 +229,7 @@ describe('Components > Drawing marks > Transcription line', function () { ) const dragMove = wrapper.find(DragHandle).find('[x=300]').at(0).prop('dragMove') - wrapper.find('circle[cx=300]').at(0).simulate('pointerdown') + wrapper.find('circle.endPoint').at(0).simulate('pointerdown') expect(mark.x2).to.equal(300) expect(mark.y2).to.equal(400) dragMove({}, { x: 10, y: 20 }) @@ -262,9 +262,9 @@ describe('Components > Drawing marks > Transcription line', function () { ) const dragMove = wrapper.find(DragHandle).find('[x=300]').at(0).prop('dragMove') - wrapper.find('circle[cx=300]').at(0).simulate('pointerdown') + wrapper.find('circle.endPoint').at(0).simulate('pointerdown') expect(mark.finished).to.be.false() - wrapper.find('circle[cx=300]').at(0).simulate('pointerup') + wrapper.find('circle.endPoint').at(0).simulate('pointerup') expect(mark.finished).to.be.true() dragMove({}, { x: 10, y: 20 }) expect(mark.finished).to.be.true() diff --git a/packages/lib-classifier/src/plugins/drawingTools/experimental/components/TranscriptionLine/components/TranscriptionLineMark/TranscriptionLineMark.js b/packages/lib-classifier/src/plugins/drawingTools/experimental/components/TranscriptionLine/components/TranscriptionLineMark/TranscriptionLineMark.js index 4db8e5650b..15918d7606 100644 --- a/packages/lib-classifier/src/plugins/drawingTools/experimental/components/TranscriptionLine/components/TranscriptionLineMark/TranscriptionLineMark.js +++ b/packages/lib-classifier/src/plugins/drawingTools/experimental/components/TranscriptionLine/components/TranscriptionLineMark/TranscriptionLineMark.js @@ -4,19 +4,29 @@ import { observer } from 'mobx-react' import { DragHandle } from '@plugins/drawingTools/components' import { HANDLE_RADIUS, GRAB_STROKE_WIDTH } from '../../helpers/constants' -// Forward ref incase it is being rendered with the Tooltip -const TranscriptionLineMark = forwardRef((props, ref) => { - const { - active, - color, - handleFinishClick, - handlePointerDown, - handleRadius, - mark, - onHandleDrag, - scale - } = props +const Circle = ({ fill, r, transform, ...props }) => ( + + + +) +// Forward ref incase it is being rendered with the Tooltip +const TranscriptionLineMark = forwardRef(({ + active, + color, + handleFinishClick, + handlePointerDown, + handleRadius = HANDLE_RADIUS, + mark, + onHandleDrag, + scale +}, ref) => { const { finished, x1, @@ -29,9 +39,18 @@ const TranscriptionLineMark = forwardRef((props, ref) => { if (mark.length) { const deltaX = x2 - x1 const deltaY = y2 - y1 - offsetX = deltaX * (handleRadius / mark.length) - offsetY = deltaY * (handleRadius / mark.length) + offsetX = deltaX * (handleRadius / mark.length) / scale + offsetY = deltaY * (handleRadius / mark.length) / scale + } + + function onDragStartPoint(e, d) { + onHandleDrag({ x1: x1 + d.x, y1: y1 + d.y, x2, y2 }) + } + + function onDragEndPoint(e, d) { + onHandleDrag({ x1, y1, x2: x2 + d.x, y2: y2 + d.y }) } + return ( { {active ? onHandleDrag({ x1: x1 + d.x, y1: y1 + d.y, x2, y2 })} + dragMove={onDragStartPoint} /> : - } {active ? onHandleDrag({ x1, y1, x2: x2 + d.x, y2: y2 + d.y })} + dragMove={onDragEndPoint} /> : - } {active && !finished && - - }