ally.js icon indicating copy to clipboard operation
ally.js copied to clipboard

Published 20 hours ago • verified publisher icon medialize

add component to safely restore focus

Open rodneyrehm opened this issue 9 years ago • 5 comments

This topic came up while talking jQueryUI integration with @jzaefferer:

The dialog tutorial features a naive method of storing and restoring focus. The following scenarios (upon restore) are not unheard of:

  • the element is not focusable anymore
  • the element does not exist anymore

safe focus()

Calling element.focus() on something that is not focusable or attached to the DOM will cause <body> to become the activeElement. In case we're trying to focus a non-focusable element, we could however traverse the parents to find a focusable element and pass focus to that element instead. ally.get.focusTarget should help with that. We would wrap this in ally.element.focus and could provide the following options:

  1. "do not shift focus away from activeElement unless there is a valid target"
  2. "predefined fallback element to focus if target cannot be resolved" (also using ally.get.focusTarget for convenience)
  3. "try focusing next element in DOM hierarchy" (before engaging 1 or 2)
  4. "wait until element is focusable" (using ally.when.focusable but not returning the service handle)

restore current activeElement service

If we simply store the activeElement and it is detached from the DOM before we restore focus, we can't get its ancestry anymore. For that reason it makes sense to provide a service that upon engaging stores the current element hierarchy. The service handle would expose a function .restore() that tries to shift focus to an element in the previously saved hierarchy. At engage() and restore() a "fallback" callback can be passed that is invoked should the service not be able to restore focus.

Should this be made available at ally.maintain.restorableFocus?

rodneyrehm avatar Nov 30 '15 13:11 rodneyrehm

Relevant snippet for restoring activeElement from jQuery UI dialog: https://github.com/jquery/jquery-ui/blob/ffcfb85c9818954adda69e73cf9ba76ea07b554c/ui/widgets/dialog.js#L224-L230 - has some inline comments for WebKit specific issues, might be useful.

That uses our safeBlur and safeActiveElement helpers - both have a bunch of support comments as well.

I haven't looked through ally.js enough to see if any of those aren't yet covered.

jzaefferer avatar Dec 14 '15 16:12 jzaefferer

thx for the pointers!

You're mostly dealing with browsers "misbehaving" when hiding or even removing the activeElement.

  • document.activeElement may be set to null in for document.activeElement.parentNode.removeChild(document.activeElement) in Internet Explorer. The "same" is true in Firefox if the active element was in a Shadow DOM (likely a subsequent fault because of Shadow DOM leaking the active element in the first place)
  • I was not aware of IE's document.body.blur() behavior described in 9420
  • I assume the iframe try-catch is there to avoid cross-origin warnings?!

I haven't looked through ally.js enough to see if any of those aren't yet covered.

nothing so far. I guess I'll want to test these things before proceeding with implementing this issue.

rodneyrehm avatar Dec 14 '15 17:12 rodneyrehm

I think the try/catch addresses: https://bugs.jqueryui.com/ticket/8443

jzaefferer avatar Dec 14 '15 17:12 jzaefferer

indeed. looks like we should copy the safeActiveElement() and safeBlur() to ally, then…

rodneyrehm avatar Dec 14 '15 17:12 rodneyrehm

Seems good to me. CCing @scottgonzalez and @tjvantoll on this, they might have some more useful insight as well.

jzaefferer avatar Dec 14 '15 22:12 jzaefferer