Classifier: Drawing Task's Active Mark should detect when Active Tool changes

[shaunanoordin]

Classifier Functionality Bug

Package: lib-classifier Affects: Drawing Tasks

Currently, the Drawing Task doesn't detect when the Active Tool changes. In specific conditions, it's possible to be actively editing a Mark X for Tool X, and then switch to Tool Y, thus causing problems as Mark X tries to run the logic for Tool Y.

Context: this was discovered by @ErikOstlund and me while we were debugging the Polygon tool.

• Unlike most other Drawing Tools on the FEM, a Polygon mark can still be "unfinished" even if the user isn't actively interacting with it.
• As a result, it's possible to switch to a different Drawing Tool while a Polygon is actively being edited.

Testing

• Create a workflow with a Drawing Task with a Polygon Tool and a Rectangle Tool.
• Open the workflow in FEM Classifier. (lib-classifier)
• Select the Polygon Tool.
• Make 3 points to create a triangular Polygon mark, but DON'T "close" the Polygon (i.e. don't "finish" the mark)
• While the incomplete Polygon is still the active mark, select the Rectangle Tool.
• Click and drag to place a Rectangle mark
  • Expected: a Rectangle mark is placed.
  • Actual: On pointer up, the triangular Polygon mark is finished and closed.
  • Note: A second attempt to click and drag will correctly place the Rectangle mark.

TODO: add a link to a project already set up for easy reference.

Dev Notes

I think this can be remedied by modifying the Interaction Layer to detect when the active tool changes, since the ILayer is responsible for keeping track of some major top-level drawing state data, e.g. if a mark is actively being created (InteractionLayer.creating = true/false)

I'm probably missing some deets, but here's a rough suggestion:

// In InteractionLayer.js...

function InteractionLayer ({ activeMark, activeTool, ... }) {
  useEffect(
    function onToolChange () {
      console.log('+++ hey, the active tool changed!')
      if (activeMark) { onFinish() }  // Or was that activeMark.finish() ?

      // Maybe un-set the active mark too? I don't know
    }, [activeTool]
  )
  ...
}

Status

Medium priority. It's not breaking any existing projects (AFAIK), but that might change once more FEM projects use more drawing tool types.

[ErikOstlund]

Here's a project with Polygon and Rectangle drawing tools. NOTE: This project, specifically the polygon tool will not be available until #2883 is merged.

Drawing Project

[eatyourgreens]

The transcription line tool will be susceptible to this bug too. We just haven’t noticed because no one’s used more than one tool in a transcription project yet.

[eatyourgreens]

With multiple subject locations, you can also change frame while creating state is true. See #2810.

Maybe we should disable various interactions like changing tool or changing the active frame while creating is true?

[shaunanoordin]

Agreed: Transcription Tasks are affected but difficult to trigger (since there are no other drawing tools to switch to). There's probably still a way to bork these "incomplete until I say otherwise" drawing marks, but I can't think of how a standard volunteer on an existing FEM project could replicate this.

❗ EDIT: crap, me and my big mouth. There IS a way to trigger the "hey my annotations is incomplete but we're switching gears anyway" problem on Transcriptions without changing tools. 1. create a complete transcription line, with text "Hello World". 2. start creating a second transcription line (click to place the first dot, but don't click to place the second dot), and then 3. click on the first (completed) transcription line to select it.

Results: A. second transcription line remains incomplete but you can continue editing/placing new lines, B. BUT you can't submit the Classification/go Next, and C. if you click on the incomplete transcription line it disappears quietly (albeit with console errors)

❓ EDIT EDIT: wait this seems familiar, wasn't there a report from users where they complained they couldn't go to the Next page, and we discovered it was because there was a hidden incomplete transcription line somewhere?

EDIT EDIT EDIT: wait, could it be as simple as modifying the proposed onToolChange() code to trigger when either one of [activeTool, activeMark] changes?

[shaunanoordin]

Maybe we should disable various interactions like changing tool or changing the active frame while creating is true?

I have no major objections to this idea, though I'm still trying to figure out all the interactions

Current PFE behaviour is: (that I know of) (EDIT: current PFE behaviour is a bit WTF)

• If you're in the middle of creating a Polygon Mark and switch Tools, the Polygon will try to complete itself.
• If you're in the middle of creating a Polygon Mark, you can't click on another existing Mark to select it.
• Note: dev classifier's polygon has a SubTask popup so that may skew behaviour a bit
• If you try to move an existing Polygon Mark, ⚠️ it locks up interactions with other existing Marks.
  • WTF?? This isn't happening to Rectangle Marks.
  • Update: okay so this is a bug on PFE

[eatyourgreens]

@shaunanoordin Incomplete Transcription lines should trigger onFinish() here. https://github.com/zooniverse/front-end-monorepo/blob/0b1232ce7371ddc54c07e92f448bd848660957a3/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/DrawingToolMarks/DrawingToolMarks.js#L59-L65

I think onFinish() will validate and delete incomplete marks, where it's the responsibility of each mark model to define 'incomplete'.

[eatyourgreens]

❓ EDIT EDIT: wait this seems familiar, wasn't there a report from users where they complained they couldn't go to the Next page, and we discovered it was because there was a hidden incomplete transcription line somewhere?

Yep, #2810, which I linked above. You could trigger it by clicking once to start a line, then changing the active frame via the image selectors down the side of the subject viewer.