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: add Overlays to navigate collapsed subprocesses #1487

Merged
merged 2 commits into from
Oct 21, 2021
Merged

feat: add Overlays to navigate collapsed subprocesses #1487

merged 2 commits into from
Oct 21, 2021

Conversation

marstamm
Copy link
Contributor

@marstamm marstamm commented Sep 13, 2021 •
edited
Loading

I copied the relevant decisions from the epic into #1483 for a better overview

recording (21)

Closes #1483, based on #1481

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label Sep 13, 2021
@marstamm marstamm force-pushed the 1483-subprocess-navigation branch 2 times, most recently from 8682c5a to d0c1e28 Compare September 13, 2021 11:44
@marstamm marstamm mentioned this pull request Sep 15, 2021
Merged
@marstamm marstamm force-pushed the 1483-subprocess-navigation branch 2 times, most recently from a73b033 to 1976473 Compare October 8, 2021 08:25
@marstamm marstamm marked this pull request as ready for review October 8, 2021 13:35
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Oct 8, 2021
@marstamm marstamm requested review from nikku, philippfromme and a team and removed request for a team October 8, 2021 13:35
nikku
nikku requested changes Oct 13, 2021
View reviewed changes
@marstamm marstamm force-pushed the 1483-subprocess-navigation branch from 1976473 to ceb0940 Compare October 18, 2021 08:30
@marstamm marstamm requested a review from nikku October 18, 2021 08:37
@marstamm
Copy link
Contributor Author

I missed something while refactoring: The File names and Component names do not align (e.g. SubprocessCompatibility.js contains CollapsedProcessesCompatibility). I'll fix it

pinussilvestrus
pinussilvestrus reviewed Oct 18, 2021
View reviewed changes
@nikku nikku self-requested a review October 19, 2021 07:54
nikku
nikku reviewed Oct 20, 2021
View reviewed changes
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'd assume that after collapsing an expanded sub-process the drill down controls would be available. It is not the case on my machine though.

Is this a missing feature?
capture vl0Xg9_optimized

@marstamm
Copy link
Contributor Author

This is intentionally missing. For now, we only want to tackle viewer components. The Modeling behavior will be handled in a later issue

@nikku
Copy link
Member

nikku commented Oct 20, 2021

What is the issue that tracks the modeler features missing?

@marstamm
Copy link
Contributor Author

I added them to the Epic: #1443

@nikku nikku force-pushed the 1483-subprocess-navigation branch from 54f143b to 271ee5c Compare October 20, 2021 13:23
nikku
nikku reviewed Oct 20, 2021
View reviewed changes
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.

One final suggestion before we get this one merged: Can we add integration tests in our Viewer and Modeler spec to verify the feature is shipped with both distributions?

It also aids us, as we do integration testing.

Something along these lines: 271ee5c, just with a meaning full drill down or assertion that the breadcrumb exists.

@marstamm
Copy link
Contributor Author

Makes sense, I will add tests to it 👍

@marstamm marstamm force-pushed the 1483-subprocess-navigation branch from 271ee5c to a31b79d Compare October 21, 2021 08:08
marstamm and others added 2 commits October 21, 2021 10:13
@marstamm
feat: add Overlays to navigate collapsed subprocesses
e1c43e6 
This introduces a new css file to bpmn-js. To upgrade, please include
`dist/assets/bpmn-js.css` in your application.

closes #1483
@nikku @marstamm
test: add drill down spec for Modeler and Viewer
00ba3af
@marstamm marstamm force-pushed the 1483-subprocess-navigation branch from a31b79d to 00ba3af Compare October 21, 2021 08:16
@marstamm marstamm requested a review from nikku October 21, 2021 08:24
nikku
nikku approved these changes Oct 21, 2021
View reviewed changes
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 job 👏

@nikku nikku merged commit 946a0ec into develop Oct 21, 2021
@nikku nikku deleted the 1483-subprocess-navigation branch October 21, 2021 09:05
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikku nikku nikku approved these changes

@philippfromme philippfromme Awaiting requested review from philippfromme

@pinussilvestrus pinussilvestrus Awaiting requested review from pinussilvestrus

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Overlays and Breadcrumbs are used for navigation
3 participants
@marstamm @nikku @pinussilvestrus