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

Improve ID handling #82

Merged
merged 5 commits into from
May 30, 2024
Merged

Improve ID handling #82

merged 5 commits into from
May 30, 2024

Conversation

marstamm
Copy link
Contributor

  • prefix element IDs
  • Add a random string to IDs to allow for multiple diagram-js-minimap instances on the same page
  • prevent elements with IDs from being copied

related to bpmn-io/bpmn-js#2179

@marstamm marstamm requested review from nikku and a team May 30, 2024 09:37
@marstamm marstamm self-assigned this May 30, 2024
@marstamm marstamm requested review from philippfromme and removed request for a team May 30, 2024 09:37
@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label May 30, 2024
@@ -28,9 +29,12 @@ import {

import { getVisual } from 'diagram-js/lib/util/GraphicsUtil';

import IdGenerator from 'diagram-js/lib/util/IdGenerator';
Copy link
Member

Choose a reason for hiding this comment

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

We must declare this as an external dependency in rollup config.

Copy link
Member

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a regex to ensure this will not happen again: 4d19666

Copy link
Member

@nikku nikku left a comment

Choose a reason for hiding this comment

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

I stumbled upon the fact that markers are still there, which is, of course, absolutely normal, because they are referenced from the diagram. This means folks don't even see the difference, visually speaking 💡.

image

@nikku nikku merged commit 85d4491 into main May 30, 2024
8 checks passed
@nikku nikku deleted the remove-ids branch May 30, 2024 12:35
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label May 30, 2024
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