focus considerations

[jdanyow]

(moved from aurelia/framework#142)

1. .autofocus / autofocus:
2. preventing users from tabbing off modals onto the underlying page
3. Ensuring the focus is restored to the previously focused element when the modal is closed.
4. Nested modals: Are all the scenarios above handled when multiple modals are opened?

[plwalters]

@jdanyow I believe we have an attachFocus behavior that handles the focusing of the dialog now properly to handle this. I'm going to put together some E2E tests now to validate and if so close this out.

[AdamWillden]

FYI my experience of using the autofocus attribute was unreliable. It worked sometimes and not others but I wasn't paying too much attention when testing and replaced all of them with focus.one-time="true" which worked reliably

[jwahyoung]

More accessibility stuff here, for reference - https://accessibility.oit.ncsu.edu/training/aria/modal-window/version-3/

[jods4]

@AdamWillden autofocus specs says that it only works for the first element with autofocus added to the DOM. The current spec is useless for a SPA, as you never navigate to a new page but you want to focus things when swapping views.

@jdanyow The attach-focus attribute found in this module is extremely useful for all applications, not just modals. Don't you think that you should move it to aurelia-templating-resources?

[EisenbergEffect]

Possibly. If we want to do that...it has to happen today. @jdanyow @jedd-ahyoung Thoughts?

[jdanyow]

My two cents:

I think building components that steal the focus when they are attached can be dangerous and hinder usability and composability of components. The attached component lifecycle event lacks context and is not necessarily associated with navigation activation or modal pop.

I know that everyone does not agree with my view on this (@jods4 I think you and I have discussed this before elsewhere 😉). I do accept that attach-focus can be a quick fix / do the job in a lot of basic scenarios.

[jwahyoung]

Perhaps we should move this into templating, but @jdanyow may be right
about this. This may not be the best way to deal with focus. However, as a
tool, it's not something that should be relegated to the dialog plugin only

• unless we're going to switch it out with something else entirely. If it's
  a tool that we provide, we should provide it in a general context and let
  developers make the choice for using it (restricting it to pages, not
  components, not using it at all, etc).

2016-06-20 13:48 GMT-04:00 Jeremy Danyow [email protected]:

My two cents:

I think building components that steal the focus when they are attached
can be dangerous and hinder usability and composability of components. The
attached component lifecycle event lacks context and is not necessarily
associated with navigation activation or modal pop.

I know that everyone does not agree with my view on this (@jods4
https://github.com/jods4 I think you and I have discussed this before
elsewhere 😉). I do accept that attach-focus can be a quick fix / do the
job in a lot of basic scenarios.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ACEz9syioL6BKhcTNfD-NP8wI6uGVrMyks5qNtJSgaJpZM4FT4hK
.

[EisenbergEffect]

There's another option, which is to remove it altogether and either, then:

• Simply add a gist for this.
• Add it to the CLI/Skeletons as a sample resource (which just happens to be useful)

(We could work to make gists installable through the cli in the future.)

[jods4]

I think building components that steal the focus when they are attached can be dangerous

Yes I remember we had this discussion in another thread.

I think this custom attribute is generally useful and I add it in all my projects. Maybe it can be misused but for me the onus is on the dev.

I don't mind if this doesn't belong to Aurelia proper, but I find it strange that it is offered by aurelia-dialog which is optional, when it could be in aurelia-templating-resources and used for the main pages/navigation.

EDIT BTW: if you have a robust and comprehensive solution for focus management, I'm interested! ;)

[jdanyow]

I liked how durandal did it: https://github.com/BlueSpire/Durandal/blob/master/src/plugins/js/dialog.js#L502

And in past projects had added logic to execute the same code on navigation complete events. This resulted in a convention that could be used throughout the app, avoiding components "fighting" for focus.

On one hand I agree we could prescribe an approach with gists, etc, on the other hand it would be nice (if everyone agreed the durandal way is the best) if we specified a convention, updated the router and dialog plugins to follow that convention, and people could opt in by adding .autofocus or autofocus (depending on the convention).

[jwahyoung]

Perhaps @EisenbergEffect has the right idea here. If the ai-dialog elements don't make use of this attribute at all, we might be able to replace it or place it in a separate repository.

As for the autofocus....that's something that I've been thinking about in terms of accessibility for the modal. Perhaps when the modal is opened, it will search for an autofocus attribute and give that element focus? I'm not quite sure of this yet.

[EisenbergEffect]

@jdanyow I would be down with that. Then we can remove it from here. We can add the capability to the core. Anyone who wants another behavior can always write their own custom attribute to handle it.

[jods4]

$childView.find('.autofocus').first().focus();

I've done this in the past as well. I went even further:

• I filtered to only the visible, non-disabled, non-readonly inputs. This enables more scenarios, like having multiple tabs and you don't know which one is visible (so you put multiple .autofocus). There are better tie-breakers than first, which should be last (sic) ressort.
• If there was no .autofocus I would fallback to focusing the first input anyway as it's a good thing for usability and less work for the dev in the usual case.

As you can see there is a fair bit of policy going on. Maybe best to do so in a global dialogOpened() event? Could use an overridable default.

In the end, what I like with the attribute, is that it doesn't need duplication and works everywhere. You probably are going to want the same behavior for dialogs, but also pages (in the router) and possibly controls that make other UI appears, such as small non-modal popups... What is lost is the "first input by default" but I can live with that. attach-focus is not much to add in your templates.

Anyway it seems all these options are going to be non-framework samples for the moment. And it's probably best like that ;)

I still wish WHATWG would fix autofocus for SPA...

[jwahyoung]

All of that sounds good. However, I think it might be good to search for
the autofocus attribute itself instead of an autofocus class, as it's more
semantic. Aside from that, I'm not sure about the first focusable element
fallback; I think that should be an opt-in, if anything. I don't think
that's a behavior that should be forced by default. (One of the reasons for
this is that if a developer wants to focus an element, he or she should
have to explicitly do this; having implicit behavior could actually lead to
a lack of usability in many cases, such as autofocusing the first hyperlink
in a paragraph for a screen reader user - a situation that should be
avoided.)

