fix: don't reopen dropdownDiv if it was already open

[maribethb]

The basics

• I branched from develop
• My pull request is against develop
• My code follows the style guide
• I ran npm run format and npm run lint

The details

Resolves

Fixes #6037

Proposed Changes

When fields that use the dropdownDiv are already open and then are clicked again, they will close (and not reopen)

Behavior Before Change

1. Click color field
2. Dropdown div opens with color picker
3. Click same color field
4. Dropdown div closes briefly then reopens

Behavior After Change

1. Click color field
2. Dropdown div opens with color picker
3. Click same color field
4. Dropdown div closes

Reason for Changes

Feels more natural and expected, more similar to other UI inputs.

Test Coverage

Working on making unit tests, having trouble simulating a click on a field. Will add if we actually want to do this change.

Documentation

none

Additional Information

I had some free time and wanted to pick up a bug... Sadly this is complicated to do :/

The dropdown is automatically closed whenever a gesture is started (think mousedown). The field doesn't receive the click on itself until the gesture is confirmed to be a click on the field (think mouseup).

So, it's not that the menu "animates a reopen" - from the editor's perspective the menu was already closed and it is just always going to open. What needs to happen is the field needs to know "right before this gesture started, was the dropdown div already opened and owned by me? if so, don't open it again." It's not enough to just know the last owner of the dropdown div. You really need to know who owned it right before the current gesture closed it. Therefore, gesture.ts has to be involved here, and you can't just have the fields ask the dropdownDiv what to do.

Note that with the current approach, this will affect all fields including custom fields that use the dropdown div. This seems fine based on the description of the current behavior as a "bug" but I wanted to note that. Because as an alternative, we could pass in alreadyOpen or whatever as a parameter to showEditor and showEditor_ to let each field determine individually what to do. But that is more complicated and it's most likely that this is the desired behavior.

Also note that for fields that use both widget div and dropdown div, this still works great. For example, the angle field uses both. When you click "the angle field" again you are actually clicking the html text input which eats the events, so the dropdown is never hidden in the first place, and simply remains open, as you would expect.

Alternatives Considered

• Storing the last owner of the dropdownDiv on the dropdownDiv itself. Again, this isn't enough because you need to know who owned it right before the gesture caused it to be closed. Otherwise if the div was already closed from a different gesture, then next time you opened it the last owner is still itself, and we would incorrectly not open the editor.
• Not hiding the chaff on mousedown, and only hiding it...
  • on mouseup: You can't handle it in gesture.handleUp at the beginning, because that'd still be before the field gets it. or at the end, because that would hide it right after the field opens it for real.
  • in every individual gesture type. We already hide it at the start of a lot of other gestures, like when the workspace is dragged, flyout is opened, etc. But if you go this route then you also have to force every single field to hide the chaff itself, even ones that don't have any reason to care. So that isn't ideal and leaves a lot of room for error.

[BeksOmega]

While I don't /love/ these changes (I have problems with the gesture system being a monolith in general). All of these changes are internal, so we can always restructure it later if we want to. So I'm totally cool with merging as is!

[maribethb]

While I don't /love/ these changes (I have problems with the gesture system being a monolith in general). All of these changes are internal, so we can always restructure it later if we want to. So I'm totally cool with merging as is!

I agree with you completely, but at this point I don't know where else it could go without significantly changing the gesture system.