Backspace doesn't work in input fields when a Fancytree with tabbable = false is active

[Koloto]

How to reproduce

jsFiddle to demonstrate: http://jsfiddle.net/688Vy/5/

NOTE: tabbable: false doesn't work with 'table' extension. But if you manually remove tabIndex attribute, you can reproduce this bug with 'table' extension too.

How to fix

I think it's wrong to bind keydown event to the document in _bind method. It's a great overhead in any case. You have to bind tree.$container instead. In order to tree.$container can get focus you can insert any DOM element with tabIndex="0" and style="width:0;height:0" inside the $container. I experimented with 'table' extension, it's enough to add the second empty 'tbody' element with tabIndex="0":

<table>
    <thead><tr><th/><th/><tr></thread>
    <tbody><tbody> <!-- main tbody -->
    <tbody tabindex="0"></tbody> <!-- second tbody for getting focus -->        
</table>

This trick should work with the "classic" tree presentation too (you can insert any div with style="width:0;height:0").
Also in this case no need to check !tree.hasFocus() in the 'keydown' handler:

tree.$container.bind("keydown" + ns, function(event){
    // some code
    // WAS: if(opts.disabled || opts.keyboard === false || !tree.hasFocus()) {
    if(opts.disabled || opts.keyboard === false) {
        return true;
    }
    // other code
});

[Koloto]

Martin, any chances to fix this issue in the near future? If you think my way is correct, I can try to make a pull request. What do you think?

[mar10]

Hi,

help is always very much appreciated ;-) If you like, you could start a branch / a pull request.
I don't think I will find time to handle this in the very near future.

Some nice to haves

• I would prefer a html-compliant solution (are multiple <tbody> tags valid?).
• It should not break tabbable=true mode.
• it should work, if the nodes contain tabbable elemts (like <a> or <input>)

These alternatives come to my mind:

• add a dummy element with a tabindex (your proposal)
• make tree.hasFocus() smarter, i.e. record if the tree is focused in tabbable=false mode. Then we could return false in the keyboard handler for inactive trees, which should solve the issue.
• improve 'tabbable=true' behavior (why do one want to disable it)?

What do you think? Other ideas?

p.s.
(I fixed the 'tabbable' issue for tables)

[Koloto]

Some comments:

I would prefer a html-compliant solution (are multiple <tbody> tags valid?)

Yes, multiple tbodies are valid. And div or span inside table aren't valid.

it should work, if the nodes contain tabbable elemts (like <a> or <input>)

I tried to insert <input> elements into the tree - neither backspace nor arrows don't work for them now (even when tabbable = true). I think we should handle keyup or keypress instead of keydown event.

make tree.hasFocus() smarter, i.e. record if the tree is focused in tabbable=false mode.

I don't think It's a good idea. A browser already has own focus, it's enough. And as I wrote earlier, binding to the document element has a great performance overhead. Binding to the concrete $container will work faster.

improve 'tabbable=true' behavior (why do one want to disable it)?

What do you mean? I think it would be great to improve 'tabbable=false' mode. Individual nodes can receive focus instead of a whole tree. 'Tab' can move focus to the next node like 'Down' key (and 'Shift+Tab' - to the previous node like 'Up' key). If focus is inside a nested element (e.g. <input>) 'Tab' will move it to the next node.
It may be 'tabbable=false' default behavior or we can introduce the third mode (tabbable="nodes"?). To implement this we can add the tabIndex attribute to node elements (li or tr for table) or just handle 'Tab' in the keydown/keyup handler. What do you think?

[mar10]

Yes, multiple tbodies are valid. And div or span inside table aren't valid.

I see now that multiple <tbody> are allowed, but - if I read the spec correctly - must have at least one <tr>.

Anyway: no big deal, if it works on all supported browsers.

I tried to insert <input> elements into the tree - neither backspace nor arrows don't work for them now (even when tabbable = true). I think we should handle keyup or keypress instead of keydown event.

At least we should try with inserted <a> tags also.

Btw, Older implementations of Dynatree used <a> tags for the nodes, which can get focus by default, so key handling was easy. But they led to some problems (e.g. IE scrolls them into view when clicked sometimes, ...)

I don't think It's a good idea. A browser already has own focus, it's enough. And as I wrote earlier, binding to the document element has a great performance overhead. Binding to the concrete $container will work faster.

I havn't seen a great performance overhead so far when binding to document, at least nothing a user would notice.

Make tree.hasFocus() smarter only means that $.ui.fancytree.focusTree must be set correctly to a tree instance or 'null', so I still think it might be a valid alternative.

Some comments: ...

