ibm-js / delite

HTML Custom Element / Widget infrastructure
http://ibm-js.github.io/delite/
Other
68 stars 28 forks source link

delite/HasDropDown: opt-out from focusing the dropdown open by keyboard #373

Closed AdrianVasiliu closed 9 years ago

AdrianVasiliu commented 9 years ago

HasDropDown allows the host widget to opt-out from focusing the dropdown, thanks to the flag dropDown.focusOnOpen. However, this flag is currently bypassed when opening the dropdown using the keyboard - see https://github.com/ibm-js/delite/blob/0.6.0-beta.2/HasDropDown.js#L250-257.

What is the deep reason behind bypassing focusOnOpen for keyboard interaction? Could there be a way for a host widget to opt-out including for keyboard interaction?

That is, the change in HasDropDown could either consist in removing the keyboard flag currently used to enforce the call of dropDown.focus() even if dropDown.focusOnOpen is false, or in introducing yet another flag (as focusOnOpen) allowing to opt-out including for keyboard.

The concrete use-case is for a host widget (Combobox) using a dropdown which is a subclass of KeyNav (such as List). For this use-case, delite/KeyNav has been enriched with the property focusDescendants (https://github.com/ibm-js/delite/blob/0.6.0-beta.2/KeyNav.js#L72-100), such that the host widget can opt-out from focusing descendants, preserving the focus on an external element (the input element). Consistently it seems HasDropDown should also allow to opt-out from the call of dropDown.focus().

wkeese commented 9 years ago

What is the deep reason behind bypassing focusOnOpen for keyboard interaction?

It's to make the software accessible. Usually, the dropdown needs to be focused so that the (keyboard only) user can interact with it, and the user cannot manually focus it via the keyboard. Therefore it needs to be focused automatically. Also, a blind user likely won't even realize there a popup has opened unless it's focused automatically and then the screen reader announces it.

The concrete use-case is for a host widget (Combobox) using a dropdown which is a subclass of KeyNav (such as List). For this use-case, delite/KeyNav has been enriched with the property focusDescendants (https://github.com/ibm-js/delite/blob/0.6.0-beta.2/KeyNav.js#L72-100), such that the host widget can opt-out from focusing descendants, preserving the focus on an external element (the input element).

OK, yes that is a special case. It's similar to dijit/form/ComboBox. For dijit, _ComboboxMenu doesn't have a focus() method so it doesn't get focused, even by keyboard. Similarly, you could probably remove the focus method from your dropdown list as a workaround, but it doesn't seem like a proper solution.

I can't think of any solutions right now for delite besides introducing another flag, and even then, I'm not sure what the new and old flags should be called. Perhaps something like focusOnPointerOpen (for mouse and touch) and focusOnKeyboardOpen for keyboard.

AdrianVasiliu commented 9 years ago

It's to make the software accessible. Usually, the dropdown needs to be focused

Sure, I know it usually needs it, my only question was why does it always need it for keyboard interaction, regardless of the focusOnOpen flag. But okay, I understand there are use-cases where the host widget sets focusOnOpen to false while still needing focus on keyboard opening.

I can't think of any solutions right now for delite besides introducing another flag, and even then, I'm not sure what the new and old flags should be called. Perhaps something like focusOnPointerOpen (for mouse and touch) and focusOnKeyboardOpen for keyboard.

These sound as reasonable flag names for me. Alternatively, maybe HasDropDown could rely on the existing focusDescendants flag of the dropdown, however this flag only holds for dropdowns extending KeyNav, and semantically it rather refers to chldren of the dropdown, not to the dropdown itself. So both solutions would do the trick for my case but I'm not sure which one would be the cleanest. Anyway, just let me know if you pick one or another solution for HasDropDown, or whether I need to use an ugly workaround in Combobox (such as removing the focus() method from the List instance or overridding the private _focusDropDownOnOpen() inherited from HasDropDown).

wkeese commented 9 years ago

> Sure, I know it usually needs it, my only question was why does it always need it for keyboard interaction, regardless of the focusOnOpen flag

Oh, the reason it works that way was specifically for drop down menus like on http://download.dojotoolkit.org/release-1.10.2/dojo-release-1.10.2/dijit/tests/test_Menu.html. The idea was that the normal way menus work is that the first menu item get focused only when the menu is opened via the keyboard.

These sound as reasonable flag names for me. Alternatively, maybe HasDropDown could rely on the existing focusDescendants flag of the dropdown, however this flag only holds for dropdowns extending KeyNav, and semantically it rather refers to chldren of the dropdown, not to the dropdown itself.

Agreed, either one is OK. I guess I'll do the two flags. We've already cut our 0.6 beta, but if this useful for you for the 0.6 release, I can slip it in. Let me know.

AdrianVasiliu commented 9 years ago

The idea was [...]

Okay, thanks for the explanation.

if this useful for you for the 0.6 release, I can slip it in. Let me know.

Great, thanks! Now, I'm still fighting with other obstancles for the keyboard navigation of Combobox, and I might have other changes in KeyNav or HasDropDown to ask... unless I find other solutions. Supposing that you'd accept these other potential changes, to avoid releasing once again, I'd say let's count on a delite release tomorrow, not right now.

AdrianVasiliu commented 9 years ago

I'd say let's count on a delite release tomorrow, not right now.

Just to clarify, I can wait for the release tomorrow, but it would be nice that the commit is pushed today, such that I can adapt my code accordingly already (for my upcomming PR).

AdrianVasiliu commented 9 years ago

I'm still fighting with other obstancles for the keyboard navigation of Combobox, and I might have other changes in KeyNav or HasDropDown to ask... unless I find other solutions.

I eventually found other solutions for all remaining obstacles. You can push and re-release whenever you want/can, from my point of view.

wkeese commented 9 years ago

OK, does "all remaining obstacles" include this one?

I didn't have time to work on this today, but I did look at the HasDropDown.html test, which tests for your use case (to never focus the dropdown) by setting focus: null on the dropdown widget.

That doesn't seem ideal, but it's not much worse than setting focusOnOpen or focusOnKeyboardOpen on the dropdown widget either.

I'm wondering now that perhaps these flags should be on HasDropDown mixin rather than the widget used as a dropdown.

The reason the flag is on the dropdown in dijit is so that an app can declare <div dojoType=dijit/form/DropDownButton><div dojoType=dijit/Menu></div></div> or alternately <div dojoType=dijit/form/DropDownButton><div dojoType=dijit/TooltipDialog></div></div>, and it automatically does the right thing. But that doesn't work well for your case. Have to think about that more.

Also, on a more minor note, instead of having two flags, I could keep a single focusOnOpen flag, but instead of being boolean, have string values of "keyboard", "always", or "never". Not sure if that's better or worse.

AdrianVasiliu commented 9 years ago

does "all remaining obstacles" include this one?

No, it doesn't ;-) I meant I was afraid I might have other blocker (for me) issues in addition to this one, but finally this is not the case.

I'm wondering now that perhaps these flags should be on HasDropDown mixin rather than the widget used as a dropdown.

Yes, I was also a bit surprised that it isn't on HasDropDown.

I could keep a single focusOnOpen flag, but instead of being boolean, have string values of "keyboard", "always", or "never". Not sure if that's better or worse.

Well, unless we forsee that we might need even more fine-grain flags, I'd personally favor the two flags solution, just because the names are more self-explanatory. But no dramatic difference in my eyes either.

Have to think about that more.

Understood. Honestly, my short term objective is... anything that allows me to release the keyboard navigation of Combobox. So if there's no quick solution in delite, my only option is to go with your suggested workaround (although you said it doesn't seem like a proper solution) for now. I guess you'll not object to my PR because of it ;-)

AdrianVasiliu commented 9 years ago

Thanks @wkeese. So I'll move to using these new APIs in Combobox when they'll be released.