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

feat(preview-support): add copied marker as part of djs-visual #906

Merged
merged 2 commits into from
May 28, 2024

Conversation

marstamm
Copy link
Contributor

@marstamm marstamm commented May 27, 2024

This PR

  • Adds support for non-root def sections in the svg
  • Moves the copied svg markers into the preview visuals. This makes cleanup easier and more consistent.

closes #904
image

Test it out with

npx @bpmn-io/sr bpmn-io/bpmn-js#marker-rework -l bpmn-io/diagram-js#904-marker-rework

downstream draft PR: bpmn-io/bpmn-js#2173

@marstamm marstamm requested a review from a team May 27, 2024 12:16
@marstamm marstamm self-assigned this May 27, 2024
@marstamm marstamm requested review from philippfromme and barmac and removed request for a team May 27, 2024 12:16
@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label May 27, 2024
};
// Markers are cleaned up with visuals, keep stub for compatibility
// cf. https://github.com/camunda/camunda-modeler/issues/4307
PreviewSupport.prototype.cleanUp = function() {};
Copy link
Member

Choose a reason for hiding this comment

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

We could consider to log a warning here that this is no longer being used (and will be removed in future major releases).

Cf. ContextPad.getPad or other instances.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@@ -204,12 +190,12 @@ PreviewSupport.prototype._cloneMarker = function(gfx, marker, markerType, classN

this._clonedMarkers[ markerId ] = clonedMarker;

var defs = domQuery('defs', this._canvas._svg);
var defs = domQuery(':scope > defs', parentGfx);
Copy link
Member

Choose a reason for hiding this comment

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

What is the support for :scope within CSS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everything except IE11

image

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.

Looks good. Just one thing to consider regarding API deprecation I think.

@philippfromme
Copy link
Contributor

I tested it and it looks good to me as well.

@marstamm
Copy link
Contributor Author

Added deprecation warning with 0f12ab6

Copy link
Contributor

@philippfromme philippfromme left a comment

Choose a reason for hiding this comment

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

👏🏻

@marstamm marstamm merged commit 609a5dc into develop May 28, 2024
11 of 12 checks passed
@marstamm marstamm deleted the 904-marker-rework branch May 28, 2024 11:38
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label May 28, 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.

Dragging preview does not show Connection markers
3 participants