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

consider implementing "autofocus" #136

Closed
jdanyow opened this issue Jun 24, 2015 · 10 comments
Closed

consider implementing "autofocus" #136

jdanyow opened this issue Jun 24, 2015 · 10 comments

Comments

@jdanyow
Copy link
Contributor

jdanyow commented Jun 24, 2015

The Durandal dialog plugin had a feature where it would find the first .autofocus element and focus it. Code for this is here.

In Durandal, this behavior didn't extend to standard (non-modal) routing scenarios but it could easily be added by doing something like this:

router.on('router:navigation:composition-complete')
  .then((instance, instruction, router) => {
    // set the focus to the .autofocus:first:enabled element...
  });

Should we consider adding this functionality to Aurelia's router?

Another possibly relevant link: https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Forms_in_HTML#The_autofocus_attribute

@bryanrsmith
Copy link
Contributor

Sounds good to me! I think we can add this pretty trivially to app-router. Will we need to do anything to make sure we don't clobber any focus attempts made by the new autofocus custom attribute that was contributed a few weeks back?

@jods4
Copy link
Contributor

jods4 commented Aug 28, 2015

I'm not sure about that. Why hardcode this inside the router when writing an auto-focus custom attribute is trivial (well mostly call focus() in attached())?

It's too bad the HTML5 spec only gives focus to the first autofocus element added to the DOM. That doesn't play nice with SPA.

@bryanrsmith
Copy link
Contributor

@EisenbergEffect Thoughts on this? I think I agree with @jods4. If we add support to the router then we probably need to add an API for disabling it. I'm not sure it's worth the complication.

@jdanyow
Copy link
Contributor Author

jdanyow commented Oct 29, 2015

Couple thoughts:

  1. I don't think there's a need for an option, the inclusion of the autofocus class would be considered "opting in" to the behavior.
  2. An autofocus custom attribute is a bad idea because it prevents you from composing multiple items that have autofocus attributes. They will steal the focus from each other. IMHO It's better to tie the focus assignment logic to navigation, perhaps throught a NavigationFocus class that people can override, etc. This would enable reuse in the aurelia dialog.

@jods4
Copy link
Contributor

jods4 commented Oct 29, 2015

An autofocus custom attribute is a bad idea because it prevents you from composing multiple items that have autofocus attributes.

I am not sure I see the alternative. Inserting multiple autofocus elements at the same time would indeed be a race (last one attached wins), but then what do you expect if you design your application with multiple autofocus at the same time?

Although I never encountered this problem in practice (I use my own autofocus custom attribute) I wonder what alternative design would solve the issue. If you insert multiple elements with class=autofocus at the same time, you still have a kind of undefined behaviour, don't you? I mean: in the end only one can receive focus...
Sure there could be a rule in a FocusManager that says which one gets focus, yet it will seem arbitrary to the developer who faces an unexpected situation with multiple autofocus.

Do you have use cases in mind where things would get hairy? My typical use of autofocus is give focus to the first/main field of a dialog box or a form. Those cases are rather basic. There's only one autofocus per view and the behavior is obvious.

@jdanyow
Copy link
Contributor Author

jdanyow commented Oct 29, 2015

@jods4 take a look at the original post for this issue. It describes how durandal used the .autofocus class. It's also mentioned in the durandal docs. The logic is simple, focus the first element with the autofocus class.

This is an important and extremely common scenario that I feel should be covered or at least documented in some way. Having a prescribed focus strategy will help developers make their apps more accessible and help devs avoid pitfalls like the race conditions that can arise with the custom attribute approach.

Another scenario is when using aurelia-dialog, when the modal is popped the focus remains on the original screen and you can tab off the modal and onto the page below. The bootstrap modal enforces a rule where the modal gets the focus when it's popped and users cannot tab off the modal into the background. Come to think of it, we should probably make whatever we do here (if anything) consistent with what is done here: aurelia/dialog#1

If there's no appetite to add this behavior to aurelia I can look into providing it with a plugin.

@jods4
Copy link
Contributor

jods4 commented Oct 29, 2015

I agree with you that it's important. I think implementation-wise I just don't see what's so bad with the autofocus custom attribute. To try to bring something new to the table, here are a few additional considerations:

  • an autofocus custom attribute that would focus when attached() would mirror closely the HTML5 autofocus attribute behavior. I think it's really unfortunate that the HTML5 attribute only works for the first autofocus element added to the DOM (so great for page loads, bad for SPA). I think it should work every time and hopefully they will revisit that with the <dialog> specs, etc.
  • I think it is important that the autofocus feature -- no matter how it's done -- works in every scenario. That means when a dialog pops up but also when a view is navigated to, or even simply when a small non-modal popup control appears (e.g. after the user clicks on something)! It seems to me that delegating the responsability of focusing (e.g. based on css class) to some "manager" class would narrow the scenarios were autofocusing could actually work (e.g. only when using aurelia's router navigation or built-in dialogs). If we can keep templating/navigation distinct from focus management that's better separation of concerns IMHO.
  • Also if you start imagining complex scenarios, I am unclear how a .autofocus selector would distinguish between previously existing elements and elements just added to the DOM. I think that if we start talking crazy stuff there is potential for confusion as well.
  • Speaking about crazy stuff, I have had no problem with the autofocus custom attribute. Do you have any use case where it was trouble (because of several elements with autofocus or otherwise)? I don't really "get" the issue at hand.

Now, the thing about preventing tabbing off is a totally different issue I think. And probably an important one. Let's be honest: in today's browsers tab control is not good. Escaping a "modal" dialog is most of the time possible and a bad thing.
I think it's worthy having some kind of FocusManager or something like that in Aurelia but I don't really have an opinion about how it should be done. All I can say is that it may not be trivial:

  • there are standard specs coming to enable "truly" modal dialogs in HTML. Can we align?
  • a truly robust tab control solution is very complex. It should handle cycling in sub-containers, restoring focus to the same element when going back to a previous focus "container" etc.

@bryanrsmith
Copy link
Contributor

I agree that we shouldn't implement something that may not be flexible enough. In my own apps I prefer not to return the Promise from activate() because I think it makes the app feel more responsive. In that situation, it's not possible for the router to correctly handle autofocus.

The router also has no DOM references right now, and I'd love to keep it that way :-)

@jdanyow Would you be happy to push this feature into a plugin?

@jdanyow
Copy link
Contributor Author

jdanyow commented Oct 30, 2015

good points- closing

@jdanyow jdanyow closed this as completed Oct 30, 2015
@jods4
Copy link
Contributor

jods4 commented Nov 3, 2015

For the sake of use-cases that could be interesting tests:

Today I built a dynamic list in a form, with a complex template and an add button. The list itself is simply a repeat.for. When I add a new item, I want that the first field of the newly inserted template to get focus (so that the form is totally usable with keyboard).

For this I used an autofocus custom attribute that .focus() on attach and it worked nicely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants