Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(classifier): automatically sync drawing annotations #5923

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

eatyourgreens
Copy link
Contributor

@eatyourgreens eatyourgreens commented Feb 15, 2024

  • Add an annotation.actualTask view to drawing tasks and transcription tasks.
  • Add an autorun reaction which observes self.actualTask.marks for changes, and updates the annotation value.
  • Remove the manual annotation.update(newValue) when a new mark is drawn.
  • Rewrite annotation.toSnapshot() to use the annotation value.
  • Add multiple frames to the tests.

This might be a more robust fix for problems where the classification is not updated properly after adding or removing a mark. It should keep drawing annotations in sync with their corresponding drawing tools. Whenever a drawing task's marks change, a MobX reaction updates the task's annotation.

Current behaviour, in production, is that adding a mark silently removes existing marks from the classification, except for the active frame. As a consequence, drawn marks appear to vanish from all frames in the subject viewer, except the active frame. See #5929.

Package

  • lib-classifier

Linked Issue and/or Talk Post

How to Review

Similar to #5919, you should be able to draw marks on any frame of a subject, and those marks will still be recorded in the classification and shown on top of the subject in subsequent steps of a workflow.

For How Did We Get Here, you should be able to draw on both frames of a subject without apparently losing drawn marks in the second step of the workflow.
https://local.zooniverse.org:8080/?project=communitiesandcrowds/how-did-we-get-here&env=production

Checklist

PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.

General

  • Tests are passing locally and on Github
  • Documentation is up to date and changelog has been updated if appropriate
  • You can yarn panic && yarn bootstrap or docker-compose up --build and FEM works as expected
  • FEM works in all major desktop browsers: Firefox, Chrome, Edge, Safari (Use Browserstack account as needed)
  • FEM works in a mobile browser

General UX

Example Staging Project: i-fancy-cats

  • All pages of a FEM project load: Home Page, Classify Page, and About Pages
  • Can submit a classification
  • Can sign-in and sign-out
  • The component is accessible

Refactoring

  • The PR creator has described the reason for refactoring
  • The refactored component(s) continue to work as expected

@coveralls
Copy link

coveralls commented Feb 15, 2024

Coverage Status

coverage: 80.361% (-0.004%) from 80.365%
when pulling b358513 on eatyourgreens:refactor-drawing-annotations
into d18c20b on zooniverse:master.

@eatyourgreens eatyourgreens mentioned this pull request Feb 16, 2024
@eatyourgreens eatyourgreens force-pushed the refactor-drawing-annotations branch 6 times, most recently from 34b0ba1 to 3df50f8 Compare February 18, 2024 18:58
@kieftrav kieftrav self-requested a review February 20, 2024 18:15
@eatyourgreens eatyourgreens force-pushed the refactor-drawing-annotations branch 4 times, most recently from cf4acb3 to 695697d Compare March 15, 2024 17:44
@eatyourgreens eatyourgreens force-pushed the refactor-drawing-annotations branch 2 times, most recently from 4cb512f to 47569e6 Compare March 24, 2024 09:05
@eatyourgreens
Copy link
Contributor Author

Bumping this so it doesn’t go stale.

@eatyourgreens eatyourgreens force-pushed the refactor-drawing-annotations branch 2 times, most recently from e140319 to 2e07c8a Compare March 28, 2024 14:22
@eatyourgreens eatyourgreens force-pushed the refactor-drawing-annotations branch 3 times, most recently from 44b6f33 to 8d730de Compare April 11, 2024 17:17
@eatyourgreens eatyourgreens force-pushed the refactor-drawing-annotations branch 3 times, most recently from 9909c98 to b219b6e Compare May 8, 2024 08:23
@eatyourgreens eatyourgreens changed the title refactor: automatically sync drawing annotations fix(classifier): automatically sync drawing annotations May 23, 2024
- Add an `annotation.actualTask` view to drawing tasks and transcription tasks.
- Add an `autorun` listener which observes `self.actualTask.marks`  for changes, and updates the annotation value.
- Remove the manual `annotation.update(newValue)` when a new mark is drawn.

This might be a more robust fix for problems where the classification is not updated properly after a change to a drawing tool. It should keep drawing annotations in sync with their corresponding drawing tools.
Use `annotation.value` to generate mark snapshots for `annotation.toSnapshot()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiframe subjects: Drawing tools only render previous marks for the active frame
2 participants