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

Handle Adding Vertical Lanes #2090

Merged
merged 3 commits into from
Jan 30, 2024
Merged

Conversation

sombrek
Copy link
Contributor

@sombrek sombrek commented Jan 27, 2024

Closes #2086

  • I fixed come CRLF line endings in a separate commit.
  • I did not add tests for undefined lanes, since special handling for such lanes already exists and is tested (IsHorizontalFixSpec).

@nikku
Copy link
Member

nikku commented Jan 29, 2024

As we're adding more features in the context of vertical lanes: I think it makes sense to add to dedicated "vertical" examples to our modeler and viewer specs. These are the ones we usually use for end-to-end (integration) testing.

@nikku nikku requested review from marstamm and barmac January 29, 2024 11:12
@barmac
Copy link
Member

barmac commented Jan 30, 2024

Thank you for your contribution.

I've added a vertical collaboration example via 1cf81d2

image

In this PR, the nested lane is still not handled correctly:

Screen.Recording.2024-01-30.at.10.20.35.mov

We may consider to flip the icons in the future, but it's not required to merge this.

@barmac
Copy link
Member

barmac commented Jan 30, 2024

As a follow-up, I created an issue for the hit box: #2093

@sombrek
Copy link
Contributor Author

sombrek commented Jan 30, 2024

@barmac Thanks for the additional test and the follow-up.

In this PR, the nested lane is still not handled correctly:

Honestly, until now I didn't think of splitting lanes as adding nested ones, so I didn't work on splitting yet. But I see your point now. Would you like me to include splitting in this PR (may take two weeks) or can I do it in a separate one?

@barmac
Copy link
Member

barmac commented Jan 30, 2024

It's OK to have it as a separate PR then. I will merge this.

@barmac
Copy link
Member

barmac commented Jan 30, 2024

I checked the issue again. You mentioned splitting there: #2086 (comment) So I won't close it but rather keep a link in this PR.

@barmac barmac merged commit 64002bc into bpmn-io:develop Jan 30, 2024
9 checks passed
@barmac
Copy link
Member

barmac commented Jan 30, 2024

Released via #2094

@sombrek sombrek deleted the add-vertical-lanes branch January 17, 2025 12:28
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.

Handle Adding Vertical Lanes
3 participants