Sub process: visible label(s) after changing to collapsed sub process

[mnp-mid]

Visible label(s) after changing to collapsed sub process with start, task and end event inside.
The label of the start event is even moving far more outside.

This issue occures only by clicking the steps through. Import and Export of the diagram works proberly.
It is tricky to reproduce. See the uploaded gif animation for detailed steps.

Steps to Reproduce

1. append task and end event
2. change task to sub process (expanded)
3. append task and end event in the sub process
4. Add labels to start, task and end event
5. Click on collapsed sub process

Hint: Its not quite clear if the start event inside the sub process may or may not be moved in order to reproduce the error.

Expected Behavior

Hide the label inside the sub process.

Environment

• Browser: [Chrome 69 103.0.5060.66 (Offizieller Build) (64-Bit), Edge 103.0.1264.49 (Offizielles Build) (64-Bit)]
• OS: [e.g. Windows 10]

[pinussilvestrus]

Thanks for the detailed explanation, I can reproduce this 👍 Moving this to backlog

[philippfromme]

Root Cause Analysis 🔍

This was assumed to have been fixed (cf. #1617). However, the fix only works if you don't move the label, so effectively, it's still broken.

Here's what's happening:

• during move the hovered element is 🐒 patched to be the root element when moving a label (cf. https://github.com/bpmn-io/bpmn-js/blob/v9.3.2/lib/features/modeling/behavior/FixHoverBehavior.js#L45) so the label always ends up becoming a child of the root element
• when collapsing the sub process the label stays visible as it's not a child of the sub process diagram shape (cf. https://github.com/bpmn-io/diagram-js/blob/v8.8.0/lib/features/modeling/cmd/ToggleShapeCollapseHandler.js#L31)

However, not 🐒 patching still doesn't fix the issue since the parent will still end up being the root element (cf. https://github.com/bpmn-io/bpmn-js/blob/v9.3.2/lib/features/ordering/BpmnOrderingProvider.js#L134). So we're basically trying to fix the parent of the label in 2️⃣ places. 🤡

I see two possible approaches:

1. Labels are always children of the parents of their label target

• this would require us to not 🐒 patch the hovered element anymore and to default to the label targets parent when computing the order

2. Move labels to appropriate root element when collapsing a shape

• this would require us to add a behavior

[philippfromme]

I will go for the second approach as it is less intrusive.

[philippfromme]

Fixed by #1719.