Skip to content

Conversation

abdul99ahad
Copy link
Contributor

@abdul99ahad abdul99ahad commented Sep 30, 2024

Closes #2239

Proposed Changes

Text annotation can now co exist with flow element label

Checklist

To ensure you provided everything we need to look at your PR:

  • Brief textual description of the changes present
  • Visual demo attached
  • Steps to try out present, i.e. using the @bpmn-io/sr tool
  • Related issue linked via Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}

Requires upstream: bpmn-io/diagram-js#940

@nikku nikku added the in progress Currently worked on label Oct 2, 2024 — with bpmn-io-tasks
@nikku nikku removed the needs review Review pending label Oct 2, 2024
@nikku nikku marked this pull request as draft October 6, 2024 18:08
@abdul99ahad abdul99ahad force-pushed the 2239-text-annotation-alignment branch from bcb9dcf to 53ed469 Compare October 14, 2024 17:06
@abdul99ahad abdul99ahad added the fixed upstream Requires integration of upstream change label Oct 14, 2024
@abdul99ahad abdul99ahad force-pushed the 2239-text-annotation-alignment branch from f6160b2 to 56fda2e Compare October 14, 2024 17:10
@abdul99ahad abdul99ahad marked this pull request as ready for review October 14, 2024 17:10
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on fixed upstream Requires integration of upstream change labels Oct 14, 2024
@abdul99ahad abdul99ahad force-pushed the 2239-text-annotation-alignment branch from 56fda2e to 87b4f7c Compare October 16, 2024 09:42
@nikku
Copy link
Member

nikku commented Oct 16, 2024

Seems like tests fail on MacOS. This happens because elements have slightly different bounds across different OS; usually fixed by allocating additional space in the test case.

Can you have a look @abdul99ahad?

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.

Cf. previous comment. Test currently fails on MacOS.

@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Oct 16, 2024
@abdul99ahad abdul99ahad force-pushed the 2239-text-annotation-alignment branch from 87b4f7c to 005f09f Compare October 21, 2024 10:45
@abdul99ahad abdul99ahad requested a review from nikku October 21, 2024 11:16
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Oct 21, 2024
@abdul99ahad
Copy link
Contributor Author

Seems like tests fail on MacOS. This happens because elements have slightly different bounds across different OS; usually fixed by allocating additional space in the test case.

Can you have a look @abdul99ahad?

Updated. Test is now passing on all platforms. Ready for review.

@nikku nikku self-requested a review October 30, 2024 13:30
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.

Good stuff, thanks for following up!

@nikku nikku merged commit e5f1637 into develop Oct 30, 2024
9 checks passed
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Oct 30, 2024
@nikku nikku deleted the 2239-text-annotation-alignment branch October 30, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Aligning Text Annotation with a Sequence flow with name, creates unhandled error in event listener

2 participants