2016-06-20 15:15 GMT-04:00 jods [email protected]:

$childView.find('.autofocus').first().focus();

I've done this in the past as well. I went even further:

• I filtered to only the visible, non-disabled, non-readonly inputs.
  This enables more scenarios, like having multiple tabs and you don't know
  which one is visible (so you put multiple .autofocus). There are
  better tie-breakers than first, which should be last (sic) ressort.
• If there was no .autofocus I would fallback to focusing the first
  input anyway as it's a good thing for usability and less work for the dev
  in the usual case.

As you can see there is a fair bit of policy going on. Maybe best to do so
in a global dialogOpened() event? Could use an overridable default.

In the end, what I like with the attribute, is that it doesn't need
duplication and works everywhere. You probably are going to want the same
behavior for dialogs, but also pages (in the router) and possibly controls
that make other UI appears, such as small non-modal popups... What is lost
is the "first input by default" by I can live with that. attach-focus is
not much to add in your templates.

Anyway it seems all these options are going to be non-framework samples
for the moment. And it's probably best like that ;)

I still wish WHATWG would fix autofocus for SPA...

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ACEz9ma-NSHIg6kFIpE6ylVz5czchOzCks5qNubDgaJpZM4FT4hK
.

[jods4]

such as autofocusing the first hyperlink in a paragraph

In previous project, I only auto-focused textboxes and when opening dialog boxes.

I agree all of this is highly personal policy/preferences dependent.

[jwahyoung]

Thinking more about this -a few thoughts:

• It seems as though there have been woes with autofocus in modals on mobile devices: HTML5 <input> autofocus attribute inside modals twbs/bootstrap#12525
• Autofocus should probably be checked when the router navigates, and this might need to end up being part of the app-router (or templating). @jods4 made a good point (which i missed earlier) about the autofocus attribute not necessarily being valid after a navigation due to dynamic DOM rendering.
• We definitely need to change focus when the modal opens (in order to call focus to the modal itself). I originally thought that we might be able to invalidate all tabindexes on all tabbable elements in the DOM to achieve this, but this might be difficult and error-prone.

We can leave this up to the developer to implement, but I don't think that makes for a good modal plugin. Alternatively, users can create a custom renderer plugin to use a modal solution that provides accessibility already (Bootstrap, JQueryUI, etc). I'm still not sure that I see a good solution to this.

[EisenbergEffect]

Ok, so what is the proposal? Is it to remove the autofocus from the dialog and instead add a core capability to the templating engine which is then used by modal and the router to focus UI?

[jdanyow]

@EisenbergEffect yes I think so. Then we'd just need to define how that core capability works and how it can be overridden or disabled.

I'm on board with going further than what the durandal logic did. What @jods4 described in #1 (comment) sounded good to me.

in terms of .autofocus vs autofocus (class vs attribute) for our standard behavior, I'm leaning attribute... we'd effectively be making a standard work for SPAs. Either way, it can be overridden... most important thing is to get this in so people don't have to figure out how to build their own from scratch whenever they want to use the dialog or the router.

