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

Improve selection of a pool #2859

Closed
ingorichtsmeier opened this issue Mar 28, 2022 · 16 comments
Closed

Improve selection of a pool #2859

ingorichtsmeier opened this issue Mar 28, 2022 · 16 comments
Assignees
Labels
BPMN modeling spring cleaning Could be cleaned up one day ux
Milestone

Comments

@ingorichtsmeier
Copy link

ingorichtsmeier commented Mar 28, 2022

Is your feature request related to a problem? Please describe.

As a user I wan to model properties for elements that I click on the diagram. This works well and intuitive for tasks and connections, but does not work for container elements such as participants and sub-processes:

capture 20MKr7_optimized

If I click into a pool only selects the Collaboration properties. But it is much more important to access the elements of the Participant (name, history cleanup, candidate starter, ...)

Describe the solution you'd like

  • Clicking into a container element selects it
  • Dragging a container element (outside the drag area) still does not drag it (but drags the canvas instead)

Describe alternatives you've considered

None.

Additional context

When I model my process diagram in a pool, I have to select the begin of the pool to access the properties of the process in the properties panel.

In larger diagrams, it makes it very tedious. Scroll to the beginning, ...

The selection of the Collaboration is much less important for a developer.

@pinussilvestrus

This comment was marked as outdated.

@nikku

This comment was marked as outdated.

@nikku nikku added the help wanted Extra attention is needed label Mar 31, 2022
@ingorichtsmeier

This comment was marked as outdated.

@nikku

This comment was marked as outdated.

@ingorichtsmeier

This comment was marked as outdated.

@smbea smbea added backlog Queued in backlog and removed backlog Queued in backlog labels Apr 26, 2022
@nikku nikku added backlog Queued in backlog and removed help wanted Extra attention is needed labels May 2, 2022
@nikku
Copy link
Member

nikku commented May 2, 2022

Verified with @ingorichtsmeier that this is a UX issue. Moving to backlog.

We should investigate if we're able to separate click to select (for containers) from drag start. This way we'd be able to support this without breaking the existing move interaction.

@nikku nikku added the spring cleaning Could be cleaned up one day label May 2, 2022
@barmac barmac self-assigned this May 16, 2022
@barmac
Copy link
Contributor

barmac commented May 16, 2022

I am going to look into this this week.

@barmac barmac added in progress Currently worked on and removed backlog Queued in backlog labels May 16, 2022
@barmac
Copy link
Contributor

barmac commented May 16, 2022

I am sharing here my findings so far:

  • For "frame" elements like Participant and Group, we use CSS property pointer-events to decide which parts of the shape are interactive.
  • Because we use pointer-events: stroke for the body of both of the shapes, it is transparent to the browser regarding mouse/touch interaction.

Solution sketch:

  • Use pointer-events: all for the frame shapes.
  • Decide in the application layer whether and how to react on the interaction.
  • Ideally, the hit boxes should allow either both dragging and click interactions or only the clicks.

@barmac
Copy link
Contributor

barmac commented May 16, 2022

Use pointer-events: all for the frame shapes.

After a while, I think this should apply to Participants and SubProcesses, but not to the Group. I should select the Group only when I click the frame because there is no "body" of this element.

@barmac
Copy link
Contributor

barmac commented May 16, 2022

These are the events supported in diagram-js InteractionEvents:

  • element.click
  • element.contextmenu
  • element.dblclick
  • element.hover
  • element.mousedown
  • element.mousemove
  • element.mouseup
  • element.out

As long as there is no dragging in progress, only these should be fired for the element's body:

  • element.click
  • element.contextmenu
  • element.dblclick

However, the rest should be fired for an element below to allow moving the canvas. Also, we should reactivate the events for the element when dragging interaction is in progress (handled currently via CSS).

@barmac
Copy link
Contributor

barmac commented May 17, 2022

Another, more detailed solution sketch:

  • Introduce djs-hit-click-only type which accepts all pointer events
  • However, when this new type is activated, check the element type:
    • For click-like events, handle the event as with other hit boxes.
    • For other events, handle the event with target overwritten to the first non-djs-hit-click-only hit taken from document.elementsFromPoint.

This may degrade the performance, so let's be careful.

@barmac
Copy link
Contributor

barmac commented May 17, 2022

Alternative (limited) solution:
Instead of using document.elementsFromPoint, always fall back to the canvas. The rest is the same.

@barmac
Copy link
Contributor

barmac commented May 17, 2022

Another idea:
Have hit boxes which don't allow dragging. Build the functionality in the dragging module so that it can decide whether dragging should be activated for the hit box.

Edit: This should be rather implemented in the Move module.

barmac added a commit to bpmn-io/diagram-js that referenced this issue May 17, 2022
Use `no-move` type of hit box to disable element movement via hit box.

Related to camunda/camunda-modeler#2859
barmac added a commit to bpmn-io/bpmn-js that referenced this issue May 17, 2022
barmac added a commit to bpmn-io/diagram-js that referenced this issue May 17, 2022
Use `no-move` type of hit box to disable element movement via hit box.

Related to camunda/camunda-modeler#2859
@barmac
Copy link
Contributor

barmac commented May 17, 2022

Another idea: Have hit boxes which don't allow dragging. Build the functionality in the dragging module so that it can decide whether dragging should be activated for the hit box.

Edit: This should be rather implemented in the Move module.

I ended up implementing this solution but the new hit box type disables only the element movement feature. As a result, we don't mess with events or document.elementsFromPoint, but still achieve the desired selectable Participant/SubProcess/Lane.

fake-join bot pushed a commit to bpmn-io/diagram-js that referenced this issue May 17, 2022
Use `no-move` type of hit box to disable element movement via hit box.

Related to camunda/camunda-modeler#2859
barmac added a commit to bpmn-io/bpmn-js that referenced this issue May 18, 2022
barmac added a commit to bpmn-io/bpmn-js that referenced this issue May 18, 2022
nikku pushed a commit to bpmn-io/bpmn-js that referenced this issue May 18, 2022
@barmac
Copy link
Contributor

barmac commented May 18, 2022

This is fixed upstream via bpmn-io/bpmn-js#1646

@barmac barmac added fixed upstream Requires integration of upstream change and removed in progress Currently worked on labels May 18, 2022
@MaxTru MaxTru added this to the M54 milestone May 31, 2022
@Skaiir
Copy link
Contributor

Skaiir commented Jul 8, 2022

Closed via bc8b748

@Skaiir Skaiir closed this as completed Jul 8, 2022
@bpmn-io-tasks bpmn-io-tasks bot removed the fixed upstream Requires integration of upstream change label Jul 8, 2022
Skaiir added a commit that referenced this issue Jul 8, 2022
transitive deps:

`bpmn-js-properties-panel@0.15.1`
Closes #2989, #3003, #2725, #2920

`properties-panel@0.15.0`
Closes #2990

`bpmn-js@9.3.1`
Closes #2993, #2859
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BPMN modeling spring cleaning Could be cleaned up one day ux
Projects
None yet
Development

No branches or pull requests

7 participants