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

[image-view] Add scroll support to zoom-in and zoom-out #1197

Open
wants to merge 10 commits into
base: updated-latest-electron
Choose a base branch
from

Conversation

asiloisad
Copy link
Contributor

It's natural to zoom-in and zoom-out images by Ctrl-Scroll. It was required to change how zoom-to-fit works.

@confused-Techie
Copy link
Member

confused-Techie commented Jan 16, 2025

Really excited about this feature idea, and something I've absent-mindedly tried to do way to many times forgetting it doesn't exist.

But I do see some test failures here, that I don't necessarily believe are the fault of your code at all, but are ones we don't expect to see, so we will probably want to take a closer look at those sometime soon.

@DeeDeeG a ping for whenever your available if you'd like to help look into this.


EDIT:

Sorry, I missed this was aimed at updated-latest-electron, which most certainly may be the reason we are seeing the failures.

If you intended to target the dev electron branch, feel free to ignore my above comment. But if you otherwise think this is something that can work now, which seems to me like it could, maybe you can retarget to the master branch and we can merge it for the rolling releases now, and ideally our CI will be much happier

@asiloisad
Copy link
Contributor Author

My pull request introduce scroll zooming, but I think it's not best approach. I will prepare another solutions like this: https://dev.to/stackfindover/zoom-image-point-with-mouse-wheel-11n3. Please wait to check it, I will prepare a new version.

@asiloisad
Copy link
Contributor Author

Summary:

  • zoom by Ctrl-Scroll added,
  • zoom-to-fit mechanism changed, but effects remain the same
  • added native background,
  • added button Center,
  • fixed bug in Pulsar of false start,
  • zoom buttons target set to centre of picture (instead top-left corner),
  • added 5% scale to predefined levels,
  • changed speed of navigation via arrows.

I have tested it with Pulsar and PulsarNext.

@asiloisad asiloisad changed the base branch from updated-latest-electron to master January 16, 2025 22:59
@asiloisad asiloisad changed the base branch from master to updated-latest-electron January 16, 2025 22:59
@asiloisad
Copy link
Contributor Author

I have tried to change the base branch of the pull request, but there are 200+ commits from the updated-latest-electron branch. How can I change the base branch to avoid this?

@savetheclocktower
Copy link
Contributor

I have tried to change the base branch of the pull request, but there are 200+ commits from the updated-latest-electron branch. How can I change the base branch to avoid this?

I'd suggest creating a new branch off of master, then cherry-picking the five commits in this PR onto that branch in order. Then you can open a second PR against master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants