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 arrow key controls for accessibility #142

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gibinealias
Copy link

No description provided.

@jackmoore
Copy link
Owner

@gibinealias Thanks for the pull request.

I cannot merge this request because it is globally applied. You've taken away the user's ability to navigate around the page using the arrow keys. Also, the keyboard navigation is applying to every instance of zoom on the page. There has some intent expressed by the user in order to activate keyboard controls to a particular zoomed image, then be some way for the user to release those controls when done.

1) Avoided global apply of keyboard events. It's now only applied when the zoom is active and destroyed once the zoom is stopped.
2) Set the keyboard navigation option as false by default, which enables the user to activate it on desired images alone.
@gibinealias
Copy link
Author

@jackmoore - Thanks a lot for your prompt response. Really appreciate it.
I've made changes to my PR as you suggested. Please review and let me know your thoughts on the same.

@Hecsall
Copy link

Hecsall commented Jul 8, 2020

Hi everyone, sorry to revive this old discussion.
I was working to make this plugin ADA compliant on a client website when I saw this PR.
This is a really nice addition to this plugin, but if I may, this PR should also allow moving with arrow keys without having to rely on mouseover, to allow people that use the Tab key to move to pan the image.
So I took @gibinealias code and added just a few lines near the end, just after the if (settings.touch) { ... } block:

$source.on('focus', function (e) {
    start(e);
});

$source.on('blur', function (e) {
    stop();
});

This will allow us to start and stop the zoom when the $source element is focused with Tabs.
Hope that gibinealias can add this few lines to his PR and that this PR manages to be successfully merged 😄

With this, many US websites can be ADA compliant without having to switch to some other zoom plugin 👍

@gibinealias
Copy link
Author

@Hecsall - Thanks :)
I've included your code in the PR. Please review and let me know if you have any comments.

@Hecsall
Copy link

Hecsall commented Jul 17, 2020

Seems good to me, good job!
Now we just have to wait for this PR to be merged 😄

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.

None yet

3 participants