medialize / ally.js

JavaScript library to help modern web applications with accessibility concerns
http://allyjs.io/
MIT License
1.53k stars 83 forks source link

Setting the focus source #150

Closed jantimon closed 7 years ago

jantimon commented 7 years ago

Hi,

if I read http://allyjs.io/api/style/focus-source.html correctly there is only lock but no set.

My use case would be the following:

The script redirects the focus to a certain element and for this one time it should behave like a keyboard focus. After that ally.js should detect the real source just like before.

dream code:

var currentSource = handle.current();
document.getElementId('demo').focus();
handle.set(currentSource);

Is this already possible?

rodneyrehm commented 7 years ago

This used to be possible but was removed in 318848f. This became obsolete (to me) with 378ed5e extending the duration of handle.current() returning the identified state. If you look at the example and click on the "focus" button, you'll notice the element getting the pointer style. However, if you Tab to the button and press Space the element will get the keyboard style.

So this the following will simply "forward" focus with the correct style for the given user input:

element.addEventListener('click', function() {
  otherElement.focus();
});

This will fail for async forwarding of focus. The following will interpret the focus to be given by script:

element.addEventListener('click', function() {
  setTimeout(function() {
    otherElement.focus();
  });
});

The removed .next() function was a special case of .lock(), that basically did the following async compatible thing:

var handle = ally.style.focusSource();

element.addEventListener('click', function() {
  handle.lock(handle.current());
  setTimeout(function() {
    otherElement.focus();
    handle.lock(false);
  });
});

If you feel any of this is important to know, the documentation and example are editable. Also there's a doc for docs.

rodneyrehm commented 7 years ago

@jantimon did this solve your problem or do we need to take some action?

jantimon commented 7 years ago

@rodneyrehm yeah thanks - I am quite impressed how easy it works as the synchronous focus redirect keeps the original source.

Still I don't get why there is handle.used(<string>) it seems to me as it would be a useless abstraction:

if (handle.current() === 'key') { ... }
if (handle.used('key')) { ... }

I didn't saw the "unlock" part because the docs say handle.lock(<String>) so I didn't read the description saying false will unlock.. maybe handle.unlock() is a cleaner API than handle.lock(false)`

But it works pretty well 👍

rodneyrehm commented 7 years ago

Still I don't get why there is handle.used(<string>) it seems to me as it would be a useless abstraction

Firefox (I think, and possibly other browsers) may not apply focus style on buttons when clicked, unless at some point the user pressed Tab at least once before. The .used() is there to easily identify if the user has ever used the keyboard to shift focus. I've not used this myself, but it seemed like a simple thing to provide.

I didn't saw the "unlock" part because the docs say handle.lock(<String>) so I didn't read the description saying false will unlock.. maybe handle.unlock() is a cleaner API than handle.lock(false)

First of all the docs should probably show handle.lock(false) in the usage block, instead of only mentioning it in a comment. Secondly I agree, .unlock() would be cleaner.

Would you like to send a PR for this?

jantimon commented 7 years ago

Done #151