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.

Sign up for GitHub

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

Jump to bottom

[FEAT] Render vertical lanes #573

Merged
merged 10 commits into from
Sep 2, 2020
Merged

[FEAT] Render vertical lanes #573

merged 10 commits into from
Sep 2, 2020

Conversation

csouchet
Copy link
Member

@csouchet csouchet commented Sep 2, 2020

Closes #127

Expected

image

Result

image

@csouchet csouchet added enhancement New feature or request BPMN rendering Something about the way the lib is rendering BPMN elements labels Sep 2, 2020
@csouchet csouchet force-pushed the 127-Render_vertical_lanes branch from e665491 to 02dc94a Compare September 2, 2020 14:15
@csouchet csouchet mentioned this pull request Sep 2, 2020
Merged
tbouffard
tbouffard reviewed Sep 2, 2020
View reviewed changes
Copy link
Member

@tbouffard tbouffard left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@csouchet for the non regression test, I suggested in https://github.com/process-analytics/bpmn-visualization-js/pull/561/files/41881bb6feeb1405d9d6157190542fa52ae41d1b#diff-522289dbc2f12ccea3f9476c4890e406 (collapsed comment) to update the existing pools-02-vertical bpmn diagram to also have sublanes in the vertical orientation with labels. This will increase the rendering test coverage: it is important to have global examples but also to improve our automatic rendering tests
I can manage it directly if ok for you

@csouchet csouchet marked this pull request as ready for review September 2, 2020 14:38
@csouchet
Copy link
Member Author

csouchet commented Sep 2, 2020

@csouchet for the non regression test, I suggested in https://github.com/process-analytics/bpmn-visualization-js/pull/561/files/41881bb6feeb1405d9d6157190542fa52ae41d1b#diff-522289dbc2f12ccea3f9476c4890e406 (collapsed comment) to update the existing pools-02-vertical bpmn diagram to also have sublanes in the vertical orientation with labels. This will increase the rendering test coverage: it is important to have global examples but also to improve our automatic rendering tests
I can manage it directly if ok for you

I didn't see it; you can do it, if you want ;)

tbouffard
tbouffard reviewed Sep 2, 2020
View reviewed changes
src/component/mxgraph/config/StyleConfigurator.ts Outdated Show resolved Hide resolved
test/e2e/mxGraph.model.test.ts
@@ -206,11 +207,13 @@ describe('mxGraph model', () => {
}

function expectModelContainsPool(cellId: string, modelElement: ExpectedShapeModelElement): void {
expectModelContainsShape(cellId, { ...modelElement, kind: ShapeBpmnElementKind.POOL, styleShape: mxConstants.SHAPE_SWIMLANE });
const mxCell = expectModelContainsShape(cellId, { ...modelElement, kind: ShapeBpmnElementKind.POOL, styleShape: mxConstants.SHAPE_SWIMLANE });
Copy link
Member

Choose a reason for hiding this comment

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

👍 for adding model tests for both vertical pool and lanes

@csouchet csouchet merged commit 3c7fe98 into master Sep 2, 2020
@csouchet csouchet deleted the 127-Render_vertical_lanes branch September 2, 2020 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbouffard tbouffard tbouffard approved these changes

Assignees
No one assigned
Labels
BPMN rendering Something about the way the lib is rendering BPMN elements enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEAT] Render vertical lanes
2 participants
@csouchet @tbouffard