Fix previous marks from caesar render for transcription task

packages/lib-classifier/src/components/Classifier/components/MetaTools/MetaTools.js

@@ -10,11 +10,10 @@ import SHOWN_MARKS from '@helpers/shownMarks'
 
 function storeMapper (stores) {
   const { active: subject, isThereMetadata } = stores.classifierStore.subjects
-  const { activeStepTasks } = stores.classifierStore.workflowSteps
-  const [activeInteractionTask] = activeStepTasks.filter(task => task.type === 'drawing' || task.type === 'transcription') || {}
+  const { interactionTask } = stores.classifierStore.workflowSteps
   const upp = stores.classifierStore.userProjectPreferences.active
   return {
-    activeInteractionTask,
+    interactionTask,
     isThereMetadata,
     subject,
     upp
@@ -37,8 +36,15 @@ class MetaTools extends React.Component {
 
   // TODO: Add fallbacks for when Panoptes is not serializing the subject favorite info
   render () {
-    const { activeInteractionTask, className, isThereMetadata, screenSize, subject, upp } = this.props
-    const { shownMarks, marks, togglePreviousMarks, type } = activeInteractionTask || {}
+    const {
+      className,
+      interactionTask,
+      isThereMetadata,
+      screenSize,
+      subject,
+      upp
+    } = this.props
+    const { shownMarks, marks, togglePreviousMarks, type } = interactionTask
     const gap = (screenSize === 'small') ? 'xsmall' : 'small'
     const margin = (screenSize === 'small') ? { top: 'small' } : 'none'
 
@@ -60,9 +66,9 @@ class MetaTools extends React.Component {
           disabled={!subject || !upp}
           onClick={this.addToCollection}
         />
-        {activeInteractionTask && (
+        {Object.keys(interactionTask).length > 0 && (
           <HidePreviousMarksButton
-            disabled={marks.length === 0}
+            disabled={marks?.length === 0}
             shownMarks={shownMarks}
             type={type}
             onClick={(state) => togglePreviousMarks(state)}
@@ -73,25 +79,21 @@ class MetaTools extends React.Component {
   }
 }
 
-MetaTools.defaultProps = {
-  activeInteractionTask: {
-    shownMarks: SHOWN_MARKS.ALL,
-    togglePreviousMarks: () => {},
-    type: ''
-  },
+MetaTools.wrappedComponent.defaultProps = {
   className: '',
+  interactionTask: {},
   isThereMetadata: false,
   subject: null,
   upp: null
 }
 
-MetaTools.propTypes = {
-  activeInteractionTask: PropTypes.shape({
+MetaTools.wrappedComponent.propTypes = {
+  className: PropTypes.string,
+  interactionTask: PropTypes.shape({
     shownMarks: PropTypes.string,
     togglePreviousMarks: PropTypes.func,
     type: PropTypes.string
   }),
-  className: PropTypes.string,
   isThereMetadata: PropTypes.bool,
   screenSize: PropTypes.string,
   subject: PropTypes.object,

packages/lib-classifier/src/components/Classifier/components/MetaTools/MetaTools.spec.js

@@ -7,16 +7,18 @@ import Metadata from './components/Metadata'
 import CollectionsButton from './components/CollectionsButton'
 import HidePreviousMarksButton from './components/HidePreviousMarksButton'
 import { Factory } from 'rosie'
+import SHOWN_MARKS from '@helpers/shownMarks'
 
 const subjectWithMetadata = Factory.build('subject', { metadata: { foo: 'bar' } })
 
 const favoriteSubject = Factory.build('subject', { favorite: true })
 
 const spy = sinon.spy()
 
-const activeInteractionTask = {
+const interactionTask = {
   hidePreviousMarks: false,
   marks: [{ x: 0, y: 0 }],
+  shownMarks: SHOWN_MARKS.ALL,
   togglePreviousMarks: spy
 }
 
@@ -112,7 +114,7 @@ describe('Component > MetaTools', function () {
       let wrapper
 
       beforeEach(function () {
-        wrapper = shallow(<MetaTools.wrappedComponent activeInteractionTask={activeInteractionTask} />)
+        wrapper = shallow(<MetaTools.wrappedComponent interactionTask={interactionTask} />)
       })
 
       it('should render', function () {

packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/InteractionLayerContainer.js

@@ -4,7 +4,7 @@ import PropTypes from 'prop-types'
 import React, { Component } from 'react'
 
 import InteractionLayer from './InteractionLayer'
-import DrawingToolMarks from './components/DrawingToolMarks'
+import PreviousMarks from './components/PreviousMarks'
 import SHOWN_MARKS from '@helpers/shownMarks'
 
 function storeMapper (stores) {
@@ -61,16 +61,6 @@ class InteractionLayerContainer extends Component {
 
     return (
       <>
-        {shownMarks === SHOWN_MARKS.ALL && interactionTaskAnnotations.map((annotation) => {
-          const annotationValuesPerFrame = annotation.value.filter(value => value.frame === frame)
-          return (
-            <DrawingToolMarks
-              key={annotation.task}
-              marks={annotationValuesPerFrame}
-              scale={scale}
-            />
-          )
-        })}
         {activeInteractionTask && activeTool &&
           <InteractionLayer
             activeMark={activeMark}
@@ -87,6 +77,7 @@ class InteractionLayerContainer extends Component {
             width={width}
           />
         }
+        <PreviousMarks scale={scale} />
       </>
     )
   }

packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/InteractionLayerContainer.spec.js

@@ -2,7 +2,7 @@ import { shallow } from 'enzyme'
 import React from 'react'
 import InteractionLayerContainer from './InteractionLayerContainer'
 import InteractionLayer from './InteractionLayer'
-import DrawingToolMarks from './components/DrawingToolMarks'
+import PreviousMarks from './components/PreviousMarks'
 import SHOWN_MARKS from '@helpers/shownMarks'
 
 describe('Component > InteractionLayerContainer', function () {
@@ -41,6 +41,17 @@ describe('Component > InteractionLayerContainer', function () {
     expect(wrapper).to.be.ok()
   })
 
+  it('should render PreviousMarks',  function () {
+    const wrapper = shallow(
+      <InteractionLayerContainer.wrappedComponent
+        activeInteractionTask={drawingTask}
+        height={height}
+        width={width}
+      />
+    )
+    expect(wrapper.find(PreviousMarks)).to.have.lengthOf(1)
+  })
+
   describe('with an active drawing task and drawing tool', function () {
     let wrapper
 
@@ -59,34 +70,6 @@ describe('Component > InteractionLayerContainer', function () {
     })
   })
 
-  describe('with annotations from previous drawing or transcription tasks', function () {
-    it('should render DrawingToolMarks', function () {
-      const wrapper = shallow(
-        <InteractionLayerContainer.wrappedComponent
-          interactionTaskAnnotations={drawingAnnotations}
-          frame={0}
-          height={height}
-          width={width}
-        />
-      )
-      expect(wrapper.find(DrawingToolMarks)).to.have.lengthOf(2)
-    })
-
-    it('should render DrawingToolMarks with marks per frame', function () {
-      const wrapper = shallow(
-        <InteractionLayerContainer.wrappedComponent
-          interactionTaskAnnotations={drawingAnnotations}
-          frame={0}
-          height={height}
-          width={width}
-        />
-      )
-
-      expect(wrapper.find(DrawingToolMarks).first().prop('marks')).to.have.lengthOf(1)
-      expect(wrapper.find(DrawingToolMarks).last().prop('marks')).to.have.lengthOf(2)
-    })
-  })
-
   describe('with transcription task', function () {
     let wrapper
     const hidingTask = {
@@ -189,10 +172,6 @@ describe('Component > InteractionLayerContainer', function () {
         )
       })
 
-      it('should hide previous annotations', function () {
-        expect(wrapper.find(DrawingToolMarks)).to.have.lengthOf(0)
-      })
-
       it('should hide all marks before the hiding index', function () {
         const { marks } = wrapper.find(InteractionLayer).props()
         expect(marks).to.have.lengthOf(1)
@@ -215,7 +194,6 @@ describe('Component > InteractionLayerContainer', function () {
 
         const marks = wrapper.find(InteractionLayer).props().marks
         expect(marks).to.have.lengthOf(2)
-        expect(wrapper.find(DrawingToolMarks)).to.have.lengthOf(0)
       })
     })
   })

packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.js

@@ -0,0 +1,37 @@
+import React from 'react'
+import PropTypes from 'prop-types'
+import DrawingToolMarks from '../DrawingToolMarks'
+import SHOWN_MARKS from '@helpers/shownMarks'
+
+function PreviousMarks (props) {
+  const {
+    frame = 0,
+    interactionTaskAnnotations = [],
+    scale = 1,
+    shownMarks = 'ALL'
+  } = props
+  const marksToShow = shownMarks === SHOWN_MARKS.ALL || shownMarks === SHOWN_MARKS.USER
+
+  if (interactionTaskAnnotations?.length > 0 && marksToShow) {
+    // Wrapping the array in an react fragment because enzyme errors otherwise 
+    // React v16 allows this, though. 
+    return (
+      <>
+        {interactionTaskAnnotations.map((annotation) => {
+          const annotationValuesPerFrame = annotation.value.filter(value => value.frame === frame)
+          return (
+            <DrawingToolMarks
+              key={annotation.task}
+              marks={annotationValuesPerFrame}
+              scale={scale}
+            />
+          )
+        })}
+      </>
+    )
+  }
+
+  return null
+}
+
+export default PreviousMarks

packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js

@@ -0,0 +1,91 @@
+import { shallow } from 'enzyme'
+import React from 'react'
+import PreviousMarks from './PreviousMarks'
+import DrawingToolMarks from '../DrawingToolMarks'
+import SHOWN_MARKS from '@helpers/shownMarks'
+
+describe('Component > PreviousMarks', function () {
+  const interactionTaskAnnotations = [{
+    task: 'T2',
+    value: [
+      { id: 'point1', frame: 0, toolIndex: 0, x: 100, y: 200 }
+    ]
+  }, {
+    task: 'T3',
+    value: [
+      { id: 'line1', frame: 0, toolIndex: 0, x1: 100, y1: 200, x2: 150, y2: 200 },
+      { id: 'line2', frame: 0, toolIndex: 0, x1: 200, y1: 300, x2: 250, y2: 300 },
+      { id: 'line3', frame: 1, toolIndex: 0, x1: 150, y1: 250, x2: 100, y2: 250 },
+      { id: 'line4', frame: 1, toolIndex: 0, x1: 250, y1: 350, x2: 200, y2: 350 }
+    ]
+  }]
+
+  it('should render without crashing', function () {
+    const wrapper = shallow(<PreviousMarks />)
+    expect(wrapper).to.be.ok()
+  })
+
+  it('should pass scale along as a prop to each DrawingToolMarks component', function ()  {
+    const wrapper = shallow(<PreviousMarks interactionTaskAnnotations={interactionTaskAnnotations} scale={2} />)
+    expect(wrapper.find(DrawingToolMarks).first().props().scale).to.equal(2)
+    expect(wrapper.find(DrawingToolMarks).last().props().scale).to.equal(2)
+  })
+
+  describe('when there are no interaction task annotations', function () {
+    it('should render null', function () {
+      const wrapper = shallow(<PreviousMarks />)
+      expect(wrapper.html()).to.be.null()
+    })
+  })
+
+  describe('when there are interaction task annotations', function () {
+    it('should render a DrawingToolMarks for each task', function ()  {
+      const wrapper = shallow(<PreviousMarks interactionTaskAnnotations={interactionTaskAnnotations} />)
+      expect(wrapper.find(DrawingToolMarks)).to.have.lengthOf(2)
+      expect(wrapper.find(DrawingToolMarks).first().key()).to.equal(interactionTaskAnnotations[0].task)
+      expect(wrapper.find(DrawingToolMarks).last().key()).to.equal(interactionTaskAnnotations[1].task)
+    })
+
+    it('should pass along the correct number of marks per task by frame', function () {
+      const firstTaskAnnotationsFirstFrameFirstMark = interactionTaskAnnotations[0].value[0]
+      const secondTaskAnnotationsFirstFrameFirstMark = interactionTaskAnnotations[1].value[0]
+      const secondTaskAnnotationsFirstFrameSecondMark = interactionTaskAnnotations[1].value[1]
+      const secondTaskAnnotationsSecondFrameFirstMark = interactionTaskAnnotations[1].value[2]
+      const secondTaskAnnotationsSecondFrameSecondMark = interactionTaskAnnotations[1].value[3]
+      const wrapper = shallow(<PreviousMarks interactionTaskAnnotations={interactionTaskAnnotations} />)
+      let firstTaskAnnotationRenderedMarks = wrapper.find(DrawingToolMarks).first().props().marks
+      let secondTaskAnnotationRenderedMarks = wrapper.find(DrawingToolMarks).last().props().marks
+      expect(firstTaskAnnotationRenderedMarks).to.have.lengthOf(1)
+      expect(firstTaskAnnotationRenderedMarks[0]).to.deep.equal(firstTaskAnnotationsFirstFrameFirstMark)
+      expect(secondTaskAnnotationRenderedMarks).to.have.lengthOf(2)
+      expect(secondTaskAnnotationRenderedMarks[0]).to.deep.equal(secondTaskAnnotationsFirstFrameFirstMark)
+      expect(secondTaskAnnotationRenderedMarks[1]).to.deep.equal(secondTaskAnnotationsFirstFrameSecondMark)
+      wrapper.setProps({ frame: 1 })
+      firstTaskAnnotationRenderedMarks = wrapper.find(DrawingToolMarks).first().props().marks
+      secondTaskAnnotationRenderedMarks = wrapper.find(DrawingToolMarks).last().props().marks
+      expect(firstTaskAnnotationRenderedMarks).to.have.lengthOf(0)
+      expect(secondTaskAnnotationRenderedMarks).to.have.lengthOf(2)
+      expect(secondTaskAnnotationRenderedMarks[0]).to.deep.equal(secondTaskAnnotationsSecondFrameFirstMark)
+      expect(secondTaskAnnotationRenderedMarks[1]).to.deep.equal(secondTaskAnnotationsSecondFrameSecondMark)
+    })
+  })
+
+  describe('when shown marks is USER', function () {
+    it('should not render DrawingToolMarks if there are not annotations', function () {
+      const wrapper = shallow(<PreviousMarks shownMarks={SHOWN_MARKS.USER} />)
+      expect(wrapper.html()).to.be.null()
+    })
+
+    it('should render DrawingToolMarks if there are also annotations', function () {
+      const wrapper = shallow(<PreviousMarks interactionTaskAnnotations={interactionTaskAnnotations} shownMarks={SHOWN_MARKS.USER} />)
+      expect(wrapper.find(DrawingToolMarks)).to.have.lengthOf(2)
+    })
+  })
+
+  describe('when shown marks is NONE', function () {
+    it('should render null', function ()  {
+      const wrapper = shallow(<PreviousMarks shownMarks={SHOWN_MARKS.NONE} />)
+      expect(wrapper.html()).to.be.null()
+    })
+  })
+})

packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarksConnector.js

@@ -0,0 +1,41 @@
+import React from 'react'
+import PropTypes from 'prop-types'
+import { MobXProviderContext, observer } from 'mobx-react'
+import PreviousMarks from './PreviousMarks'
+
+function useStores() {
+  const stores = React.useContext(MobXProviderContext)
+  const { interactionTask } = stores.classifierStore.workflowSteps
+  const {
+    active: classification
+  } = stores.classifierStore.classifications
+  const interactionTaskAnnotations = classification?.interactionTaskAnnotations || []
+  const {
+    frame
+  } = stores.classifierStore.subjectViewer
+  return { frame, interactionTask, interactionTaskAnnotations }
+}
+
+function PreviousMarksConnector({ scale, ...rest }) {
+  const {
+    frame = 0,
+    interactionTask,
+    interactionTaskAnnotations
+  } = useStores()
+  const { shownMarks } = interactionTask
+  return (
+    <PreviousMarks
+      frame={frame}
+      interactionTaskAnnotations={interactionTaskAnnotations}
+      scale={scale}
+      shownMarks={shownMarks}
+      {...rest}
+    />
+  )
+}
+
+PreviousMarksConnector.propTypes = {
+  scale: PropTypes.number
+}
+
+export default observer(PreviousMarksConnector)