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

Add switch between DataStore and DataObject in replace menu #1382

Merged
merged 1 commit into from
Dec 3, 2020

Conversation

azeghers
Copy link
Contributor

@azeghers azeghers commented Dec 2, 2020

Which issue does this PR address?

Closes #1372

Acceptance Criteria

  • Corresponds to the concept
  • Corresponds to the design

Definition of Done

@azeghers azeghers requested review from a team, pinussilvestrus and MaxTru and removed request for a team December 2, 2020 11:34
@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Dec 2, 2020
@azeghers azeghers force-pushed the 1372-replace-data-object-with-data-store branch from d93b2f9 to c7b32b4 Compare December 2, 2020 11:38
@pinussilvestrus
Copy link
Contributor

Are the package-lock.json changes intended?

Comment on lines +256 to +264
// The DataStoreReference element is 14px wider than the DataObjectReference element
// This ensures that they stay centered on the x axis when replaced
if (
newElement.type === 'bpmn:DataStoreReference' ||
newElement.type === 'bpmn:DataObjectReference'
) {
newElement.x = element.x + (element.width - newElement.width) / 2;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@azeghers
Copy link
Contributor Author

azeghers commented Dec 2, 2020

Are the package-lock.json changes intended?

They aren't, but that's what happens when I npm install

Do you have insights on this?

@MaxTru
Copy link
Contributor

MaxTru commented Dec 2, 2020

Are the package-lock.json changes intended?

They aren't, but that's what happens when I npm install

Do you have insights on this?

See https://stackoverflow.com/questions/64813775/is-there-any-way-to-fix-package-lock-json-lockfileversion-so-npm-uses-a-specific.
Apparently @azeghers uses a different npm version that what was used for bpmn-js initially. (Alexis, didnt we see did already when we looked at another use-case?). Therefore the package-lock.json is recreated with a new lockfileVersion.

Therefore I would not include the package-lock.json in this case.

@nikku
Copy link
Member

nikku commented Dec 2, 2020

Please use npm@6 if possible. Given that version no changes in the package-lock should happen on npm install.

It also makes sense to stick to Node@14, as it is the latest lts.

@azeghers Can you verify this?

@azeghers azeghers force-pushed the 1372-replace-data-object-with-data-store branch from c7b32b4 to 660348a Compare December 3, 2020 10:37
@azeghers
Copy link
Contributor Author

azeghers commented Dec 3, 2020

I fought a bit with nvm but I should be on the correct npm version now.
Feel free to re-review!

@fake-join fake-join bot merged commit 1ccba5c into develop Dec 3, 2020
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Dec 3, 2020
@fake-join fake-join bot deleted the 1372-replace-data-object-with-data-store branch December 3, 2020 11:01
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.

Add DataStoreReference to DataObjectReference replace menu and vice-versa
4 participants