Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Gh1994 keyboard navigation of open empty folder in tree should work as expected #2000

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e8f18cf
(GH1994) adds stub of unit test for reported bug
Jun 21, 2017
ee22a86
(GH1994) changes variable name for clarity
Jun 22, 2017
2f7d4c7
(GH1994) tests down arrow press between siblings
Jun 22, 2017
39055be
(GH1994) removes accidentally left in console statment
Jun 22, 2017
70e781c
(GH1994) breaks data out into separate files
Jun 22, 2017
12a4478
(GH1994) adds example of a tree that consumes a static JSON array
Jun 23, 2017
558fd6c
(GH1994) tests down arrow keypress on open empty folder
Jun 23, 2017
9943e3b
(GH1994) merges static data tree changes
Jun 24, 2017
f4f6631
(GH1994) removed recently added unreleased now redundant populate.fu.…
Jun 24, 2017
2653f66
(GH1994) listen for initialized instead of loaded
Jun 24, 2017
d4b1bda
(GH1994) cleans up down key module tests a bit
Jun 24, 2017
c8ef917
(GH1994) cleans up staticDataTree.html a bit. Can be used for testing…
Jun 24, 2017
26e60eb
(GH1994) makes up and down arrows move over open empty branches as ex…
Jun 24, 2017
cd82ee2
(GH1994) adds unit tests for empty branch keyboard nav on tree
Jun 24, 2017
a18d24d
Merge branch 'master' into GH1994---keyboard-navigation-of-open-empty…
Jun 27, 2017
445452e
Merge remote-tracking branch 'origin/master' into GH1994---keyboard-n…
Jun 27, 2017
c956e9e
Merge branch 'master' of github.com:ExactTarget/fuelux into GH1994---…
Jun 27, 2017
7f6730a
Merge branch 'GH1994---keyboard-navigation-of-open-empty-folder-in-tr…
Jun 27, 2017
9f323ff
(GH1994) merges latest from master
Jun 28, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions dev/staticDataTree.html
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@

<body>
<section id="tree">
<h2>Tree</h2>
<h2>Static Data Tree</h2>
<div class="thin-box">
<!-- Utilizes Handlebars templates (see index.js) -->
<h3>folders selectable (please note structure of treebranch)</h3>
<p>This file is useful for testing static data tree & keyboard navigation on tree</p>
<div id="myTreeWrapper"></div>
</div>
</section>
Expand Down Expand Up @@ -127,6 +127,14 @@ <h3>folders selectable (please note structure of treebranch)</h3>
}
]
},
{
name: 'Empty Folder',
type: 'folder',
attr: {
id: 'emptyFolder'
},
children: []
},
{
name: 'Folder Sibling',
type: 'item',
Expand Down
36 changes: 17 additions & 19 deletions js/tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@
$loader.removeClass('hide hidden'); // jQuery deprecated hide in 3.0. Use hidden instead. Leaving hide here to support previous markup
}


this.options.dataSource(treeData ? treeData : {}, function populateNodes (items) {
$.each(items.data, function buildNode (i, treeNode) {
var nodeType = treeNode.type;
Expand Down Expand Up @@ -223,12 +222,8 @@
});

$parent.find('.tree-loader').addClass('hidden');

// return newly populated folder
self.$element.trigger('loaded.fu.tree', $parent);
// loaded.fu.tree was getting swallowed up somehow and was not triggering my listener when writing
// accessibility unit tests. Added populated.fu.tree so that I could listen for it instead.
self.$element.trigger('populated.fu.tree', $parent);
});
},

Expand Down Expand Up @@ -296,7 +291,7 @@

