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

Skip label moving if label is not yet created. #1227

Merged
merged 1 commit into from Oct 25, 2019

Conversation

oguzeroglu
Copy link
Contributor

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Oct 23, 2019
@oguzeroglu oguzeroglu changed the title Skip label moving if label is not yet created. [WIP] Skip label moving if label is not yet created. Oct 23, 2019
@oguzeroglu oguzeroglu requested review from nikku and removed request for philippfromme October 23, 2019 08:29
@oguzeroglu oguzeroglu force-pushed the flow-with-label-copy-paste-crash-fix branch from a7c80a6 to 395488a Compare October 23, 2019 09:39
@oguzeroglu oguzeroglu changed the title [WIP] Skip label moving if label is not yet created. Skip label moving if label is not yet created. Oct 23, 2019
@nikku
Copy link
Member

nikku commented Oct 25, 2019

Thanks for updating this PR (adding an integration test).

I did a small amount of minor updates on top that I'd like you to inspect (cf. b98dbca):

  • I documented the !label.parent check as it can easily look like a bad coding practice otherwise
  • I've pulled out that specific test (+ dependency to grid snapping) into its own test suite, using its own diagram
  • I've simplified the test case, asserting an absolute, expected position. Otherwise it's can be hard to assess what's actually being verified.

@nikku nikku force-pushed the flow-with-label-copy-paste-crash-fix branch from 395488a to 29b29b9 Compare October 25, 2019 13:08
@fake-join fake-join bot merged commit 64d3605 into master Oct 25, 2019
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Oct 25, 2019
@fake-join fake-join bot deleted the flow-with-label-copy-paste-crash-fix branch October 25, 2019 13:16
nikku added a commit to camunda/camunda-modeler that referenced this pull request Oct 25, 2019
Corrects an editor failure when pasting labels.

Cf. bpmn-io/bpmn-js#1227
merge-me bot pushed a commit to camunda/camunda-modeler that referenced this pull request Oct 25, 2019
Corrects an editor failure when pasting labels.

Cf. bpmn-io/bpmn-js#1227
@nikku nikku mentioned this pull request Oct 28, 2019
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants