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

#2909 - movement of structure ceases when it reaches the boundaries of canvas requiring the consistent movement of mouse to continue its motion #3342

Merged
merged 9 commits into from
Sep 25, 2023
Merged

#2909 - movement of structure ceases when it reaches the boundaries of canvas requiring the consistent movement of mouse to continue its motion #3342

merged 9 commits into from
Sep 25, 2023

Conversation

nanoblit
Copy link
Collaborator

@nanoblit nanoblit commented Sep 19, 2023 •
edited
Loading

How the feature works? / How did you fix the issue?

Made smooth scroll work using requestAnimationFrame to update the scroll every frame (it only happens when the user is holding a structure close to a screen border.). Also made sure to make scrolling framerate independent, among other things.

Check list

  • unit-tests written
  • e2e-tests written
  • documentation updated
  • PR name follows the pattern #1234 – issue name
  • branch name doesn't contain '#'
  • PR is linked with the issue
  • base branch (master or release/xx) is correct
  • task status changed to "Code review"
  • reviewers are notified about the pull request

@nanoblit
#2909 - Add smooth movement
d0745e7
@nanoblit
Merge branch 'master' of https://github.com/epam/ketcher into 2909-mo…
01771df 
…vement-of-structure-ceases-when-it-reaches-the-boundaries-of-canvas-requiring-the-consistent-movement-of-mouse-to-continue-its-motion
@nanoblit nanoblit linked an issue Sep 19, 2023 that may be closed by this pull request
Movement of structure ceases when it reaches the boundaries of canvas, requiring the consistent movement of mouse to continue its motion #2909
Closed
@nanoblit
Copy link
Collaborator Author

Most of the issues below are also reproducible in live version.
Issues:

  • When the view is moved up or left and trying to move a structure up or left further, there is an area on the screen where the view will scroll backwards.
  • Scroll speed is inconsistent.
  • When moving a structure down or right and scrolling then leaving the structure, the canvas will be resized to the position of the structure. This happens only if the structure is close enough to the border of the canvas. This doesn't happen when moving the structure up or left.
  • When moving the structure down and then left or right / moving the structure right then up or down, the canvas position will reset. This doesn't happen for up and left.
  • When moving a structure far in any direction and returning, the canvas' size will reset. It won't reset if leaving the structure anywhere in between the farthest position and the original position, leaving a lot of empty space.

@nanoblit
#2909 - Refactor code
2fee521
@nanoblit
#2909 - Make canvas movement framerate independent
f7b57a5
@nanoblit
#2909 - Refactor
e6effdb
@nanoblit
Merge branch 'master' of https://github.com/epam/ketcher into 2909-mo…
8ad4ad0 
…vement-of-structure-ceases-when-it-reaches-the-boundaries-of-canvas-requiring-the-consistent-movement-of-mouse-to-continue-its-motion
@nanoblit nanoblit mentioned this pull request Sep 21, 2023
Closed
@nanoblit
Copy link
Collaborator Author

all-bonds.spec.ts tests passing locally:
all bonds spec ts PASS

Other failing tests should be fixed by Nitvex soon. They are unrelated to this task.

@nanoblit nanoblit marked this pull request as ready for review September 21, 2023 13:13
@nanoblit
Merge branch 'master' of https://github.com/epam/ketcher into 2909-mo…
28f30a0 
…vement-of-structure-ceases-when-it-reaches-the-boundaries-of-canvas-requiring-the-consistent-movement-of-mouse-to-continue-its-motion
Nitvex
Nitvex reviewed Sep 22, 2023
View reviewed changes
packages/ketcher-react/src/script/editor/tool/select.ts Outdated
class SelectTool implements Tool {
readonly #mode: SelectMode;
readonly #lassoHelper: LassoHelper;
private readonly editor: Editor;
private dragCtx: any;
isMousedDown = false;
private previousMouseMoveEvent: any;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I propose to use proper type MouseEvent

@Nitvex Nitvex merged commit 4d84438 into master Sep 25, 2023
@Nitvex Nitvex deleted the 2909-movement-of-structure-ceases-when-it-reaches-the-boundaries-of-canvas-requiring-the-consistent-movement-of-mouse-to-continue-its-motion branch September 25, 2023 12:52
@Nitvex Nitvex mentioned this pull request Sep 25, 2023
Closed
8 tasks
SashaGraves pushed a commit that referenced this pull request Sep 28, 2023
@nanoblit @SashaGraves
#2909 - movement of structure ceases when it reaches the boundaries o…
63ec1b8 
…f canvas requiring the consistent movement of mouse to continue its motion (#3342)

* #2909 - Add smooth movement

* #2909 - Refactor code

* #2909 - Make canvas movement framerate independent

* #2909 - Refactor

* #2909 - Remove scrollMultiplier

* #2909 - Add type for previousMouseMoveEvent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nitvex Nitvex Nitvex left review comments

@chgayane chgayane chgayane approved these changes

@y-holik y-holik y-holik approved these changes

Assignees

@nanoblit nanoblit

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@nanoblit @Nitvex @chgayane @y-holik