Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hovering Palette While Moving Bendpoint Results in Error #1387

Closed
philippfromme opened this issue Dec 8, 2020 · 2 comments · Fixed by #1419
Closed

Hovering Palette While Moving Bendpoint Results in Error #1387

philippfromme opened this issue Dec 8, 2020 · 2 comments · Fixed by #1419
Labels
bug Something isn't working spring cleaning Could be cleaned up one day

Comments

@philippfromme
Copy link
Contributor

philippfromme commented Dec 8, 2020 •
edited by nikku

Describe the Bug

Hovering the palette while moving a bendpoint results in an error.

08-12-_2020_09-46-36

Steps to Reproduce

  1. Move a bendpoint
  2. Hover the palette

Expected Behavior

No error is thrown.

Environment

  • Library version: 7.5.0

Related to https://sentry.io/share/issue/524ce0be0f384b8ab40fe6c4ab643769/
Depends on bpmn-io/diagram-js#497
@philippfromme philippfromme added the bug Something isn't working label Dec 8, 2020
@nikku nikku added the ready Ready to be worked on label Dec 8, 2020
nikku added a commit to bpmn-io/diagram-js that referenced this issue Dec 11, 2020
@nikku
fix(bendpoints): prevent superfluous preview error
66b7796 
This happens as users hover over the palette or other
non-diagram elements during bendpoint move.

Related to bpmn-io/bpmn-js#1387
@nikku nikku added fixed upstream Requires integration of upstream change and removed ready Ready to be worked on labels Dec 11, 2020 — with bpmn-io-tasks
@nikku nikku self-assigned this Dec 11, 2020
@nikku nikku added in progress Currently worked on ready Ready to be worked on and removed fixed upstream Requires integration of upstream change in progress Currently worked on labels Dec 16, 2020 — with bpmn-io-tasks
@nikku nikku removed their assignment Dec 18, 2020
@nikku nikku added the spring cleaning Could be cleaned up one day label Jan 7, 2021
@nikku nikku added backlog Queued in backlog and removed ready Ready to be worked on labels Jan 7, 2021 — with bpmn-io-tasks
@nikku nikku added ready Ready to be worked on and removed backlog Queued in backlog labels Feb 11, 2021 — with bpmn-io-tasks
@philippfromme philippfromme mentioned this issue Mar 1, 2021
Closed
@philippfromme
Copy link
Contributor Author

philippfromme commented Mar 1, 2021 •
edited

Why does this error occur?

The error occurs when a connection waypoint is being moved without hovering a diagram element. If, for example, the user moves a waypoint and hovers the palette, an out event is fired causing the BendpointMove feature to unset hover, source and target and to set allowed to false. When the preview of the connection is to be drawn allowed (which is called canConnect in the context of drawPreview) having the value of false will cause the ConnectionPreview feature to draw a no-op preview. Note, that this works just fine when creating a new connection or reconnecting an existing one. In the case of moving a waypoint, a no-op preview isn't what we want. Anyway, drawing the no-op preview causes the error since source and target are missing (they were unset on out).

What is the proposed fix?

I'd argue that setting allowed to false on out is not necessary (it is when connecting or reconnecting). It's okay to allow the user to move a connection waypoint regardless of where the mouse cursor is. What would that look like?

fix

I'd say that's exactly what we want. I will create a pull request that implements the proposed fix.

@philippfromme philippfromme mentioned this issue Mar 1, 2021
Merged
philippfromme added a commit to bpmn-io/diagram-js that referenced this issue Mar 1, 2021
@philippfromme
fix(bendpoint-move): always allow moving bendpoint
ada17b0 
Related to bpmn-io/bpmn-js#1387
nikku pushed a commit to bpmn-io/diagram-js that referenced this issue Mar 1, 2021
@philippfromme
fix(bendpoint-move): always allow moving bendpoint (#536)
94a140a 
Related to bpmn-io/bpmn-js#1387
@nikku nikku added fixed upstream Requires integration of upstream change and removed ready Ready to be worked on labels Mar 1, 2021
@nikku
Copy link
Member

nikku commented Mar 1, 2021

Addressed via bpmn-io/diagram-js#536.

nikku added a commit that referenced this issue Mar 3, 2021
@nikku
deps: update to diagram-js@7.2.1
c8f69d2 
Closes #1387
@nikku nikku mentioned this issue Mar 3, 2021
Merged
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed fixed upstream Requires integration of upstream change labels Mar 3, 2021
@fake-join fake-join bot closed this as completed in #1419 Mar 3, 2021
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Mar 3, 2021
fake-join bot pushed a commit that referenced this issue Mar 3, 2021
@nikku @fake-join
deps: update to diagram-js@7.2.1
3b288f1 
Closes #1387
This was referenced Mar 16, 2021
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working spring cleaning Could be cleaned up one day
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

deps: update to diagram-js@7.2.1 bpmn-io/bpmn-js
2 participants
@nikku @philippfromme