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

Add move cursor #643

Closed
wants to merge 0 commits into from
Closed

Add move cursor #643

wants to merge 0 commits into from

Conversation

nikku
Copy link
Member

@nikku nikku commented May 20, 2022

This builds on top of #640 and adds a move cursor as known from other well known drawing tools.

The cursor visually indicates to me whether I'm hovering over an active drag region:

capture E1qwQq_optimized


Can be tried out via

npx @bpmn-io/sr bpmn-io/bpmn-js#develop -l bpmn-io/diagram-js#move-cursor

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label May 20, 2022
@nikku nikku changed the base branch from develop to select-hover-states May 20, 2022 09:06
@nikku nikku changed the title Move cursor Add move cursor May 20, 2022
@nikku nikku requested review from andreasgeier, barmac and smbea May 20, 2022 09:07
@barmac
Copy link
Member

barmac commented May 20, 2022

To consider: Movement is not the only action available in such situation. I can also click to select the shape which is not the action indicated by the cursor. Also, over a task, the double-click allows to change the label name which is totally different from what the cursor shows.

I think it's interesting that the proposed solution is in line with what Excalidraw offers but different than Miro's experience (where it's as is in bpmn-js now).

@barmac
Copy link
Member

barmac commented May 20, 2022

I believe this solution makes sense if we stick to the decision to remove the hover outline.

@nikku
Copy link
Member Author

nikku commented May 20, 2022

I think it's interesting that the proposed solution is in line with what Excalidraw offers but different than Miro's experience (where it's as is in bpmn-js now).

Agreed. It is also similar to GSlides + a bunch of other drawing tools that I investigated; hence I proposed it.

@nikku
Copy link
Member Author

nikku commented May 20, 2022

@barmac Could you share a screencapture how it looks like for you (during hovering and dragging)? We have to make sure our overall cursor-interaction works and my OS is very minimal in this case (just a few cursors to choose).

@barmac
Copy link
Member

barmac commented May 20, 2022

Sure, I will do that shortly.

@barmac
Copy link
Member

barmac commented May 20, 2022

Screen.Recording.2022-05-20.at.11.43.21.mov

@barmac
Copy link
Member

barmac commented May 20, 2022

It looks the same in Safari, so the cursor is probably browser-independent.

@nikku
Copy link
Member Author

nikku commented May 20, 2022

It looks the same in Safari, so the cursor is probably browser-independent.

Yes, it is OS-dependent.

@nikku
Copy link
Member Author

nikku commented May 20, 2022

Looks worse on Mac than it does on Linux I gotta say.

@nikku nikku requested a review from christian-konrad May 20, 2022 09:47
@nikku
Copy link
Member Author

nikku commented May 20, 2022

Alternative is to use cursor: grab rather than cursor: move. It may be less "in your face", but also different from how Gslides and Excalidraw (as examples) do it.

@christian-konrad
Copy link

It clashes a little with the sequence flow elbows when you come near the connection drag handle:

Bildschirmaufnahme.2022-05-20.um.12.00.08.mov

It is then not clear if you create a new elbow or pick the connection drag handle.

I would also suggest not use cursor: grab; this post here comes up with a great explanation: https://ux.stackexchange.com/a/75840

@barmac s point is valid, but until today no one came up with a solution for a cursor that indicates the double click function (secondary action)... and we can not simply show a cursor: text as it is not the primary action.

@nikku
Copy link
Member Author

nikku commented May 20, 2022

Let's allow @andreasgeier to chime in and then we can pragmatically decide whether we want this (or not).

@nikku nikku force-pushed the select-hover-states branch from 21e4c8f to c5b1706 Compare May 20, 2022 12:10
@nikku nikku marked this pull request as ready for review May 20, 2022 12:19
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels May 20, 2022
@nikku nikku added the ux label May 20, 2022
Base automatically changed from select-hover-states to develop May 23, 2022 13:12
@nikku nikku closed this May 23, 2022
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label May 23, 2022
@nikku nikku deleted the move-cursor branch May 23, 2022 20:34
@nikku nikku restored the move-cursor branch May 23, 2022 20:34
@nikku nikku deleted the move-cursor branch May 23, 2022 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants