bpmn-io / diagram-js

A toolbox for displaying and modifying diagrams on the web.
MIT License
1.69k stars 419 forks source link

Popup-menu closes when refresh is called during initial render #877

Open marstamm opened 6 months ago

marstamm commented 6 months ago

Describe the Bug

In this Component, the popup menu will close directly when the promise is resolved instantly and the timeout is removed https://github.com/bpmn-io/refactorings/blob/main/lib/popup-menu/RefactoringsPopupMenuProvider.js#L30-L32

Steps to Reproduce

Steps to reproduce the behavior:

  1. do this
  2. do that

If possible, try to build a test case that reproduces your problem.

Expected Behavior

A clear and concise description of what you expected to happen.

Environment

Please complete the following information:

marstamm commented 6 months ago

Root Cause We open the Menu using a click event, that originates outside of the Popup menu.

We register event listeners on the body, to check whether and outside click occurred, in an useEffect hook. Per default, the effect execution is delayed until after the browser renders the next frame.

Because of how Promises are scheduled when they are immediately resolved we call popupMenu.refresh() before the original click event bubbles to the body. This forces a re-render of the Preact component, which forces execution of the delayed effects. The event listeners are created on the body, while the original click event is still pending.

When it bubbles up to the body, the check fails and closes the menu directly.

How to fix it We discussed possible solutions in the Hour of Code. We agree that the use-case is valid and this is a bug in the core.

However, we were unable to find an elegant solution. The solution sketch we came up with is as follows:

Conclusion As we have a workaround in refactorings, this is an edge-case and the solution is not elegant, I will move this to the backlog for now. If you want to experiment with it, I added the test-case I used to showcase it in the HOC here: https://github.com/bpmn-io/diagram-js/tree/877-experiement-popup-menu-event-handler