Time Series Refactor

[jblake42]

Ongoing work to refactor time series base component and its relevant context provider. The main goal of this is performance improvements through better stateful behaviour and to simplify the state flow to align more closely to the annotorious approach for the video annotation view.

[samueljackson92]

Hi @jblake42, just taking a quick look at this. On a quick scan the code looks good, However, I have a few issues:

• I can no longer use right click to create an annotation. It seems that preventDefault is no longer called, and I just get the system toolbar.
• I am not sure that buttons are the right thing for right toolbar? It get's very long for me:

• On OSX pressing Ctrl+Left Click also seems to open the system right click context menu as well as starting a new annotation.

• I think drawing annotations by hand should maybe start when we do a Ctrl+left mouse click and finish when the user releases Ctrl or the Mouse button?

• Trying to save annotations and I get:

• After selecting a region with the selection tool, the selection area seems to hand around, even after I try and cancel it or draw another selection.

[jblake42]

@samueljackson92 - I will create a comment for each of the issues raised as I work on them so I can keep track.

Context Menu to Add Annotations

For your initial point regarding the right-click to add. I can add this if you think it is required, my thinking behind leaving it out it is that initially it was included as we did not have the funcitonality to draw directly on to the plot which we now have. I always found the context menu approach clunky and contains logic which cluttered the code base a little.

The approaches I see are:

• Include it as before providing the user with two ways of adding annotations (I personally think having these two distinct ways of annotation could be confusing)
• Remove this functionality but still block the default context menu so that it is handled more gracefully on windows and OSX

I'm happy with either approach, it will not be much work to add it back in, I have just always had it in my head we wanted to move away from the right-click approach.

[jblake42]

OSX Behaviour and Draw Behaviour

@samueljackson92 - apologies, I forgot about the OSX mouse behaviour. This will be corrected with whichever approach we go with above.

The ctrl+left click and drag has been the intended behaviour and is what happens on Windows. If this is not what you are seeing it may be a OSX issue that I need to address - it may also be solved with the changes to prevent default context menu behaviour

[samueljackson92]

Context menu annotations - I am happy to leave behind the right-click add.

The ctrl+left click and drag has been the intended behaviour and is what happens on Windows. If this is not what you are seeing it may be a OSX issue that I need to address - it may also be solved with the changes to prevent default context menu behaviour

I can do a ctrl+left click to create an annotation but:

1. OSX opens the system context menu as well. This is easily fixed.
2. I can release ctrl and left click and continue to "create" the annotation.
3. I must press ctrl again to finish creating an annotation.

I think we should have something more like:

1. ctrl+left click to create
2. User can release left click but continues to hold ctrl
3. User releases ctrl to finish creating annotation.

[jblake42]

Toolbar Structure

Fully agree with your comments about the toolbar - I was in the process of moving to this approach:

Whilst this needs some styling to make it clearer to the user, I think having a drop down for each tool to select the label works a lot better

[samueljackson92]

Toolbar Structure

I agree, something like that is better.

[jblake42]

Failed to save

@samueljackson92 - the failure to save is caused due to a disparity between the annotation type that FastAPI expects and the type defined in the front end. Specifically this is the timestamp and shot id parameters. Other views work as type errors are ignored. We should align the expected type, I don't know which is the correct one though - shall I go with what is defined in FastAPI (no timestamp or shot ID?)

[samueljackson92]

@jblake42 definitions are in this file:

toktagger/toktagger/api/schemas/annotations.py

Line 1 in 7e213fb

from typing import Literal, Optional, Union

Although all annotations coming out of the API should have a timestamp and a shot id.

[samueljackson92]

Looking good. Now working pretty much as expected (although I had to remind myself it's the ctrl button i need to press!).

Right click now seems to be working correctly!

Maybe we also need a keyboard shortcut to toggle "edit mode"? If there isn't one already?

A few small problems spotted in review:

UI Tools

One small problem is that the Select Label option jumps around when changing between time point/time region. I think maybe the buttons should be on the same level (i.e. horizontal) and the select label is always in the same place?

Panning Bug

Another small problem is that when I

1. Pan the plot
2. Click the rescale button in the plotly mode bar

It does not reset the zoom of the graph, which feels incorrect. I guess this might be a side effect of your refactoring efforts.

Batch Change Label Bug

If I use the box selection tool I can select a subset of annotations -> right click -> delete them. However, I should also be able do the same for setting the type, but this only seems to work for the single annotation I right clicked on.