[glen-84]

I put attach-focus on this line and it doesn't appear to work.

That's the 5th bug I've noticed since I started using this plug-in. 😞

[RomkeVdMeulen]

@jedd-ahyoung The latest version of "The Incredible Accessible Modal Window" is now at https://github.com/gdkraus/accessible-modal-dialog

[plwalters]

@glen-84 on latest this is confirmed as working.

[RomkeVdMeulen]

I'm confused. I understand that the autofocus behaviour is moving to templating-resources (btw: is there an issue for this that I can track?). But what of the other considerations from @jdanyow's original comment? I want to prevent users from tabbing of the modal, and restore focus when the modal closes. Will this be added to aurelia-dialog eventually? Or should I build a custom solution?

[plwalters]

@RomkeVdMeulen sorry I missed testing the point of tabbing on to the underlying view - is there a suggested approach for this in the modal that you linked before?

Ok I was lazy just went and looked and they are high jacking the tab key and limiting to items that are in the dialog only. I'll open this back up to make sure we can do the same I don't think this will be a major change but a nice feature. Good catch!

[timfish]

Allowing the user to tab to the underlying page and execute events on it is huge potential source of bugs.

I shouldn't really have to add logic to every control to disable it when dialogs are open!

[jwahyoung]

Putting this here as I was thinking of a custom attribute that would determine a "tabcontext" - basically tabindex the way I think it should have worked originally. This doesn't directly address the "initial focus" issue but it would address the "tabbing to underlying content" issue.

Jedd Ahyoung @jedd-ahyoung 10:06
a custom attribute is just a class, right? meaning it's just a JS object, meaning that it can have a prototype?
if i wanted all custom attributes of a specific type to be aware of each other, would a prototype reference be the best way to handle that, or is there a better, more aurelia-specific way?
i just had an idea for handling tabbing within things like dialogs

Ashley Grant @AshleyGrant 10:10
What do you mean by "be aware of each other?"

