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.

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

Make hosts sticky for valid attachments #368

Merged
merged 1 commit into from
Jun 20, 2019

Conversation

barmac
Copy link
Member

@barmac barmac commented Jun 19, 2019

The (potential) host's hit shape will now grow so that it gives a stickiness effect. When merged, this will fix bpmn-io/bpmn-js#1078.

assets/diagram-js.css Outdated Show resolved Hide resolved
assets/diagram-js.css Outdated Show resolved Hide resolved
@barmac barmac force-pushed the 1078-attached-boundary-events-should-be-sticky branch 2 times, most recently from 0c67f73 to 242f23d Compare June 19, 2019 14:42
@barmac barmac changed the title Add extended hit shape class Make hosts sticky for valid attachments Jun 19, 2019
@barmac
Copy link
Member Author

barmac commented Jun 19, 2019

@nikku I rewrote this PR so that it actually fixes the bpmn-js problem within diagram-js. Please feel free to leave your comments.

@barmac barmac requested a review from nikku June 19, 2019 14:46
This implements sticky attachers functionality.
@nikku nikku force-pushed the 1078-attached-boundary-events-should-be-sticky branch from 242f23d to aec89d7 Compare June 20, 2019 19:20
var shape = shapes[0],
hostGfx;

if (shape.host) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We offer the method Canvas#addMarker and Canvas#removeMarker to add temporary classes to an element. All related logic is also contained in both, Move and MovePreview.

From what it looked like the only thing that is missing is to reuse the logic is setting a valid hover target when starting the move operation. I've sketched that idea here.

Works fine in diagram-js, makes bpmn-js fail with two random test cases 😢.

I've spent way too much time today trying to dig into the root cause of these failures.

Let's keep this as is for now and see if we can improve things later!

@nikku nikku merged commit 31e49ff into master Jun 20, 2019
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Jun 20, 2019
@delete-merged-branch delete-merged-branch bot deleted the 1078-attached-boundary-events-should-be-sticky branch June 20, 2019 19:22
@nikku
Copy link
Member

nikku commented Jun 20, 2019

Updated with minor changes, cf. aec89d7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attached boundary events should be sticky
2 participants