fix(context-pad): only update visibility once per frame

marstamm commented 1 month ago

Significantly improves deselection performance

nikku commented 3 weeks ago

I've added my cents via https://github.com/bpmn-io/diagram-js/pull/915/commits/e3d1ebc3bec72d157d5521d31792daba7dcfa359, specifically the following:

Make it a generally re-usable service.
Allow integrations to sensibly handle return values and errors. Don't swallow errors!
Test without asserting internals

On a related note you could argue that the actual bug is a broken ("not performant") Canvas#hasMarker method. As a user you're not supposed to fiddle with the CSS of the element anyway, so markers (tested via hasMarker) could be cached, too.

marstamm commented 2 weeks ago

I think it is both. Canvas#hasMarker should not query the DOM and the context pad should not calculate visibility multiple times in one frame.

I added the changes to hasMarker to decouple it from the DOM in 6cefa79

nikku commented 2 weeks ago

Great! Spun off marker check to https://github.com/camunda/camunda-modeler/issues/4335.

nikku commented 2 weeks ago

Cleaned up history, split both feature (new scheduler) + fix (update once per frame).

bpmn-io / diagram-js

fix(context-pad): only update visibility once per frame #915