Jedd Ahyoung @jedd-ahyoung 10:11
a custom attribute can be placed on any element in the DOM or in a view; when the element is attached, i'd like to register that instance in a central place
so if i had something like and somewhere else i had i would want those attributes to be "aware" of each other so that they can interact
i'm aware that i can probably accomplish that with prototypes, but i'm wondering if there is a different way
(in this case, i'm thinking about basically building tabindex the way it should have been implemented in the first place)

Jedd Ahyoung @jedd-ahyoung 10:17
if the elements could "interact" in this way, i could do something like build a stack of active tabcontexts, and exiting one tabcontext would pop that one from the stack, etc
so in the case of modals, you could open infinite modals without regard for DOM placement for keyboard navigation and tabbing would work correctly
by adding a root tabcontext, we can "invalidate" everything in that context except for what's in the active child tabcontext by setting tabindexes to -1 across the board
so essentially, all of the attributes would at least have to interact with the attribute on the root node

Meirion Hughes @MeirionHughes 10:19
Could you register your own Factory to the DI?
alternatively, just inject a service into the CustomAttribute ctor

Vildan Softic @zewa666 10:20
Or if using rxjs expose an subject and subscribe to that
Or really just classic pub/sub

Jedd Ahyoung @jedd-ahyoung 10:21
the service-based approach does make sense, don't know why i didn't think of that - it is just a class
using events could work as well but i don't think it would scale as well

[jmezach]

Any updates on this? We're still seeing the focus moving off the dialog and onto the underlying content. We would really like to prevent this from happening.

[plwalters]

PR welcome here. Some pseudo-code you could use -

<template>
  <require from="./tab-order-custom-attr"></require>
  <input value.bind="name" tab-order="1" />
  <input value.bind="desc" tab-order="2" />
  <input value.bind="age" tab-order="3" />
</template>

export class TabOrderCustomAttribute {
  static inject = [Element];  
  constructor(element) {
    this.element = element;
  }

  bind() {
    // get all of the elements with a tab-order ex code
    this.tabItems = $(this.element.parentElement).find('[tab-order]`);
    this.element.addEventListener('keypress', (e) => {
      // check if last tabbable item and tab was pressed
      let lastItem = this.tabItems[this.tabItems.length - 1];
      let firstItem = this.tabItems[0];
      if (e.keyCode == 9 && e.target === lastItem) {
        e.preventDefault();
        // focus the first item
        firstItem.focus();
      } else if (e.keyCode == 9 && e.target === firstItem) {
        e.preventDefault();
        // focus the first item
        lastItem.focus();
      }
    });
}

[rokaskuciauskas]

Any news when this will happen?

[timfish]

I've got something working by putting this trap-focus attribute on the parent modal element. It even works with multiple dialogs open as long as all apart from the top one are hidden via css display: none. It currently only traps focus and does nothing to ensure any of the elements inside your dialog get focus initially so you'll need to use it with attach-focus.

<template>
  <div class="modal" trap-focus>
    <input attach-focus value.bind="name" />
    <input value.bind="desc" />
    <input value.bind="age" />
  </div>
</template>

@customAttribute('trap-focus')
@inject(Element)
export class TrapFocus {
  private element: HTMLElement;

  constructor(element: HTMLElement) {
    this.element = element;
  }

  public attached() {
    document.addEventListener('keyup', this.keyPress);
  }

  public detached() {
    document.removeEventListener('keyup', this.keyPress);
  }

  private keyPress = (e: KeyboardEvent) => {
    const key = e.which || e.keyCode;
    if (key === 9 && this.element.offsetParent && !this.element.contains(e.srcElement)) {
      const elements = Array.from(this.element.querySelectorAll('input:not(disabled), textarea:not(disabled), button:not(disabled), [tabindex]'))
        .filter((el: HTMLElement) => el.tabIndex >= 0);

      const next = elements[e.shiftKey ? elements.length - 1 : 0] as HTMLElement;
      next.focus();
    }
  }
}

[dannyBies]

Any updates on this? We're still seeing the focus moving off the dialog. I'm hoping for an official solution.

[RomkeVdMeulen]

@timfish I like your solution, but if I understand this correctly it intercepts every tab press, which might not be the best in terms of a11y (I'm not sure, but I imagine some screenreader software might have other uses for tab that are incompatible with this).

I was thinking we could reverse this approach: once focus enters an element with trap-focus or one of its descendants, set tabindex="-1" on every focusable element outside the trap-focus element. This way we can leave the focus behaviour to the browser and other assistive software, but still prevent focus on elements outside the active dialog. What do you think?

[timfish]

My only concern with that is "every focusable element outside the trap-focus element" could potentially be hundreds of elements which would need to be tracked to return their original tabindex.

If we're going to consider screen readers there are other things too

• aurelia-dialog should be adding aria-hidden="true" to the non-modal content (ie. everything behind).
• When dialog is closed, focus should return to the element which had focus before the dialog was opened

[RomkeVdMeulen]

Complying with all a11y requirements is indeed important, but I suggest we stick to focus considerations for this issue. That does include returning focus to the opening element when the dialog closes, but I'm not sure how to do that with the current structure of the aurelia-dialog plugin.

My only concern with that is "every focusable element outside the trap-focus element" could potentially be hundreds of elements which would need to be tracked to return their original tabindex.

Good point. I've done some searching, but there's a lot of ways to potentially handle this. Here's an example from W3C: it listens for focus events. (See here for demo)

[RomkeVdMeulen]

Here are the WAI-ARIA specifications for dialog keyboard behaviour.

Keyboard Interaction
In the following description, the term tabbable element refers to any element with a tabindex value of zero or greater. Note that values greater than 0 are strongly discouraged.

When a dialog opens, focus moves to an element inside the dialog. See notes below regarding initial focus placement.
Tab:
Moves focus to the next tabbable element inside the dialog.
If focus is on the last tabbable element inside the dialog, moves focus to the first tabbable element > inside the dialog.
Shift + Tab:
Moves focus to the previous tabbable element inside the dialog.
If focus is on the first tabbable element inside the dialog, moves focus to the last tabbable element inside the dialog.
Escape: Closes the dialog.

[RomkeVdMeulen]

This SO answer may also be helpful

[timfish]

My implementation will probably also stop users from tabbing to the browsers search/URL entry and other UI which is not really desirable. My app is currently only running in Electron where this is not an issue.

[dannyBies]

Is it a good idea to use the implementation @timfish provided while waiting for official support? I'd like to use an official implementation instead of this one so we don't have to support this piece of code for our dialogs.

[timfish]

My testing of these demos in Chrome 64 seem to suggest that the HTML dialog element and showModal() seem the handle all the focus issues. No doubt they have better accessibility too. For this reason I suggest we migrate the dialog renderer to use the HTML dialog element.

EDIT
Also tested showModal() in Safari and it limits focus so the polyfill appears to be doing its job.

EDIT 2
If we don't use <dialog> elements in aurelia-dialog it's probably worth copying how Google implemented focus trapping in dialog-polyfill.

[RomkeVdMeulen]

The native dialog tag comes with one caveat: it does not return focus when the dialog closes. See #375.