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 right-click to reset zoom and pan #87

Closed
wants to merge 2 commits into from

Conversation

dlg-yahoo
Copy link

Add an option to enable right clicking to reset the zoom and pan.

@bumbu
Copy link
Owner

bumbu commented Feb 10, 2015

Thanks for the pull.
Please provide a pull request without dist files. We do build these files (and bump versions) only before releases.

I'm not sure if this should be a core functionality as right click has default behavior in majority of browsers (for me on OS X Chrome it displays a contextual menu). And I would expect it to work that way.

But even if we're going to add this functionality it is necessary first to:

  • Check before double click
  • Add enable and disable methods to the API for this functionality (as with other options).

@dlg-yahoo
Copy link
Author

Will do. (Mainly wanted to make sure that it was interesting to someone other than me before spending too much time.)Do you think that double-right-clicking should have a distinctive behavior? Options (and I only want to implement one):- Right click to zoom out. Double right-click to reset zoom and pan. - Right click to reset zoom and pan. Double right-click to reset pan. - ??

@bumbu
Copy link
Owner

bumbu commented Feb 10, 2015

Sincerely I think that we shouldn't touch right click at all as it is not a feature on which we can and should rely. Some browser may even not allow you to block contextual menu on right click. So you'll have pan zoom functionality and the menu. And this will be confusing.

But I would like to hear opinions of other people. @ariutta?

@ariutta
Copy link
Collaborator

ariutta commented Mar 4, 2015

Hmm... would it be possible for this functionality to work as a plugin? I appreciate the pull request, but I agree with @bumbu that this may not belong in the core of the library.

@dlg-yahoo
Copy link
Author

Because of how deeply click handling is intertwined with the library, it seems like a plugin wouldn't work particularly well. (I think that the current code doesn't -- depending on the options set -- distinguish between double-right and double-left clicks.)

@dlg-yahoo
Copy link
Author

The application, by the way, is viewing SVGs rather the size of blueprints which require a large amount of zooming-in, zooming-out, and panning on a computer screen.

I'm open to suggestions as to the right way to do this... (Right now I'm using this trivial patch internally.)

@bumbu
Copy link
Owner

bumbu commented Mar 4, 2015

It should be possible to create a plugin without changing the core via customEventsHandler option.
But I agree that it needs some effort as you'll have to handle manually panning and zooming by double-click as these actions are dependent on mouseclick event.

It is an idea for a future version to allow easier manipulation of inner used events.

@ariutta
Copy link
Collaborator

ariutta commented Mar 4, 2015

I like that. We'll refactor to allow easier manipulation of inner used events. Then the code from this pull request can become the first plugin, thanks to @dlg-yahoo!

At this point, the core of the library seems nearly feature complete to me. To keep the size reasonable, let's think twice before adding new features to the core. But once the above refactoring is done, we can freely add unlimited new features as plugins.

@ariutta ariutta mentioned this pull request Mar 4, 2015
34 tasks
@ariutta
Copy link
Collaborator

ariutta commented Mar 4, 2015

@dlg-yahoo, thanks again for the pull request and sorry for my delay in responding. I added you as a collaborator so you can contribute more easily. Feel free to edit examples/documentation/etc. at any time. Just give me or @bumbu a chance to review first if you want to commit to the core code in the master branch.

@bumbu
Copy link
Owner

bumbu commented Jun 3, 2015

This code may be moved into a plugin (as per #98) but it will be not merged into core.

@bumbu bumbu closed this Jun 3, 2015
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