Fix annotator creation mode bugs

[subdavis]

Several issues.

• [minor] Reverted checkCreationIncomplete because I still don't know what it does ([BUG] Changing track type during creation causes weird glitch. #475)
• [important] Fixed issue where, during bbox creation, in editing mode, if you move away a frame then come back, you'd be kicked out of editing mode.
• [important] Fixed issue where newly created bounding boxes didn't go directly to editing mode
• [minor] Fixed issue where, during continuous creation, moving between annotation types (rect, poly, back to rect) caused you to get kicked out of continuous mode.
• [important] fixed text styling issue where all colors changed when track entered editing mode (rather than just the text for a single track)

Noticed mostly during demo recording. Still no fix for continuous head/tail creation

[subdavis]

this line was super weird. I think frameData[frameData.length - 1].selected is always the same as trackFrame.selected given the push on line 130...

[subdavis]

it's not very nice, but it makes a lot more sense to me than what was happening with checkCreationIncomplete.

It is also a patch in a much less critical part of the code. This saves us from having to reason about checkCreationIncomplete. and how it impacts future bugs/features in the future.

[BryonLewis]

Below is just me trying to document and understand what is happening. Given the trade-offs and mistakes I made in the checkCreationIncomplete I like your version better.

So the problem before was that you were set up in editing mode to create a detection. You click on the type to change it but don't refocus the bar to commit the change and then click to create your new detection. The act of clicking will unfocus the select/edit for the type and cause the type to get updated which triggers an update in LayerManager, which would then kick you out of creation/editing mode by creating an empty track.

This changes it so the trackTypechange after you click to change focus is caused 100ms after your first click. So it technically creates an empty detection again on the initial click but your other changes in the PR leave it it edit mode so that when the type changes, the type changes for the empty detection. Your second subsequent click is then the creation click which can be used to create the detection. Or if you choose to hit escape you cancel the creation and since it is empty it will delete it. If you're quick enough you can create the detection in one click if you click and drag before the 100ms, if you're too slow you will be required to do a secondary click. So by delaying the nudge to updating LayerManager we are preventing the previous problem, but potentially requiring two clicks if they have edited the type.

The checkCreationIncomplete was a confusing way to have the editAnnotationLayer itself check if the update was called while we were still awaiting creation. The problem I didn't account for was if the incoming data from editingTracks already had features for the given type (box, poly, line). Meaning if the incoming FrameTrackData for the edit mode already had bounds, or geoJSON features for the type it shouldn't skipNextExternalUpdate.

[subdavis]

It's just a endless wack-a-mole with this bug.

The new bug created here:

1. Create new detection
2. Change detection type without saving
3. Draw detection (And see it immediately disappear)

So we're back to square 1. It resolves the issue where you end up in an invalid state, but it's bad UX still and should be avoided.

I've had a new idea for how to address this. Could you look at 5b934b4 and see if you can think of possible unintended consequences?

On a completely separate note, the type dropdown isn't the only place that could cause this. All of the attribute editing controls will result in the same bug. #620 would make this more obvious if we don't fix it properly.

[subdavis]

Well, I completely failed to consider that drawing could occur on detections other than the very first.

[BryonLewis]

yeah I was just going to ask about featureIndex. I didn't remember it's usage entirely but you really want to see if there are no features for the current frame/type(bounds,poly, line) combo.

[subdavis]

yeah I was just going to ask about featureIndex. I didn't remember it's usage entirely but you really want to see if there are no features for the current frame/type(bounds,poly, line) combo.

I don't think that's quite right either. After the first successful creation, you can edit type and attributes without there being a feature for the current frame. For example, during interpolation (or outside track boundaries) you still want to be able to change type and attributes.

[subdavis]

I can't come up with anything better right now, so my suggestion is to use both.

• setTimeout will prevent the user from entering an invalid state, though it still may cause the new annotation to disappear
• Prevention of broadcast before the first feature will solve 60(??)% of encounters with this bug because users who create a new track/detection will change stuff immediately after creation if they try to change it during annotation drawing.
• All other cases are so rare that you and I have never even noticed them.

3cfec48 is the best my brain can do right now.

[BryonLewis]

I don't think that's quite right either. After the first successful creation, you can edit type and attributes without there being a feature for the current frame. For example, during interpolation (or outside track boundaries) you still want to be able to change type and attributes.

I think what I'm talking about is the instance of creating rects for a track. Keeping it in edit mode and going back to previous existing detections. Switch to poly/line mode. Edit the type but don't commit with a focus change/enter. First click of the new edit type will be eaten like it was for the initial items.

I know this is rare occurance and at this point I'm with agreement about the cases being rare.

[subdavis]

Right, this is still a problem. However, this PR implements a partial fix. the main branch does a slightly worse thing where, during creation of a new detection starting with a polygon instead of a box, at the end, the newly drawn polygon is unmoored and disappears when you've finished drawing it (instead of on next click)

boooo.mp4

I think this bug is less important than getting kicked out of editing mode by frame changes