Pressing "R" while in modeling mode enables simulation features

[hkupitz]

Describe the Bug

Pressing "R" on the keyboard when the "Token simulation" mode is disabled turns on simulation capabilities without hiding the palette or disabling the modeling mode.

Clicking the simulation mode toggle button twice resets the erroneous state back to default.

This only seems to be a bug with the Desktop Modeler plugin. Testing with bpmn-js-token-simulation itself or the C8 Web Modeler doesn't show this issue.

Steps to Reproduce

1. Draw a basic process model with at least one element.
2. Make sure that "Token simulation" is turned off.
3. Press "R" on your keyboard.
4. Manipulate the process model while simulation capabilities are enabled.

Expected Behavior

Pressing "R" on the keyboard should do nothing when the simulation mode is disabled.

Environment

• OS: Windows 10
• Desktop Modeler version: 5.5.1
• Plugin version: 0.16.0

[marstamm]

Hi, thanks for opening the issue. I can reproduce the issue.

Pressing R normally resets the token when token simulation is active, but in this case it creates a weird state where it is not active (toggle on top is off), but the token simulation UI is still visible.

I will move this ticket into backlog for now.

[marstamm]

Root cause

• The action is triggered through a global shortcut (accelerator) + editor action instead of a native keyboard event https://github.com/camunda/camunda-modeler-token-simulation-plugin/blob/master/menu.js#L23
• Therefore, no active check is performed when pressing r
• Overlays also get shown when simulation is disabled

[nikku]

@marstamm I'm not sure if I agree on https://github.com/bpmn-io/bpmn-js-token-simulation/commit/9f1dc59a43b91ee6af19d8558fbb37f549f64010 to fix it given the root cause you provided.

Why don't we ensure that the shortcut is only emitted contextually in the app? Feel weird to fix it in the core.

[marstamm]

I think the active/inactive state is inherently an internal token simulation state. An integrating library should be able to use the provided API safely without having to replicate this internal state.

Therefore, I think the correct fix is in the core, specifically in the EditorActions: https://github.com/bpmn-io/bpmn-js-token-simulation/commit/8ebee4445051e88efb76fd8b52068a2c5c2d230c

I have no strong feelings about https://github.com/bpmn-io/bpmn-js-token-simulation/commit/9f1dc59a43b91ee6af19d8558fbb37f549f64010 (overlays), and it is not needed to fix this issue. It was just weird to me that we would not have a check there as well, so I "fixed" it as well. If you think this is misplaced, we can drop it.

[nikku]

I think the active/inactive state is inherently an internal token simulation state. An integrating library should be able to use the provided API safely without having to replicate this internal state.

You could also argue the exact opposite:

• In order for meaningful UI integration to happen token simulation must expose the active state. It should so via a dedicated API. It does already expose dedicated change events (https://github.com/bpmn-io/bpmn-js-token-simulation/blob/master/lib/features/toggle-mode/modeler/ToggleMode.js#L84) for externals to consume and react to.

[nikku]

I agree with you on https://github.com/bpmn-io/bpmn-js-token-simulation/commit/8ebee4445051e88efb76fd8b52068a2c5c2d230c as there is no meaningful way for any external component to drive this.

[marstamm]

Let's go with https://github.com/bpmn-io/bpmn-js-token-simulation/commit/8ebee4445051e88efb76fd8b52068a2c5c2d230c then to prevent the invalid state from occurring