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

Zoom and Pan #22

Closed
Davejkane opened this issue May 8, 2019 · 12 comments · Fixed by #67
Closed

Zoom and Pan #22

Davejkane opened this issue May 8, 2019 · 12 comments · Fixed by #67
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Milestone

Comments

@Davejkane
Copy link
Owner

  • Add ZoomIn, ZoomOut, PanUp, PanDown, PanRight & PanLeft to Action enum in ui.rs
  • Otherwise make a new ZoomAction enum with two variants, and a PanAction enum with the four variants and add Zoom(ZoomAction) to the Action enum and Pan(PanAction) to the Action enum
  • Match i and UpArrow to ZoomIn
  • Match o and DownArrow to ZoomOut
  • Match shift+arrow keys and shift+j,k,l,; to Pan directions.

In the zoom function limit zoom in to 10x size and limit zoom out to actual size for images smaller than screen, or fit screen for images larger than screen.

In the pan function limit panning to not allow image edge to go past screen edge. It shouldn't be possible to pan the image more than that.

@Davejkane Davejkane added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels May 8, 2019
@Davejkane Davejkane added this to the 0.3 milestone May 8, 2019
@ori155
Copy link

ori155 commented May 9, 2019

Hi,
I'd like to take this one.
I think I'll take the ZoomAction and PanAction approach, as I think it will be easier to extend later if needed.

@Davejkane
Copy link
Owner Author

Excellent. It's all yours. Thanks a lot for getting involved.

@gurgalex
Copy link
Collaborator

Hi @ori155

There has been a large refactor in the past few days #34 (issue #28)

The development of #37 will be relevant as well shortly.

@Davejkane
Copy link
Owner Author

Hi @ori155. I don't want to put any pressure on, but can you please clarify if you're still taking care of this? If not I'd like to have a look at it.

@gurgalex
Copy link
Collaborator

@Davejkane I'd like to work on this while waiting for the implementation of #40 to finalize (which NickHackman is working on in #58)

@gurgalex
Copy link
Collaborator

@Davejkane a couple questions.

if someone pans 100 pixels right and 100 pixels down from center, then toggles back and forth between actual fit and fit to screen. Should the offset from center be preserved? What about the zoom level?

I'd go with both being preserved between toggling.

I assume the offset should be reset to centered on image change.

Should the zoom level be reset when moving between images?

@Davejkane
Copy link
Owner Author

@gurgalex. In my mind neither the offset or the zoom should be preserved if you start playing with the z key. Totally fine for that to just centre the image, unless it's easier to implement it with preserving, in which case go ahead.

And yes, I'd expect zoom and pan to be reset when changing images.

@gurgalex
Copy link
Collaborator

What should each of the j k l and ; keys correspond to in pan directions?

@Davejkane
Copy link
Owner Author

J = down, K = up, H = left, L = right. Sorry I said semi-colon. Was getting mixed up between vim and i3. Just the same as vim.

@gurgalex gurgalex self-assigned this May 21, 2019
@Davejkane
Copy link
Owner Author

@gurgalex. Now that Nick's branch is released, do you want to focus on Numerical arguments and I'll take this one? Or do you still want to do this one as well?

@gurgalex
Copy link
Collaborator

@Davejkane
I'll take Numerical arguments.

Hard part in this is zooming in.

@gurgalex gurgalex assigned Davejkane and unassigned gurgalex May 24, 2019
@Davejkane Davejkane mentioned this issue May 25, 2019
@ori155
Copy link

ori155 commented May 25, 2019

Hi, It seems I can't put the time right now in order to be in your pace, sorry about the late replay.
I'm leaving it to you @Davejkane .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants