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

Label moves twice when using space tool #878

Closed
nikku opened this issue Oct 22, 2018 · 17 comments
Closed

Label moves twice when using space tool #878

nikku opened this issue Oct 22, 2018 · 17 comments
Labels
bug Something isn't working modeling
Milestone

Comments

@nikku
Copy link
Member

nikku commented Oct 22, 2018

Describe the Bug

When using the space tool along with the sequence flow the labels will move twice.

Steps to Reproduce

The following reproduces the bug on demo.bpmn.io.

foo

Expected Behavior

The label is only moved once.

Environment

  • Browser: Any
  • OS: Any
  • Library version: 2.5.2
@nikku nikku added bug Something isn't working modeling backlog Queued in backlog labels Oct 22, 2018
nikku added a commit that referenced this issue Dec 5, 2018
@ghost ghost assigned nikku Dec 5, 2018
@ghost ghost added in progress Currently worked on and removed backlog Queued in backlog labels Dec 5, 2018
@nikku
Copy link
Member Author

nikku commented Dec 5, 2018

The problem underlying this bug is that the LabelBehavior moves the label after the space tool already moved it.

@nikku
Copy link
Member Author

nikku commented Dec 5, 2018

I added a test case that reproduces the issue on the 878-space-moves-label-twice branch.

@philippfromme
Copy link
Contributor

Came across this bug again while working on #1019. The problem is that a couple of behaviors kick in after moving and resizing shapes. In the case of using the space tool we don't want that as we're explicitly moving and resizing elements.

@nikku
Copy link
Member Author

nikku commented Jun 14, 2019

Hints could be a way to to disallow the post-move behavior(s).

@philippfromme
Copy link
Contributor

Was thinking about that. Otherwise, the space tool has to be aware of all those behaviors. 💩

@philippfromme
Copy link
Contributor

@nikku something like this?

this._modeling.moveElements(shapes, delta, null, {
    allowPreExecutionBehavior: true,
    allowPostExecutionBehavior: false
  });

@nikku
Copy link
Member Author

nikku commented Jun 14, 2019

Who performs the pre-execution behavior? Can't we be a bit more explicit?

@philippfromme
Copy link
Contributor

philippfromme commented Jun 14, 2019

Attach support is doing stuff before and afterward. We also have a nice fix for the space tool inside attach support. 💩 It would become obsolete with the new hints. Label support also does stuff before. So we'd actually need to suppress both pre and post behavior. The only thing that would still mess everything up would be label adjustment that moves labels afterward.

@philippfromme
Copy link
Contributor

Of course we can also do

this._modeling.moveElements(shapes, delta, null, {
  attachSupport: false,
  labelSupport: false,
  ...
});

But that would also mean tighter coupling.

@nikku
Copy link
Member Author

nikku commented Jun 14, 2019

So the question is: Why do we sometimes do stuff pre and post and sometimes don't?

If we cannot find a distinguishing hint name based on that, what about this: Add a generic hint that describes the source of the move operation (i.e. source = spaceTool). Lower level components may interpret that hint and do stuff they like.

@nikku
Copy link
Member Author

nikku commented Jun 14, 2019

The whole space tool / attach support / label support thing is a bit of a mess, I know 👀.

@philippfromme
Copy link
Contributor

Yeah, it's quite shaky. Didn't expect to open a can of worms. 🐛

@nikku
Copy link
Member Author

nikku commented Jun 14, 2019

We could lookie lookie into dat thing together some day 👓 🏖️ 🌆.

@philippfromme
Copy link
Contributor

That would be nicey nicey. 🌞

@BelindaBecker
Copy link

Dear all,
is there any update on this?

We are facing this issue when modeling large diagrams and changing the layout afterwards. Especially, when moving elements with the space tool over a larger distance, the labels get lost and hard to recover.

Appreciate any info. Thanks!

@nikku
Copy link
Member Author

nikku commented Nov 13, 2019

Unfortunately we could not follow up on this bug to date.

@nikku nikku added this to the M34 milestone Feb 6, 2020
@nikku
Copy link
Member Author

nikku commented Feb 6, 2020

Closed via #1269

@nikku nikku closed this as completed Feb 6, 2020
@bpmn-io-tasks bpmn-io-tasks bot removed the backlog Queued in backlog label Feb 6, 2020
nikku added a commit that referenced this issue Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working modeling
Development

No branches or pull requests

3 participants