From dda0d20e3b81dcb1789421cdcdd79f255302a13a Mon Sep 17 00:00:00 2001 From: Sarah Allen Date: Fri, 22 Jan 2021 15:20:22 -0600 Subject: [PATCH 1/4] Refactor to use transcription task regardless if in active step for displaying transcribed lines and the hide marks button. --- .../components/MetaTools/MetaTools.js | 34 ++-- .../components/MetaTools/MetaTools.spec.js | 6 +- .../InteractionLayerContainer.js | 13 +- .../InteractionLayerContainer.spec.js | 46 ++--- .../components/PreviousMarks/PreviousMarks.js | 37 ++++ .../PreviousMarks/PreviousMarks.spec.js | 86 +++++++++ .../PreviousMarks/PreviousMarksConnector.js | 41 ++++ .../PreviousMarksConnector.spec.js | 179 ++++++++++++++++++ .../components/PreviousMarks/index.js | 1 + .../TranscribedLinesContainer.js | 22 ++- .../TranscribedLinesContainer.spec.js | 27 ++- .../components/DragHandle/DragHandle.js | 1 + .../store/Classification/Classification.js | 5 + .../src/store/Workflow/Workflow.js | 4 +- .../src/store/WorkflowStepStore.js | 21 ++ 15 files changed, 445 insertions(+), 78 deletions(-) create mode 100644 packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.js create mode 100644 packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js create mode 100644 packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarksConnector.js create mode 100644 packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarksConnector.spec.js create mode 100644 packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/index.js diff --git a/packages/lib-classifier/src/components/Classifier/components/MetaTools/MetaTools.js b/packages/lib-classifier/src/components/Classifier/components/MetaTools/MetaTools.js index 0a48ef3295..0746da34b9 100644 --- a/packages/lib-classifier/src/components/Classifier/components/MetaTools/MetaTools.js +++ b/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 && ( 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, diff --git a/packages/lib-classifier/src/components/Classifier/components/MetaTools/MetaTools.spec.js b/packages/lib-classifier/src/components/Classifier/components/MetaTools/MetaTools.spec.js index e023ed3b6a..4647712af4 100644 --- a/packages/lib-classifier/src/components/Classifier/components/MetaTools/MetaTools.spec.js +++ b/packages/lib-classifier/src/components/Classifier/components/MetaTools/MetaTools.spec.js @@ -7,6 +7,7 @@ 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' } }) @@ -14,9 +15,10 @@ 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() + wrapper = shallow() }) it('should render', function () { diff --git a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/InteractionLayerContainer.js b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/InteractionLayerContainer.js index de3cdca513..3e84b8fc4c 100644 --- a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/InteractionLayerContainer.js +++ b/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 ( - - ) - })} {activeInteractionTask && activeTool && } + ) } diff --git a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/InteractionLayerContainer.spec.js b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/InteractionLayerContainer.spec.js index f96bc101e2..6c4de67c37 100644 --- a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/InteractionLayerContainer.spec.js +++ b/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( + + ) + 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( - - ) - expect(wrapper.find(DrawingToolMarks)).to.have.lengthOf(2) - }) - - it('should render DrawingToolMarks with marks per frame', function () { - const wrapper = shallow( - - ) - - 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) }) }) }) diff --git a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.js b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.js new file mode 100644 index 0000000000..458b53ea5c --- /dev/null +++ b/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 ( + + ) + })} + + ) + } + + return null +} + +export default PreviousMarks \ No newline at end of file diff --git a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js new file mode 100644 index 0000000000..a350b32202 --- /dev/null +++ b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js @@ -0,0 +1,86 @@ +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() + expect(wrapper).to.be.ok() + }) + + it('should pass scale along as a prop to each DrawingToolMarks component', function () { + const wrapper = shallow() + 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() + expect(wrapper.html()).to.be.null() + }) + }) + + describe('when there are interaction task annotations', function () { + it('should render a DrawingToolMarks for each annotation', function () { + const wrapper = shallow() + 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 wrapper = shallow() + let firstAnnotationMarks = wrapper.find(DrawingToolMarks).first().props().marks + let secondAnnotationMarks = wrapper.find(DrawingToolMarks).last().props().marks + expect(firstAnnotationMarks).to.have.lengthOf(1) + expect(firstAnnotationMarks[0]).to.deep.equal(interactionTaskAnnotations[0].value[0]) + expect(secondAnnotationMarks).to.have.lengthOf(2) + expect(secondAnnotationMarks[0]).to.deep.equal(interactionTaskAnnotations[1].value[0]) + expect(secondAnnotationMarks[1]).to.deep.equal(interactionTaskAnnotations[1].value[1]) + wrapper.setProps({ frame: 1 }) + firstAnnotationMarks = wrapper.find(DrawingToolMarks).first().props().marks + secondAnnotationMarks = wrapper.find(DrawingToolMarks).last().props().marks + expect(firstAnnotationMarks).to.have.lengthOf(0) + expect(secondAnnotationMarks).to.have.lengthOf(2) + expect(secondAnnotationMarks[0]).to.deep.equal(interactionTaskAnnotations[1].value[2]) + expect(secondAnnotationMarks[1]).to.deep.equal(interactionTaskAnnotations[1].value[3]) + }) + }) + + describe('when shown marks is USER', function () { + it('should not render DrawingToolMarks if there are not annotations', function () { + const wrapper = shallow() + expect(wrapper.html()).to.be.null() + }) + + it('should render DrawingToolMarks if there are also annotations', function () { + const wrapper = shallow() + expect(wrapper.find(DrawingToolMarks)).to.have.lengthOf(2) + }) + }) + + describe('when shown marks is NONE', function () { + it('should render null', function () { + const wrapper = shallow() + expect(wrapper.html()).to.be.null() + }) + }) +}) \ No newline at end of file diff --git a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarksConnector.js b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarksConnector.js new file mode 100644 index 0000000000..af5bb6ad26 --- /dev/null +++ b/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 ( + + ) +} + +PreviousMarksConnector.propTypes = { + scale: PropTypes.number +} + +export default observer(PreviousMarksConnector) \ No newline at end of file diff --git a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarksConnector.spec.js b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarksConnector.spec.js new file mode 100644 index 0000000000..69f942d43a --- /dev/null +++ b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarksConnector.spec.js @@ -0,0 +1,179 @@ +import { shallow } from 'enzyme' +import sinon from 'sinon' +import React from 'react' +import PreviousMarksConnector from './PreviousMarksConnector' +import PreviousMarks from './PreviousMarks' +import cuid from 'cuid' + +const mockStores = { + classifications: { + active: { + interactionTaskAnnotations: [] + } + }, + subjectViewer: { + frame: 1 + }, + workflowSteps: { + interactionTask: {} + } +} + +const drawingAnnotation = { + id: cuid(), + task: 'T0', + taskType: 'drawing' +} + +const transcriptionAnnotation = { + id: cuid(), + task: 'T0', + taskType: 'transcription' +} + +const mockStoresWithDrawingAnnotations = { + classifications: { + active: { + interactionTaskAnnotations: [drawingAnnotation] + } + }, + subjectViewer: { + frame: 1 + }, + workflowSteps: { + interactionTask: { + shownMarks: 'USER', + type: 'drawing' + } + } +} + +const mockStoresWithTranscriptionAnnotations = { + classifications: { + active: { + interactionTaskAnnotations: [transcriptionAnnotation] + } + }, + subjectViewer: { + frame: 1 + }, + workflowSteps: { + interactionTask: { + shownMarks: 'USER', + type: 'transcription' + } + } +} + +describe('Component > PreviousMarksConnector', function () { + let mockUseContext + afterEach(function () { + mockUseContext.restore() + }) + + it('should render without crashing', function () { + mockUseContext = sinon.stub(React, 'useContext').callsFake(() => { + return { + classifierStore: mockStores + } + }) + const wrapper = shallow() + expect(wrapper).to.be.ok() + }) + + it('should pass along the frame index from the subject viewer store', function () { + mockUseContext = sinon.stub(React, 'useContext').callsFake(() => { + return { + classifierStore: mockStores + } + }) + const wrapper = shallow() + expect(wrapper.find(PreviousMarks).props().frame).to.equal(1) + }) + + it('should pass along the scale from props', function () { + mockUseContext = sinon.stub(React, 'useContext').callsFake(() => { + return { + classifierStore: mockStores + } + }) + const wrapper = shallow() + expect(wrapper.find(PreviousMarks).props().scale).to.equal(2) + }) + + describe('when there is not an interactive task', function () { + it('should pass along an undefined shownMarks', function () { + mockUseContext = sinon.stub(React, 'useContext').callsFake(() => { + return { + classifierStore: mockStores + } + }) + const wrapper = shallow() + expect(wrapper.find(PreviousMarks).props().shownMarks).to.be.undefined() + }) + }) + + describe('when there is an interactive task', function () { + it('should pass along the type of shownMarks from the drawing task', function () { + mockUseContext = sinon.stub(React, 'useContext').callsFake(() => { + return { + classifierStore: mockStoresWithDrawingAnnotations + } + }) + const wrapper = shallow() + expect(wrapper.find(PreviousMarks).props().shownMarks).to.equal('USER') + }) + + it('should pass along the type of shownMarks from the transcription task', function () { + mockUseContext = sinon.stub(React, 'useContext').callsFake(() => { + return { + classifierStore: mockStoresWithTranscriptionAnnotations + } + }) + const wrapper = shallow() + expect(wrapper.find(PreviousMarks).props().shownMarks).to.equal('USER') + }) + }) + + describe('when there are no interaction task annotations', function () { + it('should pass along an empty annotations array', function () { + mockUseContext = sinon.stub(React, 'useContext').callsFake(() => { + return { + classifierStore: mockStores + } + }) + const wrapper = shallow() + const { interactionTaskAnnotations } = wrapper.find(PreviousMarks).props() + expect(interactionTaskAnnotations).to.be.an('array') + expect(interactionTaskAnnotations).to.be.empty() + }) + }) + + describe('when task annotations are from drawing or transcription', function () { + it('should pass along those drawing annotations in an array', function () { + mockUseContext = sinon.stub(React, 'useContext').callsFake(() => { + return { + classifierStore: mockStoresWithDrawingAnnotations + } + }) + const wrapper = shallow() + const { interactionTaskAnnotations } = wrapper.find(PreviousMarks).props() + expect(interactionTaskAnnotations).to.be.an('array') + expect(interactionTaskAnnotations).to.have.lengthOf(1) + expect(interactionTaskAnnotations[0]).to.equal(drawingAnnotation) + }) + + it('should pass along those transcription annotations in an array', function () { + mockUseContext = sinon.stub(React, 'useContext').callsFake(() => { + return { + classifierStore: mockStoresWithTranscriptionAnnotations + } + }) + const wrapper = shallow() + const { interactionTaskAnnotations } = wrapper.find(PreviousMarks).props() + expect(interactionTaskAnnotations).to.be.an('array') + expect(interactionTaskAnnotations).to.have.lengthOf(1) + expect(interactionTaskAnnotations[0]).to.equal(transcriptionAnnotation) + }) + }) +}) diff --git a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/index.js b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/index.js new file mode 100644 index 0000000000..9bc213c190 --- /dev/null +++ b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/index.js @@ -0,0 +1 @@ +export { default } from './PreviousMarksConnector' \ No newline at end of file diff --git a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/TranscribedLines/TranscribedLinesContainer.js b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/TranscribedLines/TranscribedLinesContainer.js index c4d579cae2..586f6c6781 100644 --- a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/TranscribedLines/TranscribedLinesContainer.js +++ b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/TranscribedLines/TranscribedLinesContainer.js @@ -10,26 +10,28 @@ function useStores () { const { active: workflow } = stores.classifierStore.workflows - const { - activeStepTasks - } = stores.classifierStore.workflowSteps const subject = stores.classifierStore.subjects.active const { consensusLines } = subject.transcriptionReductions || {} - const [activeTranscriptionTask] = activeStepTasks.filter(task => task.type === 'transcription') - return { activeTranscriptionTask, consensusLines, frame, workflow } + + // We expect there to only be one + const [transcriptionTask] = stores.classifierStore.workflowSteps.findTasksByType('transcription') + + return { transcriptionTask, consensusLines, frame, workflow } } -function TranscribedLinesContainer ({ scale = 1 }) { +function TranscribedLinesContainer (props) { const { - activeTranscriptionTask = {}, + transcriptionTask = {}, frame = 0, consensusLines = [], workflow = { usesTranscriptionTask: false } } = useStores() - - const { shownMarks } = activeTranscriptionTask + const { + scale = 1 + } = props + const { shownMarks } = transcriptionTask const visibleLinesPerFrame = consensusLines.filter(line => line.frame === frame) if (workflow?.usesTranscriptionTask && shownMarks === SHOWN_MARKS.ALL && visibleLinesPerFrame.length > 0) { @@ -37,7 +39,7 @@ function TranscribedLinesContainer ({ scale = 1 }) { ) } diff --git a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/TranscribedLines/TranscribedLinesContainer.spec.js b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/TranscribedLines/TranscribedLinesContainer.spec.js index b6d7082023..ee2030060e 100644 --- a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/TranscribedLines/TranscribedLinesContainer.spec.js +++ b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/TranscribedLines/TranscribedLinesContainer.spec.js @@ -28,7 +28,13 @@ const mockStoresWithTranscriptionTask = { shownMarks: 'ALL', type: 'transcription' } - ] + ], + findTasksByType: () => { + return [{ + shownMarks: 'ALL', + type: 'transcription' + }] + } } } @@ -68,7 +74,8 @@ const mockStoresWithoutTranscriptionTask = { shownMarks: 'ALL', type: 'drawing' } - ] + ], + findTasksByType: () => { return []} } } @@ -142,7 +149,13 @@ describe('Component > TranscribedLinesContainer', function () { shownMarks: 'USER', type: 'transcription' } - ] + ], + findTasksByType: () => { + return [{ + shownMarks: 'USER', + type: 'transcription' + }] + } } } mockUseContext = sinon.stub(React, 'useContext').callsFake(() => { @@ -162,7 +175,13 @@ describe('Component > TranscribedLinesContainer', function () { shownMarks: 'NONE', type: 'transcription' } - ] + ], + findTasksByType: () => { + return [{ + shownMarks: 'NONE', + type: 'transcription' + }] + } } } mockUseContext = sinon.stub(React, 'useContext').callsFake(() => { 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 32e97f8ca6..0a7f491f04 100644 --- a/packages/lib-classifier/src/plugins/drawingTools/components/DragHandle/DragHandle.js +++ b/packages/lib-classifier/src/plugins/drawingTools/components/DragHandle/DragHandle.js @@ -4,6 +4,7 @@ import styled from 'styled-components' import draggable from '../draggable' const StyledCircle = styled('circle')` + stroke-width: 2; &:hover { cursor: move; } diff --git a/packages/lib-classifier/src/store/Classification/Classification.js b/packages/lib-classifier/src/store/Classification/Classification.js index f5445ab895..3f6baa8cf4 100644 --- a/packages/lib-classifier/src/store/Classification/Classification.js +++ b/packages/lib-classifier/src/store/Classification/Classification.js @@ -25,6 +25,11 @@ const Classification = types }) snapshot = Object.assign({}, snapshot, { annotations }) return snapshot + }, + + get interactionTaskAnnotations () { + const annotations = Array.from(self.annotations.values()) || [] + return annotations.filter(annotation => (getType(annotation).name === 'DrawingAnnotation' || getType(annotation).name === 'TranscriptionAnnotation')) } })) .preProcessSnapshot(snapshot => { diff --git a/packages/lib-classifier/src/store/Workflow/Workflow.js b/packages/lib-classifier/src/store/Workflow/Workflow.js index e8d6ef08d0..93589259b6 100644 --- a/packages/lib-classifier/src/store/Workflow/Workflow.js +++ b/packages/lib-classifier/src/store/Workflow/Workflow.js @@ -37,9 +37,11 @@ const Workflow = types }, get usesTranscriptionTask () { - return self.tasks && Object.values(self.tasks).some(task => { + const anyTranscriptionTasks = self.tasks && Object.values(self.tasks).some(task => { return task.type === 'transcription' }) + + return anyTranscriptionTasks } })) diff --git a/packages/lib-classifier/src/store/WorkflowStepStore.js b/packages/lib-classifier/src/store/WorkflowStepStore.js index 9a71a42d84..012a5d0f96 100644 --- a/packages/lib-classifier/src/store/WorkflowStepStore.js +++ b/packages/lib-classifier/src/store/WorkflowStepStore.js @@ -47,6 +47,27 @@ const WorkflowStepStore = types } return false + }, + + findTasksByType (type) { + const tasksByType = Array.from(self.steps).map(([stepKey, step]) => { + if (step?.tasks) { + return Object.values(step.tasks).filter(task => { + return task.type === type + }) + } + + return [] + }) + + return tasksByType.flat() + }, + + get interactionTask () { + const [activeDrawingTask] = self.activeStepTasks.filter(task => task.type === 'drawing') + const [transcriptionTask] = self.findTasksByType('transcription') + + return activeDrawingTask || transcriptionTask || {} } })) .actions(self => { From c2b1614a2d66997e088ba84b224b8facfd39325b Mon Sep 17 00:00:00 2001 From: Sarah Allen Date: Fri, 22 Jan 2021 17:00:42 -0600 Subject: [PATCH 2/4] Add tests for new store computed views --- .../Classification/Classification.spec.js | 65 +++++++++- .../src/store/WorkflowStepStore.spec.js | 120 ++++++++++++++++++ .../lib-classifier/test/factories/index.js | 1 + 3 files changed, 185 insertions(+), 1 deletion(-) diff --git a/packages/lib-classifier/src/store/Classification/Classification.spec.js b/packages/lib-classifier/src/store/Classification/Classification.spec.js index 49a8347621..3f826db418 100644 --- a/packages/lib-classifier/src/store/Classification/Classification.spec.js +++ b/packages/lib-classifier/src/store/Classification/Classification.spec.js @@ -1,5 +1,7 @@ -import { getSnapshot } from 'mobx-state-tree' import taskRegistry from '@plugins/tasks' +import TranscriptionLine from '@plugins/drawingTools/experimental/models/marks/TranscriptionLine' +import Point from '@plugins/drawingTools/models/marks/Point' + import Classification, { ClassificationMetadata } from './' describe('Model > Classification', function () { @@ -100,4 +102,65 @@ describe('Model > Classification', function () { expect(snapshot.annotations).to.deep.equal([ firstAnnotation.toSnapshot(), secondAnnotation.toSnapshot() ]) }) }) + + describe('interactionTaskAnnotations computed view', function () { + let singleChoice, text, drawing, transcription + + before(function () { + const singleChoiceTask = taskRegistry.get('single') + const textTask = taskRegistry.get('text') + const drawingTask = taskRegistry.get('drawing') + const transcriptionTask = taskRegistry.get('transcription') + singleChoice = singleChoiceTask.TaskModel.create({ + question: 'yes or no?', + answers: ['yes', 'no'], + taskKey: 'T1', + type: 'single' + }) + text = textTask.TaskModel.create({ + instruction: 'type something', + taskKey: 'T0', + type: 'text' + }) + drawing = drawingTask.TaskModel.create({ + instruction: 'draw something', + taskKey: 'T2', + type: 'drawing' + }) + transcription = transcriptionTask.TaskModel.create({ + instruction: 'transcribe the text', + taskKey: 'T3', + type: 'transcription' + }) + }) + + it('should return an empty array if the stored annotations are not for drawing or transcription tasks', function () { + model.addAnnotation(singleChoice, 0) + model.addAnnotation(text, 'This is a text task') + expect(model.interactionTaskAnnotations).to.be.an('array') + expect(model.interactionTaskAnnotations).to.be.empty() + model.removeAnnotation('T0') + model.removeAnnotation('T1') + }) + + it('should return an array of annotations if the stored annotations are for drawing tasks', function () { + const pointMark = Point.create({ id: 'point1', frame: 0, toolIndex: 0, x: 100, y: 200, toolType: 'point' }) + model.addAnnotation(drawing, [pointMark]) + const annotations = model.interactionTaskAnnotations + expect(annotations).to.be.an('array') + expect(annotations).to.have.lengthOf(1) + expect(annotations[0].task).to.equal(drawing.taskKey) + model.removeAnnotation('T2') + }) + + it('should return an array of annotations if the stored annotations are for transcription tasks', function () { + const transcriptionMark = TranscriptionLine.create({ id: 'transcriptionline1', frame: 0, toolIndex: 0, x1: 100, y1: 200, x2: 150, y2: 200, toolType: 'transcriptionLine' }) + model.addAnnotation(transcription, [transcriptionMark]) + const annotations = model.interactionTaskAnnotations + expect(annotations).to.be.an('array') + expect(annotations).to.have.lengthOf(1) + expect(annotations[0].task).to.deep.equal(transcription.taskKey) + model.removeAnnotation('T3') + }) + }) }) diff --git a/packages/lib-classifier/src/store/WorkflowStepStore.spec.js b/packages/lib-classifier/src/store/WorkflowStepStore.spec.js index 7c2391f881..101c703c95 100644 --- a/packages/lib-classifier/src/store/WorkflowStepStore.spec.js +++ b/packages/lib-classifier/src/store/WorkflowStepStore.spec.js @@ -2,9 +2,11 @@ import RootStore from './RootStore' import WorkflowStepStore from './WorkflowStepStore' import { + DrawingTaskFactory, MultipleChoiceTaskFactory, ProjectFactory, SingleChoiceTaskFactory, + TranscriptionTaskFactory, WorkflowFactory } from '@test/factories' import { Factory } from 'rosie' @@ -376,4 +378,122 @@ describe('Model > WorkflowStepStore', function () { expect(rootStore.workflowSteps.shouldWeShowDoneAndTalkButton).to.be.true() }) }) + + describe('Views > findTasksByType', function () { + let rootStore, workflow + const singleChoiceTask = SingleChoiceTaskFactory.build({ + answers: [ + { label: 'Yes', next: 'T4' }, + { label: 'No', next: 'T4' } + ] + }) + const multipleChoiceTaskOne = MultipleChoiceTaskFactory.build() + const multipleChoiceTaskTwo = MultipleChoiceTaskFactory.build() + before(async function () { + workflow = WorkflowFactory.build({ + first_task: 'T1', + tasks: { + T1: singleChoiceTask, + T2: multipleChoiceTaskOne, + T3: multipleChoiceTaskTwo, + } + }) + const project = ProjectFactory.build({}, { activeWorkflowId: workflow.id }) + const panoptesClientStub = stubPanoptesJs({ + projects: project, + subjects: Factory.buildList('subject', 10), + workflows: workflow + }) + rootStore = await setupStores(panoptesClientStub, project, workflow) + }) + + it('should return tasks that equal the task type argument given', function () { + const singleChoiceTasks = rootStore.workflowSteps.findTasksByType('single') + const multipleChoiceTasks = rootStore.workflowSteps.findTasksByType('multiple') + const drawingTasks = rootStore.workflowSteps.findTasksByType('drawing') + expect(singleChoiceTasks).to.have.lengthOf(1) + expect(singleChoiceTasks[0].type).to.equal('single') + expect(multipleChoiceTasks).to.have.lengthOf(2) + expect(multipleChoiceTasks[0].type).to.equal('multiple') + expect(multipleChoiceTasks[1].type).to.equal('multiple') + expect(drawingTasks).to.have.lengthOf(0) + }) + }) + + describe('Views > interactionTask', function () { + let transcriptionWorkflow, manyStepWorkflow, drawingWorkflow, singleChoiceWorkflow + const subjects = Factory.buildList('subject', 10) + + before(function () { + transcriptionWorkflow = WorkflowFactory.build({ + steps: [ + ['S1', { taskKeys: ['T1'] }] + ], + tasks: { + T1: TranscriptionTaskFactory.build() + } + }) + + manyStepWorkflow = WorkflowFactory.build({ + steps: [ + ['S1', { taskKeys: ['T0'] }], + ['S2', { taskKeys: ['T1'] }] + ], + tasks: { + T0: SingleChoiceTaskFactory.build(), + T1: DrawingTaskFactory.build() + } + }) + + drawingWorkflow = WorkflowFactory.build({ + steps: [ + ['S1', { taskKeys: ['T1'] }] + ], + tasks: { + T1: DrawingTaskFactory.build() + } + }) + + singleChoiceWorkflow = WorkflowFactory.build({ + steps: [ + ['S1', { taskKeys: ['T1'] }] + ], + tasks: { + T1: SingleChoiceTaskFactory.build() + } + }) + }) + + it('should return an empty object if the workflow does not have either an active drawing or transcription task', async function () { + const project = ProjectFactory.build({}, { activeWorkflowId: singleChoiceWorkflow.id }) + const subjects = Factory.buildList('subject', 10) + const panoptesClientStub = stubPanoptesJs({ workflows: singleChoiceWorkflow, subjects }) + const rootStore = await setupStores(panoptesClientStub, project, singleChoiceWorkflow) + expect(rootStore.workflowSteps.interactionTask).to.be.empty() + }) + + it('should return an empty object if the workflow drawing task is not part of the active step', async function () { + const project = ProjectFactory.build({}, { activeWorkflowId: manyStepWorkflow.id }) + const subjects = Factory.buildList('subject', 10) + const panoptesClientStub = stubPanoptesJs({ workflows: manyStepWorkflow, subjects }) + const rootStore = await setupStores(panoptesClientStub, project, manyStepWorkflow) + expect(rootStore.workflowSteps.interactionTask).to.be.empty() + }) + + it('should return the active step drawing task', async function () { + const project = ProjectFactory.build({}, { activeWorkflowId: drawingWorkflow.id }) + const subjects = Factory.buildList('subject', 10) + const panoptesClientStub = stubPanoptesJs({ workflows: drawingWorkflow, subjects }) + const rootStore = await setupStores(panoptesClientStub, project, drawingWorkflow) + expect(rootStore.workflowSteps.interactionTask.type).to.equal('drawing') + }) + + it('should return the transcription task', async function () { + const project = ProjectFactory.build({}, { activeWorkflowId: transcriptionWorkflow.id }) + const subjects = Factory.buildList('subject', 10) + const panoptesClientStub = stubPanoptesJs({ workflows: transcriptionWorkflow, subjects }) + const rootStore = await setupStores(panoptesClientStub, project, transcriptionWorkflow) + expect(rootStore.workflowSteps.interactionTask.type).to.equal('transcription') + }) + }) }) diff --git a/packages/lib-classifier/test/factories/index.js b/packages/lib-classifier/test/factories/index.js index a05ccba20b..5712a7cc5f 100644 --- a/packages/lib-classifier/test/factories/index.js +++ b/packages/lib-classifier/test/factories/index.js @@ -17,6 +17,7 @@ export { FieldGuideMediumFactory, TutorialMediumFactory } export { default as DrawingTaskFactory } from './tasks/DrawingTaskFactory' export { default as MultipleChoiceTaskFactory } from './tasks/MultipleChoiceTaskFactory' export { default as SingleChoiceTaskFactory } from './tasks/SingleChoiceTaskFactory' +export { default as TranscriptionTaskFactory } from './tasks/TranscriptionTaskFactory' // Classification Annotation Factories export { default as SingleChoiceAnnotationFactory } from './annotations/SingleChoiceAnnotationFactory' From d4cb6134014916afb7518ac79d9aceb945ad6913 Mon Sep 17 00:00:00 2001 From: srallen Date: Mon, 25 Jan 2021 09:07:04 -0600 Subject: [PATCH 3/4] Update packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js Co-authored-by: Jim O'Donnell --- .../components/PreviousMarks/PreviousMarks.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js index a350b32202..fcfbe22f6e 100644 --- a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js +++ b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js @@ -39,7 +39,7 @@ describe('Component > PreviousMarks', function () { }) describe('when there are interaction task annotations', function () { - it('should render a DrawingToolMarks for each annotation', function () { + it('should render a DrawingToolMarks for each task', function () { const wrapper = shallow() expect(wrapper.find(DrawingToolMarks)).to.have.lengthOf(2) expect(wrapper.find(DrawingToolMarks).first().key()).to.equal(interactionTaskAnnotations[0].task) @@ -83,4 +83,4 @@ describe('Component > PreviousMarks', function () { expect(wrapper.html()).to.be.null() }) }) -}) \ No newline at end of file +}) From 5584bb88bb96c90c267aa85213f63b84393750aa Mon Sep 17 00:00:00 2001 From: Sarah Allen Date: Mon, 25 Jan 2021 09:19:32 -0600 Subject: [PATCH 4/4] Clarify test. Remove unnecessary Object.values and check for length. --- .../PreviousMarks/PreviousMarks.spec.js | 31 +++++++++++-------- .../src/store/WorkflowStepStore.js | 4 +-- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js index fcfbe22f6e..9bed54cdfa 100644 --- a/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js +++ b/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.spec.js @@ -47,21 +47,26 @@ describe('Component > PreviousMarks', function () { }) 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() - let firstAnnotationMarks = wrapper.find(DrawingToolMarks).first().props().marks - let secondAnnotationMarks = wrapper.find(DrawingToolMarks).last().props().marks - expect(firstAnnotationMarks).to.have.lengthOf(1) - expect(firstAnnotationMarks[0]).to.deep.equal(interactionTaskAnnotations[0].value[0]) - expect(secondAnnotationMarks).to.have.lengthOf(2) - expect(secondAnnotationMarks[0]).to.deep.equal(interactionTaskAnnotations[1].value[0]) - expect(secondAnnotationMarks[1]).to.deep.equal(interactionTaskAnnotations[1].value[1]) + 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 }) - firstAnnotationMarks = wrapper.find(DrawingToolMarks).first().props().marks - secondAnnotationMarks = wrapper.find(DrawingToolMarks).last().props().marks - expect(firstAnnotationMarks).to.have.lengthOf(0) - expect(secondAnnotationMarks).to.have.lengthOf(2) - expect(secondAnnotationMarks[0]).to.deep.equal(interactionTaskAnnotations[1].value[2]) - expect(secondAnnotationMarks[1]).to.deep.equal(interactionTaskAnnotations[1].value[3]) + 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) }) }) diff --git a/packages/lib-classifier/src/store/WorkflowStepStore.js b/packages/lib-classifier/src/store/WorkflowStepStore.js index 012a5d0f96..502fc86c48 100644 --- a/packages/lib-classifier/src/store/WorkflowStepStore.js +++ b/packages/lib-classifier/src/store/WorkflowStepStore.js @@ -51,8 +51,8 @@ const WorkflowStepStore = types findTasksByType (type) { const tasksByType = Array.from(self.steps).map(([stepKey, step]) => { - if (step?.tasks) { - return Object.values(step.tasks).filter(task => { + if (step?.tasks && step.tasks.length > 0) { + return step.tasks.filter(task => { return task.type === type }) }