knowledgevis / miqa-annotate

A Medical Imaging Annotation Tool
https://annotate.knowledgevis.com
Apache License 2.0
2 stars 0 forks source link

Confirming Code is Non-Functional #11

Closed davidshq closed 1 year ago

davidshq commented 1 year ago

Hi Anne,

I've been eliminating some of the linter errors that are currently occurring when merging and I think I found some code that isn't doing anything but would like another set of eyes.

I've run MIQA with the code commented out and it seems to run fine.

It's here: https://github.com/knowledgevis/miqa-annotate/blob/961fdc98888cac51c7b39a72eb11b8e77cf25df2/web_client/src/store/index.ts#L829-L851

The switch is based on state.windowLocked.duration which I think is a number but the cases are different types of objects = scan, experiment, or project).

If it is non-functional, is it something we need to fix or can it be removed? I added a console statement before line 835 if (loadAll && state.windowLocked.lock) to see if loadAll and state.windowLocked.lock where ever both returning true and it always seems limited to only one of them being true.

Thanks!

annehaley commented 1 year ago

The purpose of this code is to unlock the window slider values when the user switches to a new frame. If the user clicked "Lock for Scan", then the lock will be released when the user goes to any frame not in the same scan as when they locked it. Same goes for "Lock for Experiment" and "Lock for Project". See the below demonstration, where the window slider becomes unlocked if these conditions are met.

One side note is that the scans in this demo project all have different window ranges, so as I skip from scan to scan, the slider may look like it changes, but the values on either end are still the same. It's simply the range of the whole slider changing. Typically, in the use cases where a reviewer would want to lock for an experiment or project, they would probably have scans with the same or similar ranges.

https://user-images.githubusercontent.com/44912689/231261674-e49ff222-2fff-4e57-8713-23bfdd529221.mp4

davidshq commented 1 year ago

Thanks Anne, that was just what I needed! I tracked the issue down to an incorrect TS type definition and have updated accordingly.