zooniverse / front-end-monorepo

A rebuild of the front-end for zooniverse.org
https://www.zooniverse.org
Apache License 2.0
104 stars 29 forks source link

Transcription Task 'Show/Hide Marks' buttons not working #2595

Closed snblickhan closed 2 years ago

snblickhan commented 2 years ago

FEM

Describe the bug

Transcription Task

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://www.zooniverse.org/projects/humphrydavy/davy-notebooks-project/classify/workflow/20178 (or any other workflow, this is the newest one, so lowest percent complete)
  2. Create at least one new line (even just as a test)
  3. Scroll down to the bottom of the image viewer & select Hide All Marks from dropdown
  4. See error: marks are still here. Some will change color, but they don't disappear as I'd expect them to. NB: same thing happens on Beyond Borders: https://www.zooniverse.org/projects/mainehistory/beyond-borders-transcribing-historic-maine-land-documents

Applicable Panoptes resource IDs (project, workflow, etc) to demonstrate the issue: DNP: project 9006 BB: project 12856

Expected behavior

Hide All Marks should hide all marks on the page. Show All Marks should show all marks. Show Your Marks should show only an individual user's marks.

Device information

Desktop (please complete the following information):

Additional context

Originally reported via Talk: https://www.zooniverse.org/projects/humphrydavy/davy-notebooks-project/talk/2586/2267433?comment=3718869&page=1

eatyourgreens commented 2 years ago

One thought: If we hide the purple lines, should you be able to place new green lines in place of the hidden purple lines? Will that mess up the transcription task, if I ignore the previous transcriptions and just add my own new transcriptions instead?

eatyourgreens commented 2 years ago

After a quick look at the code, it's working as written. I think this is a case of a feature that hasn't been implemented yet, rather than a bug. @snblickhan can you expand on how that toggle is supposed to work for the transcription task?

EDIT: existing code is this line here, which doesn't distinguish between 'show all marks' or 'show user marks'. https://github.com/zooniverse/front-end-monorepo/blob/1aa3937195afddeb689d3e56c554ef8eea7326b3/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/PreviousMarks/PreviousMarks.js#L16

snblickhan commented 2 years ago

One thought: If we hide the purple lines, should you be able to place new green lines in place of the hidden purple lines? Will that mess up the transcription task, if I ignore the previous transcriptions and just add my own new transcriptions instead?

I don't know which lines you mean by "purple lines" -- when we designed this task, we designated green lines, pink lines, grey lines, and dark blue lines. Can you use that color scheme in your description? Otherwise I'm unsure what exactly you're talking about.

In terms of placing new green lines: yes, that's actually the point of this feature. It's there in the event of really bad line placement (i.e. word by word instead of a full line of text). It won't mess up the task, but it will delay retirement slightly as the new lines need to get to consensus. It will be messy on the classify page -- that's why we need the ability to Hide Marks & only show your own. One of my eventual goals is to add some sort of 'ignore' flag or an option to tag a mark as needing deletion, but this is where we are for now.

After a quick look at the code, it's working as written. I think this is a case of a feature that hasn't been implemented yet, rather than a bug. @snblickhan can you expand on how that toggle is supposed to work for the transcription task?

The default setting should be showing all marks. If a user chooses 'User Marks' they should only see the lines they've created (or interacted with** -- not 100% sure on the latter). Show All Marks would then bring back all the marks, regardless of whether the user has made or interacted with them.

eatyourgreens commented 2 years ago

The purple lines are the lines that you can interact with to see transcriptions from other volunteers. I wanted to check that turning those off, and creating your own new, green lines instead, wouldn’t break aggregation or analysis for the transcription task.

When you say “lines they’ve created”, are those the green and blue lines?

From the code, it looks like the grey and pink/purple previous marks could be turned off as a single block, if that’s what you’re asking for.

eatyourgreens commented 2 years ago

Digging a bit further into the code, this line should hide previously transcribed lines except when the toggle is set to All. https://github.com/zooniverse/front-end-monorepo/blob/7af16218b541c313dc7b2cd33f1c15f63ecc8a39/packages/lib-classifier/src/components/Classifier/components/SubjectViewer/components/InteractionLayer/components/TranscribedLines/TranscribedLinesConnector.js#L65

I think the problem is that shownMarks isn’t being observed for changes.

Easy enough to fix but weird that this hasn’t been caught by the tests.

eatyourgreens commented 2 years ago

@snblickhan I've got a fix for this so, time permitting, I'll open a PR later today.