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.

Sign up for GitHub

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

Jump to bottom

add component to safely restore focus #81

Open
rodneyrehm opened this issue Nov 30, 2015 · 5 comments
Open

add component to safely restore focus #81

rodneyrehm opened this issue Nov 30, 2015 · 5 comments
Labels
feature focus

Comments

@rodneyrehm
Copy link
Member

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 rodneyrehm modified the milestone: 1.1.0 - second wave Nov 30, 2015
@jzaefferer
Copy link

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.

@rodneyrehm
Copy link
Member Author

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.

@jzaefferer
Copy link

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

@rodneyrehm
Copy link
Member Author

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

@jzaefferer
Copy link

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

@rodneyrehm rodneyrehm modified the milestones: 1.2.0 - jQuery UI, 1.1.0 - second wave Mar 13, 2016
This was referenced Mar 13, 2016
Closed
Closed
Closed
@rodneyrehm rodneyrehm removed this from the 1.3.0 - Bob The Builder milestone Jan 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature focus
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jzaefferer @rodneyrehm