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
zooniverse · Feb 13, 2025 · 031f718 · 031f718
1 parent ea62b95
commit 031f718
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 80 deletions.
diff --git 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
@@ -2,26 +2,26 @@ 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;
   &:hover {
     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 (
-    <g ref={ref} transform={transform} data-testid={testid} {...props} >
-      <StyledCircle r={radius} {...styleProps} vectorEffect={'non-scaling-stroke'} />
-      <StyledCircle r={2 * radius} fill='transparent' stroke='transparent' vectorEffect={'non-scaling-stroke'} />
+    <g ref={ref} transform={transform} data-testid={testid} >
+      <StyledCircle r={radius} {...styleProps} vectorEffect={'non-scaling-size'} />
+      <StyledCircle r={2 * radius} fill='transparent' stroke='transparent' vectorEffect={'non-scaling-size'} />
     </g>
   )
 })
@@ -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
@@ -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
@@ -14,48 +14,53 @@ 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 (
-    <g onPointerUp={active ? onFinish : undefined}>
+    <g transform={`scale(${1 / scale})`} onPointerUp={active ? onFinish : undefined}>
       <line
         x1='0'
         y1={-1 * crosshairSpace * selectedRadius}
         x2='0'
         y2={-1 * selectedRadius}
         strokeWidth={crosshairWidth}
-        vectorEffect={'non-scaling-stroke'}
+        vectorEffect={'non-scaling-size'}
       />
       <line
         x1={-1 * crosshairSpace * selectedRadius}
         y1='0'
         x2={-1 * selectedRadius}
         y2='0'
         strokeWidth={crosshairWidth}
-        vectorEffect={'non-scaling-stroke'}
+        vectorEffect={'non-scaling-size'}
       />
       <line
         x1='0'
         y1={crosshairSpace * selectedRadius}
         x2='0'
         y2={selectedRadius}
         strokeWidth={crosshairWidth}
-        vectorEffect={'non-scaling-stroke'}
+        vectorEffect={'non-scaling-size'}
       />
       <line
         x1={crosshairSpace * selectedRadius}
         y1='0'
         x2={selectedRadius}
         y2='0'
         strokeWidth={crosshairWidth}
-        vectorEffect={'non-scaling-stroke'}
+        vectorEffect={'non-scaling-size'}
       />
-      <circle r={active ? selectedRadius : radius} vectorEffect={'non-scaling-stroke'} />
+      <circle r={active ? selectedRadius : radius} vectorEffect={'non-scaling-size'} />
     </g>
   )
 }
@@ -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/...r/src/plugins/drawingTools/experimental/components/TranscriptionLine/TranscriptionLine.js b/...r/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/.../plugins/drawingTools/experimental/components/TranscriptionLine/TranscriptionLine.spec.js b/.../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/...al/components/TranscriptionLine/components/TranscriptionLineMark/TranscriptionLineMark.js b/...al/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 }) => (
+  <g transform={transform}>
+    <circle
+      fill={fill}
+      r={r}
+      stroke='currentColor'
+      vectorEffect={'non-scaling-size'}
+      {...props}
+    />
+  </g>
+)
 
+// 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 (
     <g
       color={color}
@@ -45,57 +64,48 @@ const TranscriptionLineMark = forwardRef((props, ref) => {
       {active ?
         <DragHandle
           fill='transparent'
-          radius={HANDLE_RADIUS}
+          radius={handleRadius}
           scale={scale}
           x={x1}
           y={y1}
-          dragMove={(e, d) => onHandleDrag({ x1: x1 + d.x, y1: y1 + d.y, x2, y2 })}
+          dragMove={onDragStartPoint}
         /> :
-        <circle
-          cx={x1}
-          cy={y1}
+        <Circle
           fill='transparent'
           r={handleRadius}
-          stroke='currentColor'
-          vectorEffect={'non-scaling-stroke'}
+          transform={`translate(${x1}, ${y1}) scale(${1 / scale})`}
         />
       }
       {active ?
         <DragHandle
-          radius={HANDLE_RADIUS}
+          radius={handleRadius}
           scale={scale}
           x={x2}
           y={y2}
-          dragMove={(e, d) => onHandleDrag({ x1, y1, x2: x2 + d.x, y2: y2 + d.y })}
+          dragMove={onDragEndPoint}
         /> :
-        <circle
-          cx={x2}
-          cy={y2}
+        <Circle
           fill='currentColor'
           r={handleRadius}
-          stroke='currentColor'
-          vectorEffect={'non-scaling-stroke'}
+          transform={`translate(${x2}, ${y2}) scale(${1 / scale})`}
         />
       }
 
       {active && !finished &&
         <g>
-          <circle
+          <Circle
+            className='startPoint'
             r={handleRadius}
-            cx={x1}
-            cy={y1}
-            fill="transparent"
+            transform={`translate(${x1}, ${y1}) scale(${1 / scale})`}
             onPointerDown={handlePointerDown}
             onPointerUp={handleFinishClick}
-            vectorEffect={'non-scaling-stroke'}
           />
-          <circle
+          <Circle
+            className='endPoint'
             r={handleRadius}
-            cx={x2}
-            cy={y2}
+            transform={`translate(${x2}, ${y2}) scale(${1 / scale})`}
             onPointerDown={handlePointerDown}
             onPointerUp={handleFinishClick}
-            vectorEffect={'non-scaling-stroke'}
           />
         </g>
       }