What I meant is: If we have a solution that always works nicely, we might as well remove the 'tabbable' option at all.

The 'tabbable: "nodes"' option sounds interesting too. When I experimented with WAI-ARIA support, I saw that this might be a better approach, but did not try yet;
https://github.com/mar10/fancytree/wiki/SpecAria

I think we can give your approach a try,

It would be great if we have a working example that handles two tree instances with surrounding and embedded input controls.

If we need to make big changes, then I would like to be able to switch between both versions for a while.
We could start a feature branch for it (or even implement it as an extension)?

[Koloto]

I arrived at a conclusion that there is no need to support keyboard navigation in 'tabbable = false' mode. How can you move focus to the tree by keyboard in that mode? I suppose you can't. So 'keyboard' option must be equal to false when 'tabbable == false'. What do you think?
It will simplify the work because it doesn't need any tricks (like inserting a dummy element or tracking focused tree).

[mar10]

keyboard: true should be used to control if standard keyboard navigation is enabled.
This assumes that the tree can be made 'keyboard-active' in some way, in case you have other trees or controls on the page. (Not necessarily the browsers system ':focus'. Maybe by adding a 'focus' class to the container or maintaining a global 'focusTree' variable).

The tabbable: true option was added to control the behavior when the tree is part of a web form.
If you TAB through a form with a tree and other controls, the tree should behave like a listbox, i.e.

• be entered / exited using TAB, i.e. be part of the tab index chain
• set focus to the active node when entered using TAB
• activate first node when entered using TAB and no node was active
• gray our active node when exited

If we have an implementation for keyboard: true that also satisfies these requirements (and makes all users happy), we can remove the tabbable option.

We should also consider that later we want to support

• WAI-ARIA compliance
• embedded HTML elements in the nodes like <a> and <input>
• the table extension (nice to have)

[Koloto]

Not necessarily the browsers system ':focus'. Maybe by adding a 'focus' class to the container or maintaining a global 'focusTree' variable

Why do you want to control keyboard navigation for the not-focused tree? I can't imagine when it can be useful. But if it's really necessary, my approach with binding keydown to $container is incorrect of course.

[mar10]

I did not say that I want to avoid to set the system focus. I wanted to express that I don't care how it is done, as long as the solution works for the users ;-)

I will start a Wiki page for discussion, requirements and specification of different aproaches this weekend, ok?

[mar10]

Ok, I started a q'n'd Wiki Page here:
https://github.com/mar10/fancytree/wiki/SpecKeyboard

It's only a first draft and everything is open. Should be public editable, so feel free to add or improve.

[Koloto]

I created the pull request, please verify it. If it's ok, I'll do the same for the table extension soon.
P.S. Sorry for a long delay, I hope I'll be able to work a bit faster now:)

[mar10]

Looks good, so far! (Although there is sometimes a system focus frame around whole expanded LI tags, at least on Safari).

Currently I am not even sure how the Interface should work. I think we should describe and discuss the interface from a users perspective in the Wiki, so we have something to test against. Could you add your ideas there?

I am currently working on styles an markup for the table extension, so maybe there will be some changes soon.
(I need a to implement a hierarchical checkbox grid-like for a project, so I guess I will get some feedback from the team too.)

[mar10]

FYI I just merged latest changes from master. Hope everything is still working.
I think the only thing that might affect your implementation is that I renamed the container class fancytree-focused to fancytree-treefocus (on the container only). Other changes include CSS and ext-table markup.

[mar10]

i have created another branch (gridnav) for some testing...

[mar10]

The gridnav branch now implements the ext-gridnav extension and a sample that includes a table (Tweaks > Keyboard nag.).
It's a combination of the tubbable branch and a new gridnav extension (When everything works we might drop the extensions and merge everything into core / ext-table).
I'd be interested in your thoughts...

[mar10]

Ich merged our changes back to master. Let me know if you have improvements or suggestions...

[Koloto]

Looks good! Special thanks for the removal of global FT.focusTree and binding to the document:)

Some suggestions:

• :input selector is used many times, but tag <a> also can get focus. I think we need to test a support <a> tag.
• I tried gridnav extension for a classic tree (without table-ext) - it seems it works fine except of handleCursorKeys option. Using tag <td> is hard coded for that option. Why? Maybe rename findNeighbourTd to findNeighbourElement and define it as a method of Fancytree? And table-ext overrides it.
• RIGHT key doesn't expand a node with table-ext. You have to press DOWN key next to expand it.
• redundant $(document).unbind(ns) in _unbind method

Anyway, great work in general! Thanks.

[mar10]

Thanks for your feedback and contribution to that topic. I added a new issue, so I can close this bug for now