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

Keyboard move and resize improvements #295

Merged
merged 1 commit into from
Nov 9, 2023
Merged

Conversation

planger
Copy link
Member

@planger planger commented Nov 3, 2023

  • Show client-side feedback without animation
  • Trigger server operation with a debounce delay
  • Respect snap modifier (ALT) for move
  • Respect movement restrictor for move

Can only be tested with the standalone workflow example!

Fixes eclipse-glsp/glsp#1156

Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

The experience is so much better now, great work Philip! It is so smooth and I especially like the moving of the view port!

I only have one question regarding the grid: I'm not sure how it is supposed to work but in my mind it was just an imaginative grid across the whole diagram. So when I move a node a few pixels using ALT but then move it with the regular key arrow I would have expected it to be aligned with the grid and not just jump one grid width/height. The mouse tool always snaps to the next grid-aligned position:

move-tool

So maybe that is not the intention of the keytool at all but I just wanted your input on that in case we need to use the snapper not just for the grid but also to calculate a valid position.

* Show client-side feedback without animation
* Trigger server operation with a debounce delay
* Respect snap modifier (ALT) for move
* Respect movement restrictor for move

Can only be tested with the standalone workflow example!

Fixes eclipse-glsp/glsp#1156
@planger
Copy link
Member Author

planger commented Nov 9, 2023

@martin-fleck-at Thank you very much! Nice find with the grid. This was indeed wrong in the original implementation. I fixed this with a72f6ff.

Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

The behavior is even better now, thank you very much for the fast update!

I noticed one small oddity which might be fine anyway but I'd like to get your input: We properly snap to the grid when not using the ALT key now but it we do it on both axis, so when I do ALT to move a bit to the right and then use the Down Arrow without ALT, it not only moves the element down but also to the left or right depending on the proper snap/grid location. Maybe this is expected anyway, I was just a bit surprised that suddenly the element moved on another axis as well.
snap-x-y

@planger
Copy link
Member Author

planger commented Nov 9, 2023

Right, that's because we pass it through the snapper, which isn't aware of the axis on whiich the user originally moved. We could reset the axis that the user didn't impact. But I guess this is fine for now. WDYT?

@martin-fleck-at
Copy link
Contributor

@planger Agreed, that's why I also approved the PR. I'm not sure how other tools are handling this but if it ever comes to it, we can simply change the behavior. Both seem "semantically" correct to me ;-)

@planger planger merged commit 939fa9e into master Nov 9, 2023
5 checks passed
@planger planger deleted the planger/issues/1156 branch November 9, 2023 16:47
holkerveen pushed a commit to holkerveen/glsp-client that referenced this pull request Dec 21, 2024
* Show client-side feedback without animation
* Trigger server operation with a debounce delay
* Respect snap modifier (ALT) for move
* Respect movement restrictor for move

Fixes eclipse-glsp/glsp#1156
holkerveen pushed a commit to holkerveen/glsp-client that referenced this pull request Dec 21, 2024
* Show client-side feedback without animation
* Trigger server operation with a debounce delay
* Respect snap modifier (ALT) for move
* Respect movement restrictor for move

Fixes eclipse-glsp/glsp#1156
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move and resize via keyboard doesn't use feedback action dispatcher and feels laggy
3 participants