Tree: reuse key navigation code in _KeyNavContainer

[bill]

[29912] essentially duplicated the letter key navigation code from Tree into _KeyNavContainer. Reuse that code from Tree to remove the duplication.

[bill]

In [29979]:

Some general cleanup of _KeyNavContainer. Also modifying Menu to not access this.focusedChild when the MenuItem of interest is already in a local variable. Refs #16323 !strict.

[bill]

In [29985]:

Split _KeyNavContainer code related to key navigation into _KeyNavMixin. Refs #16323 !strict.

[bill]

In [29998]:

Update _KeyNavMixin to use dojo/on rather than deprecated dojo/connect. On the downside though, it splits _onContainerKeypress() into two functions, _onContainerKeypress() and _onContainerKeydown(), each receiving a native event without evt.charOrCode set, so may cause issues for subclasses overriding these methods, or using this._keyNavCodes to hold printable keys. Seems ENTER and SPACE and the non-printables like arrows still work fine though.

Make _KeyNav.focusFirstChild() and _KeyNav.focusLastChild() abstract methods, rather than requiring an overloaded API for _getNextFocusableChild(). However, _KeyNavContainer._getNextFocusableChild() still functions to get the first/last child.

Refs #16323 !strict.

[bill]

In [30029]:

Refactor Tree to extend _KeyNavMixin. The only behavioral change is that when you tab into the Tree, it always goes to the first node, rather than the previously focused node. Some changes will affect people who are extending Tree though, if they are either modifying the TreeNode template or overriding non-public methods. Incidentally fixes some bugs not obeying Tree.tabIndex (i.e. just assuming the tabIndex is 0).

Also moved focusNext() and focusPrev() from _KeyNavMixin to _KeyNavContainer, thus simplifying signature of _KeyNavMixin._getNextFocusableChild().

Fixes #16323 !strict.

[bill]

In [30045]:

Make _KeyNavMixin base class, rather than _KeyNavContainer and Tree, track child node focus/blur events and update this.focusedChild accordingly. To support this, the subclass needs to define a selector (this.childSelector) to identify focusable children (ex: .dijitTreeRow).

This has two advantages for _KeyNavContainer:

1. uses event delegation, which is more efficient and more amenable to server side instantiation, etc.
2. removes undocumented requirement that child widgets extend _FocusMixin

Refs #16323 !strict.

[bill]

In [30068]:

Use keydown rather than keypress to fix arrow key navigation through ComboButtons in a Toolbar on FF. Fixes regression from [29998], refs #16323 !strict

[bill]

In [30082]:

Fix race condition that was affecting IE10 where the tabIndex on visited _KeyNavMixin children wasn't getting reset to -1. The problem was happening on IE10 because the focus event occurs asynchronously, after the focusChild() call completes. Refs #16323 !strict.

[bill]

In [30253]:

_KeyNavMixin depends on _onBlur(), so it needs to mixin _FocusMixin. Refs #16323 !strict.

[bill]

In [30254]:

Remove console.log()'s accidentally added in [30253], refs #16323 !strict.

[Douglas Hays]

Starting with r30029, Tree TAB navigation isn't correct. Using test_Tree.html, focus China, then tab out and then back into the Tree widget and China is no longer focused.

[bill]

As explained in the checkin comment to [30029]:

The only behavioral change is that when you tab into the Tree, it always goes to the first node, rather than the previously focused node.

