feat(canvas): add #scrollToElement function

[marstamm]

• adjust AutoPlace to scroll to new element
• adjust SearchPad to use centralized function

related to camunda/camunda-modeler#1249
replaces bpmn-io/bpmn-js#1437

[philippfromme]

Hint: It would have been nice to separate this PR into two commits, one that adds the new canvas method and one that adds the new scroll behavior. 😉

[marstamm]

That makes sense, I followed the "one commit per PR" rule to closely 😄

EDIT: we do not have this rule, I just misunderstood the definition of done :D

[MaxTru]

That makes sense, I followed the "one commit per PR" rule to closely

EDIT: we do not have this rule, I just misunderstood the definition of done :D

When I was onboarded I had the same confusion because of the sentence a single commit closes the issue via Closes #issuenr in our https://github.com/bpmn-io/internal-docs/blob/master/development/DEFINITION_OF_DONE.md#definition-of-done.

Since we both had that confusion I propose to re-formulate it to say
the last commit in the PR closes the issue via Closes #issuenr

I will create a PR: https://github.com/bpmn-io/internal-docs/pull/268

[nikku]

Does not seem to work great when zoomed in or out yet. Also notice how in the search case you may want to provide a larger top padding to not hide behind the search widget / dropdown.

---

[marstamm]

Thank you for the hints 👍

I made the scrolling zoom-aware and added 400px top-padding when used in the search box

[nikku]

One last remark (and I hope we'll be done after this):

As you can see in the following screen capture the functionality is not stable (indempotent, whatever). If an element scrolled into origin once I'd assume further scrolling invocations to not scroll somewhere else.

This is an issue in the search case. Is it possible to address this?

[marstamm]

What would be the expected behavior when the element is to big to fit into view (with padding)? I would focus on the middle-top, as less get's hidden by UI elements and Sub-Process have the label there

[philippfromme]

What would be the expected behavior when the element is to big to fit into view (with padding)?

I'd say, either center-center or top-left.

[marstamm]

These are our options:
Center-Center
note that we center in the padded viewbox, so it looks off-center.

top-left

Top-Center

IMO, top-left looks better for Participants and Top-Center for Sub-processes. What do you prefer?

[nikku]

We need to keep in mind that this is not bpmn-js specific. Therefore I'd ignore the participant case and vote for top-center.

[philippfromme]

I'd go for the very generic top-left, which always gives you a corner.

[marstamm]

+1 for Philipp. Top-left also is the best when focusing on Flows while modeling left-right. For example in this model, every other method would focus on a point outside of the path:

[philippfromme]

Overall very good with nice test coverage. I've got a few remarks.

[nikku]

Looks good from my perspective.

[philippfromme]

👏🏻