Multi-selection in viewer causes black boxes

[nikku]

Describe the Bug

As a user I'm embedding the viewer as recommended in the starter project. When I select elements no outline is shown, I can still react to selected events; this I expect.

However when I select more than one element a black box appears that hides the diagram:

Steps to Reproduce

1. open starter viewer example
2. select more than one element using SHIFT + click
3. see black box appear

Expected Behavior

• No black box appears
• (Alternative) Selection is not available for the viewer

Environment

• Browser: Chrome 110
• OS: Linux
• Library version: v14.0.0

[philippfromme]

I think that we need to be clear on what a viewer is. The default viewer in my opinion, should behave like viewing an SVG. No interactivity at all. I never liked the fact that you get selection behavior in the viewer. Now, there are use cases that need an interactive viewer. In these cases, users need to include the required features and stylesheets as well. Alternatively, we could offer something similar to the navigated viewer.

[philippfromme]

There would still remain the problem of interactive viewer and no selection outlines for connections. In the modeler selection is indicated by showing the bendpoints.

In an interactive viewer selecting connections should be indicated. We should have a solution for that.

[nikku]

No interactivity means that you cannot properly react to states in i.e. overlays. Hence the viewer would always offer basic interactivity, including hover and selection.

[philippfromme]

Overlays is a feature of an interactive viewer, I'd say.

[barmac]

I'd say there are 4 ways to display diagram:

• Modeler
• NavigatedViewer
• Viewer with some interactivity
• Displaying result of Viewer.saveSVG

Otherwise we should rename the Viewer to Renderer, as that's what it is without any interactivity.

[nikku]

Thanks for #2135 (comment) @barmac. I subscribe to it. Without interactivity the viewer is simply a renderer.

You can roll your own barebones renderer if you so desire on top of the BaseViewer package.

[misiekhardcore]

In the modeler starter example it works fine, it does not work locally in viewer starter example with same issue as in the linked example. I can see that the issue occurs cause there are no diagram-js styles added.
The issue here is in general indicating anything being selected, even a single element - is this expected that we don't want to outline a single selection in Viewer?

[philippfromme]

The selection and outline features are part of the both the viewer and the modeler. Unless we decide to remove it from the viewer I'd say you simply must include diagram-js.css for it to work properly.

But even then there are a couple of issues:

• connection outline not shown (we decided to not show connection outlines anymore since the bendpoints feature indicates selection, we could enable it in the viewer)
• selecting multiple elements always selects text (at least in the example)

[misiekhardcore]

After adding the missing diagram-js.css the selection visuals seem to be fixed, the issue with selecting text persists but maybe it should be handled in a separate issue?

Screencast.from.2024-10-29.15-43-07.webm

[philippfromme]

After discussing this with @nikku our proposal is:

• move multi-selection outline from SelectionVisuals to Outline feature
• remove outline feature from viewer

As a result you can use the viewer without any additional CSS file, the selection feature works but no outline will be shown by default, you can add it by adding the outline feature or applying custom CSS using the .djs-element.selected selector.

[misiekhardcore]

Won't this require the Outline feature to now be aware of selection and canvas? Or I misunderstand what's need to be done?

[nikku]

@misiekhardcore The purpose of the outline feature is to render an outline (around elements), to be styled based on selection and other state. It is an optional feature in the toolbox.

Selection on the other hand is a more core feature of the toolbox, and hence we want to keep it as minimal as possible. The way components plug into selection is through events, so we already loosely couple things (nothing breaks if selection ends up not being there, but it is, if you want to make anything selectable):

function MyComponent(eventBus) {
  
  eventBus.on('selection.changed', function({ newSelection, oldSelection }) {
    ...
  });
}

We could consider to not move the SelectionOutline into the outline feature, but make it a dedicated concern (standalone feature), as previously sketched here. However, for me it conceptional fits into the "outline" bucket.

What I propose is to add a MultiSelectionOutline component within the outline feature, rendering the (multi-selection) outline.

---

What is also useful in terms of design is to clearly separate UI and non-UI components. As part of bpmn-js-headless we investigated a way to make core bpmn-js available for headless use. This works as long as only non-UI components ("the business logic") is used, but breaks if components wildly mix UI and non-UI concerns. In this particular case, using the actual selection (to do whatever, i.e. trigger an editor action afterwards) will blow up the headless tool, unless explicitly detangled. Another reason to make this change.

I don't necessarily agree with bpmn-io/diagram-js@5db35ec, "just moving the selection visuals out", as this will also make the markers unavailable. Hence I propose moving the outline (the only UI aspect) to an actual UI component, the outline feature.

[misiekhardcore]

Okay, that brings some more light into the problem we want to fix. I personally like the idea of having a separate MultiSelectOutline feature to have a better granularity and allowing the user to decide what they need

[misiekhardcore]

So, I opened a PR, but should I open an issue in diagram-js? or opening a PR and relating it to this one is the correct way?
Also I do not really know how to test it out, how can I play with the changes I made?

[nikku]

You can link the PR in your comment, to establish the context. No other issue is needed, if the PR is self explainatory in what we want to accomplish, and why. It should provide a clear rationale, along the lines of this.

[misiekhardcore]

I have decided to create a separate issue for moving multi-select outlines to a separate module as this won't fix the issue described here.
The core problem here is that svg rect gets the black fill by default and as long as we include the diagram-js css file, it works fine, because it has styles defined for the multi-select which are setting fill: none (here).
bpmn-js has no default styling for this

How should we handle this case? Should these styles be added to bpmn-js css file?
Technically if user does not pull in the Outline feature, it will all work now, and if they did add the Outline feature, they should include the diagram-js styles right?

[nikku]

Technically if user does not pull in the Outline feature, it will all work now, and if they did add the Outline feature, they should include the diagram-js styles right?

That is the resolution, and we'll ship the viewer without the outline. As a result the viewer will not depend on the external style sheet, which always has been our intention. Or do I miss anything?

If I understood correctly, then #2253 and bpmn-io/diagram-js#943 solve this issue, which was our intention.

[misiekhardcore]

If I understood correctly, then #2253 and bpmn-io/diagram-js#943 solve this issue, which was our intention.

Yes, exactly

[nikku]

Ok, moved this issue to fixed upstream. Let's close it once the linked PRs / issues are merged.