// add the children to the folder
if (!$treeFolderContent.children().length) {
$tree.one('populated.fu.tree', disclosedCompleted);
$tree.one('loaded.fu.tree', disclosedCompleted);
this.populate($treeFolderContent);
} else {
disclosedCompleted();
Expand Down Expand Up @@ -585,7 +580,7 @@
// the navigateTree method if there is no (fake) promise, but will be fired by an event listener that will
// be triggered by another function if necessary. This way when done runs, and fires keyboardNavigated.fu.tree
// anything listening for that event can be sure that everything tied to that event is actually completed.
var noPromise = true;
var fireDoneImmediately = true;
var done = function done () {
$tree.trigger('keyboardNavigated.fu.tree', e, $targetNode);
};
Expand All @@ -601,13 +596,13 @@
var isItem = $targetNode.hasClass('tree-item');
// var isOverflow = $targetNode.hasClass('tree-overflow');

noPromise = false;
fireDoneImmediately = false;
if (isFolder) {
if (foldersSelectable) {
$tree.one('selected.fu.tree deselected.fu.tree', done);
$tree.tree('selectFolder', $targetNode.find('.tree-branch-header')[0]);
} else {
$tree.one('populated.fu.tree closed.fu.tree', done);
$tree.one('loaded.fu.tree closed.fu.tree', done);
$tree.tree('toggleFolder', $targetNode.find('.tree-branch-header')[0]);
}
} else if (isItem) {
Expand Down Expand Up @@ -642,7 +637,7 @@
break;
case 37: // left
if (isOpen) {
noPromise = false;
fireDoneImmediately = false;
$tree.one('closed.fu.tree', done);
$tree.tree('closeFolder', targetNode);
} else {
Expand All @@ -658,9 +653,13 @@
// move to previous li not hidden
$prev = $($targetNode.prevAll().not('.hidden')[0]);

// if the previous li is open, move to its last child so selection appears to move to the next "thing" up
// if the previous li is open, and has children, move selection to its last child so selection
// appears to move to the next "thing" up
if ($prev.hasClass('tree-open')) {
$prev = $($prev.find('li:not(".hidden"):last')[0]);
var $prevChildren = $prev.find('li:not(".hidden"):last');
if ($prevChildren.length > 0) {
$prev = $($prevChildren[0]);
}
}

// if nothing has been selected, we are presumably at the top of an open li, select the immediate parent
Expand All @@ -676,7 +675,7 @@
if (isOpen) {
focusIn($tree, $targetNode);
} else {
noPromise = false;
fireDoneImmediately = false;
$tree.one('disclosed.fu.tree', done);
$tree.tree('discloseFolder', targetNode);
}
Expand All @@ -686,10 +685,8 @@

case 40: // down
// move focus to next selectable tree node
var $next = [];
if (isOpen) {
$next = $($targetNode.find('li:not(".hidden"):first')[0]);
} else {
var $next = $($targetNode.find('li:not(".hidden"):first')[0]);
if (!isOpen || $next.length <= 0) {
$next = $($targetNode.nextAll().not('.hidden')[0]);
}

Expand All @@ -710,7 +707,7 @@
if (handled) {
e.preventDefault();
e.stopPropagation();
if (noPromise) {
if (fireDoneImmediately) {
done();
}
}
Expand Down Expand Up @@ -817,7 +814,8 @@
* or limit on the amount of branches that will be searched through.
*/
var findChildData = function findChildData (targetParent, rootData) {
if ($.isEmptyObject(targetParent)) {
var isRootOfTree = $.isEmptyObject(targetParent);
if (isRootOfTree) {
return rootData;
}

Expand Down
1 change: 1 addition & 0 deletions test/tests.html
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
paths: {
jquery: jQueryPath,
bootstrap: 'bower_components/bootstrap/dist/js/bootstrap',
underscore: 'bower_components/underscore/underscore',
fuelux: 'js',
tests: window.noMoment ? 'test/tests-no-moment' : 'test/tests',
text: 'bower_components/requirejs-text/text',
Expand Down
13 changes: 13 additions & 0 deletions test/tree-tests/data/callCount.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
define(function callCountModule () {
return {
data: [
{
'name': 'Convex and Concave',
'type': 'item',
'attr': {
'id': 'item4'
}
}
]
};
});
45 changes: 45 additions & 0 deletions test/tree-tests/data/emptyFolder.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
define(function emptyFolderModule () {
return [
{
name: 'Empty Folder',
type: 'folder',
attr: {
id: 'emptyFolder'
},
children: []
},
{
name: 'Full Folder',
type: 'folder',
attr: {
id: 'fullFolder'
},
children: [
{
name: 'Empty Folder 2',
type: 'folder',
attr: {
id: 'emptyFolder2'
},
children: []
},
{
name: 'Folder Sibling 2',
type: 'item',
attr: {
id: 'sibling2',
'data-icon': 'glyphicon glyphicon-file'
}
}
]
},
{
name: 'Folder Sibling',
type: 'item',
attr: {
id: 'sibling1',
'data-icon': 'glyphicon glyphicon-file'
}
}
];
});
11 changes: 11 additions & 0 deletions test/tree-tests/data/guid.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
define(function guidModule () {
return function guid () {
var s4 = function s4 () {
return Math.floor((1 + Math.random()) * 0x10000)
.toString(16)
.substring(1);
};

return s4() + s4() + '-' + s4() + '-' + s4() + '-' + s4() + '-' + s4() + s4() + s4();
};
});
13 changes: 13 additions & 0 deletions test/tree-tests/data/textData.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
define(function textDataModule () {
return {
data: [
{
text: 'node text',
type: 'folder',
attr: {
id: 'folder1'
}
}
]
};
});
80 changes: 80 additions & 0 deletions test/tree-tests/data/treeData.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
define(function treeDataModule (require) {
var guid = require('./guid');

// make sure this returns a function so that new guids will be generated each time you use this data, otherwise
// trees using this data to populate nested folders will not work because you will have duplicate IDs. ie: you
// can't just return a static object here. You must return a function, and you must call that function again for
// each use, you can't just cache and reuse the output.
return function generateFreshData () {
return {
data: [
{
name: 'Ascending and Descending',
type: 'folder',
attr: {
id: 'folder' + guid()
}
},
{
name: 'Sky and Water I (with custom icon)',
type: 'item',
attr: {
id: 'folder' + guid(),
'data-icon': 'glyphicon glyphicon-file'
}
},
{
name: 'Drawing Hands',
type: 'folder',
attr: {
id: 'folder' + guid(),
'data-children': false
}
},
{
name: 'Waterfall',
type: 'item',
attr: {
id: 'item2'
}
},
{
name: 'Belvedere',
type: 'folder',
attr: {
id: 'folder' + guid()
}
},
{
name: 'Relativity (with custom icon)',
type: 'item',
attr: {
id: 'item3',
'data-icon': 'glyphicon glyphicon-picture'
}
},
{
name: 'House of Stairs',
type: 'folder',
attr: {
id: 'folder' + guid()
}
},
{
name: 'Convex and Concave',
type: 'item',
attr: {
id: 'item4'
}
},
{
name: 'Load More',
type: 'overflow',
attr: {
id: 'overflow1'
}
}
]
};
};
});
57 changes: 54 additions & 3 deletions test/tree-tests/down-key-module.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,69 @@
define(function keyboardNavigationModuleFactory (require) {
var $ = require('jquery');
var emptyFolderData = require('./data/emptyFolder');

return function downKeyModule (QUnit) {
QUnit.module( 'should respond to down key', {}, function testDownKeyPresses () {
QUnit.skip('when focus is on node above sibling node, moves focus down to sibling', function loadTree (assert) {
// Skipped due to time constraints. If you touch this part of the tree, please complete this test
QUnit.test('when focus is on node above sibling node, moves focus down to sibling', function loadTree (assert) {
assert.expect(2);

this.$tree.on('initialized.fu.tree', function triggerDownArrow () {
var $initialBranch = $(this.$tree.find('li:not(".hidden"):first'));
var $nextBranch = $(this.$tree.find('li:not(".hidden")').get(1));

$initialBranch.attr('tabindex', 0);
$initialBranch.focus();

this.$tree.on('keyboardNavigated.fu.tree', function testDownArrowResult () {
assert.equal($(document.activeElement).attr('id'), $nextBranch.attr('id'), 'next sibling now has focus');
});

assert.equal($(document.activeElement).attr('id'), $initialBranch.attr('id'), 'initial branch has focus');

var pressDownArrow = this.getKeyDown('down', $initialBranch);
$initialBranch.trigger(pressDownArrow);
}.bind(this));

this.$tree.tree({
dataSource: this.dataSource
});
});

QUnit.skip('when focus is on last focusable child of parent, moves focus out of parent onto first focusable sibling of parent', function loadTree (assert) {
// Skipped due to time constraints. If you touch this part of the tree, please complete this test
});

QUnit.skip('when focus is on open branch, moves focus into open branch onto first focusable child', function respondsToKeyboardInput (assert) {
// Skipped due to time constraints. If you touch this part of the tree, please complete this test

});

QUnit.test('when focus is on open empty branch, moves focus down to next sibling', function respondsToKeyboardInput (assert) {
assert.expect(2);

this.$tree.on('initialized.fu.tree', function triggerDiscloseFolder () {
var $initialBranch = $(this.$tree.find('li:not(".hidden"):first'));
var $nextBranch = $(this.$tree.find('li:not(".hidden")').get(1));

this.$tree.on('disclosedFolder.fu.tree', function triggerDownArrow () {
$initialBranch.attr('tabindex', 0);
$initialBranch.focus();

this.$tree.on('keyboardNavigated.fu.tree', function testDownArrowResult () {
assert.equal($(document.activeElement).attr('id'), $nextBranch.attr('id'), 'next sibling now has focus');
});

assert.equal($(document.activeElement).attr('id'), $initialBranch.attr('id'), 'initial branch has focus');

var pressDownArrow = this.getKeyDown('down', $initialBranch);
$initialBranch.trigger(pressDownArrow);
}.bind(this));

this.$tree.tree('discloseFolder', $initialBranch);
}.bind(this));

this.$tree.tree({
staticData: emptyFolderData
});
});
});
};
Expand Down
Loading