So, this is expected behavior, is consistent with the way widgets like Toolbar and MenuBar have always worked, and actually fixes a few bugs (see #6236), such as when the China tree item is deleted, or the Asia tree item is closed, making China unfocusable.

I don't see how this new behavior can be considered incorrect.

[Douglas Hays]

I think 1.8 is compliant with WAI-ARIA standards and trunk is not:
​http://www.w3.org/WAI/PF/aria-practices/#Site_Navigator_Tree
Focus: When the tree receives focus, the item in the tree that is marked as currently displayed should receive focus.
I think this should be fixed.

[bill]

Hmm, well that's about a "Site_Navigator_Tree", which is why it's talking about "the item.. that is marked as currently displayed", a concept which has no meaning to a plain tree.

Also, if a Tree were used to implement site navigation, then according to that spec, shouldn't tabbing into the Tree send focus to the currently selected node, rather than the most recently focused node? They aren't necessarily the same because the user might have used the arrow keys to change the focus TreeNode without ever pressing ENTER to select it.

PS: Assuming an app had a tree where one item is always selected (as opposed to no selected items, or multiple selected items), they could easily change Tree.focus() to focus that item. Not sure if the logic belongs in Tree itself.

[Becky Gibson]

Thinking about this from a keyboard user's standpoint, I'd be really annoyed if I navigated to the middle of a tree and then tabbed out to go do something else and when I tabbed back into the tree I had to re-navigate to the middle of the tree again to get back to where I was when I left. Also, if you look at ​http://www.w3.org/WAI/PF/aria-practices/#TreeView it is about a hierarchical list. It has a similar requirement to the Site Navigator tree: "The last visited node in the tree control is retained in the tab order when the user navigates away from the tree control."

Yes, there could be situations where the focused node could be deleted. But, deleting the node should go through some sort of code within the tree control and if the node being deleted has a tabindex==1, then the focus gets set to the first node in the tree. The same would go for closing a node - check that none of its children has focus. I thought these behaviors were implemented in the tree control at one point.

[davidtodd1]

I agree with the comments about returning a user to a previously focused node when re-entering a tree. It's epically important when a tree contains many nodes. Returning a user to the first node would be a serious usability concern.

[bill]

Is there something special about Tree, besides likely having more items? As I said before, Toolbar and MenuBar focus the first child too, and no one has complained about them... plus a Menu on the side of the screen is very similar to a site navigation tree, except I guess without the concept of a selected item.

Also, as I said above, both ​http://www.w3.org/WAI/PF/aria-practices/#TreeView and ​http://www.w3.org/WAI/PF/aria-practices/#ListBox suggest to focus the first selected node, rather than the previously focused node, and that's also what TabContainer does. Tree and Menu are a little bit different since there's usually no selected node, although Trees do have that ability.

[Douglas Hays]

Windows Explorer has a tree of directories on the left and a list files on the right and you can tab back and forth, and navigate with arrow keys the same as with the Tree widget. It would be bad if it resetting the left side list of directories whenever you tab back. I also checked Rational Team Concert which uses tree constructs for nearly everything and it always focuses the previously focused item when you tab back and forth. I think trees are considered different because it can take considerable effort to navigate to a specific place within a large set of nodes and because it's common for another pane to render based on the current Tree context. I think this should block 1.9 alpha.

[bill]

Replying to doughays:

Windows Explorer has a tree of directories on the left

Actually, Windows Explorer's tree is an example of a Site_Navigator_Tree, and like I mentioned in comment:15 and comment:18, tabbing into the tree goes to the selected node, not the most recently focused node as you are requesting. You can confirm this by typing a new path in the "Address" bar, for example "C:\", and then tabbing into the tree.

Having said that, I agree that remembering the last focused node is useful for a Tree without a selection, and other widgets like Grid, so I'll add that if you want.

[bill]

In [30601]:

When tabbing into Tree, go to last focused node, refs #16323 !strict.

[bill]

In [30896]:

Remove _onKeyDown listener from DropDownMenu and MenuBar, and instead leverage code in _KeyNavMixin. Refs #16323 !strict.

[bill]

In [31175]:

Mark connectKeyNavHandlers() as deprecated, refs #16323 !strict.

[Bill Keese <bill@…>]

In f1878c88cc3ec71155c9447bee29587886628568/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Bill Keese <bill@…>]

In 66dc3ed198375bfbb5e0a1922efa0f730ea26d32/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled?