From d55e0371d6ab7723b82d224497f78d897fad261f Mon Sep 17 00:00:00 2001 From: Stephen James Date: Thu, 7 May 2015 13:47:08 -0400 Subject: [PATCH 1/3] Single, multiselect, select tree node bug Refactor (combine) selectFolder and selectItem to re-use code. Add clearSelected method --- index.html | 3 +- index.js | 4 ++ js/tree.js | 177 +++++++++++++++++++++++++++++------------------------ 3 files changed, 103 insertions(+), 81 deletions(-) diff --git a/index.html b/index.html index 99049f6e4..2e3d3847e 100644 --- a/index.html +++ b/index.html @@ -2358,7 +2358,8 @@

only items selectable (please note structure of treebranch)

- + + diff --git a/index.js b/index.js index 4c4173dcf..719b270b8 100644 --- a/index.js +++ b/index.js @@ -992,6 +992,10 @@ myTreeInit(); myTreeInit(); }); + $('#btnTreeClearSelected').click(function () { + log('Items/folders cleared: ', $('#myTree1').tree('clearSelected') ); + }); + $('#btnTreeDiscloseVisible').click(function () { $('#myTree1').tree('discloseVisible'); }); diff --git a/js/tree.js b/js/tree.js index 99ccbeb95..82ed5bb7a 100644 --- a/js/tree.js +++ b/js/tree.js @@ -60,6 +60,13 @@ Tree.prototype = { constructor: Tree, + clearSelected: function clearSelected(nodes) { + nodes = nodes || this.$element; + var selectedElements = $(nodes).find('.tree-selected'); + selectedElements.removeClass('tree-selected'); + return selectedElements; + }, + destroy: function destroy() { // any external bindings [none] // empty elements to return to original markup @@ -159,58 +166,104 @@ }); }, - selectItem: function selectItem(el) { - if (!this.options.itemSelect) return; - var $el = $(el); - var selData = $el.data(); - var $all = this.$element.find('.tree-selected'); - var data = []; - var $icon = $el.find('.icon-item'); + selectTreeNode: function selectItem(clickedElement, nodeType) { + var $clickedElement = $(clickedElement); + var $clickedElementIcon; + var $selectedElements = this.$element.find('.tree-selected'); + var eventType; + var selectedDataForEvent = []; + var clickedElementData; + + if (nodeType === 'folder') { + // make the $clickedElement the container branch + $clickedElement = $clickedElement.closest('.tree-branch'); + $clickedElementIcon = $clickedElement.find('.icon-folder'); + } + else { + $clickedElementIcon = $clickedElement.find('.icon-item'); + } - if (this.options.multiSelect) { - $.each($all, function (index, value) { - var $val = $(value); - if ($val[0] !== $el[0]) { - data.push($(value).data()); + clickedElementData = $clickedElement.data(); + + function multiSelectSyncNodes() { + // search for currently selected and add to selected data list if needed + $.each($selectedElements, function (index, element) { + var $element = $(element); + if ($element[0] !== $clickedElement[0]) { + selectedDataForEvent.push( $($element).data() ); } }); - } else if ($all[0] !== $el[0]) { - $all.removeClass('tree-selected') - .find('.glyphicon').removeClass('glyphicon-ok').addClass('fueluxicon-bullet'); - data.push(selData); - } - var eventType = 'selected'; - if ($el.hasClass('tree-selected')) { - eventType = 'deselected'; - $el.removeClass('tree-selected'); - if ($icon.hasClass('glyphicon-ok') || $icon.hasClass('fueluxicon-bullet')) { - $icon.removeClass('glyphicon-ok').addClass('fueluxicon-bullet'); + if ($clickedElement.hasClass('tree-selected')) { + // update styling of clicked element + $clickedElement.removeClass('tree-selected'); + if ( $clickedElement.data('type') === 'item' && $clickedElementIcon.hasClass('glyphicon-ok') ) { + $clickedElementIcon.removeClass('glyphicon-ok').addClass('fueluxicon-bullet'); // make bullet + } + // set event data + eventType = 'deselected'; + } + else { + // update styling of clicked element + $clickedElement.addClass('tree-selected'); + if ( $clickedElement.data('type') === 'item' && $clickedElementIcon.hasClass('fueluxicon-bullet') ) { + $clickedElementIcon.removeClass('fueluxicon-bullet').addClass('glyphicon-ok'); // make checkmark + } + // set event data + eventType = 'selected'; + selectedDataForEvent.push(clickedElementData); } + } - } else { - $el.addClass ('tree-selected'); - // add tree dot back in - if ($icon.hasClass('glyphicon-ok') || $icon.hasClass('fueluxicon-bullet')) { - $icon.removeClass('fueluxicon-bullet').addClass('glyphicon-ok'); + function singleSelectSyncNodes(self) { + // element is not currently selected + if ($selectedElements[0] !== $clickedElement[0]) { + var clearedElements = self.clearSelected(self.$element); + // change styling of deselected element + if ($(clearedElements[0]).data('type') === 'item' ) { + if( $(clearedElements[0]).find( '.glyphicon' ).hasClass('glyphicon-ok') ) { + $(clearedElements[0]).find( '.glyphicon' ).removeClass( 'glyphicon-ok' ).addClass( 'fueluxicon-bullet' ); + } + } + // update styling of clicked element + $clickedElement.addClass('tree-selected'); + if ( $clickedElement.data('type') === 'item' && $clickedElementIcon.hasClass('fueluxicon-bullet') ) { + $clickedElementIcon.removeClass('fueluxicon-bullet').addClass('glyphicon-ok'); // make checkmark + } + // set event data + eventType = 'selected'; + selectedDataForEvent = [clickedElementData]; } + else { + // add styling to clicked element + $clickedElement.removeClass('tree-selected'); + if ( $clickedElement.data('type') === 'item' && $clickedElementIcon.hasClass('glyphicon-ok') ) { + $clickedElementIcon.removeClass('glyphicon-ok').addClass('fueluxicon-bullet'); // make bullet + } - if (this.options.multiSelect) { - data.push(selData); + // set event data + eventType = 'deselected'; + selectedDataForEvent = []; } + } + // start here + if ( this.options.multiSelect ) { + multiSelectSyncNodes(this); } + else { + singleSelectSyncNodes(this); + } + // all done with the DOM, now fire events this.$element.trigger(eventType + '.fu.tree', { - target: selData, - selected: data + target: clickedElementData, + selected: selectedDataForEvent }); - // Return new list of selected items, the item - // clicked, and the type of event: - $el.trigger('updated.fu.tree', { - selected: data, - item: $el, + $clickedElement.trigger('updated.fu.tree', { + selected: selectedDataForEvent, + item: $clickedElement, eventType: eventType }); }, @@ -270,52 +323,16 @@ } }, - selectFolder: function selectFolder(clickedElement) { - if (!this.options.folderSelect) return; - var $clickedElement = $(clickedElement); - var $clickedBranch = $clickedElement.closest('.tree-branch'); - var $selectedBranch = this.$element.find('.tree-branch.tree-selected'); - var clickedData = $clickedBranch.data(); - var selectedData = []; - var eventType = 'selected'; - - // select clicked item - if ($clickedBranch.hasClass('tree-selected')) { - eventType = 'deselected'; - $clickedBranch.removeClass('tree-selected'); - } else { - $clickedBranch.addClass('tree-selected'); + selectFolder: function selectFolder(el) { + if (this.options.folderSelect) { + this.selectTreeNode(el, 'folder'); } + }, - if (this.options.multiSelect) { - // get currently selected - $selectedBranch = this.$element.find('.tree-branch.tree-selected'); - - $.each($selectedBranch, function (index, value) { - var $value = $(value); - if ($value[0] !== $clickedElement[0]) { - selectedData.push($(value).data()); - } - }); - - } else if ($selectedBranch[0] !== $clickedElement[0]) { - $selectedBranch.removeClass('tree-selected'); - - selectedData.push(clickedData); + selectItem: function selectItem(el) { + if (this.options.itemSelect) { + this.selectTreeNode(el, 'item'); } - - this.$element.trigger(eventType + '.fu.tree', { - target: clickedData, - selected: selectedData - }); - - // Return new list of selected items, the item - // clicked, and the type of event: - $clickedElement.trigger('updated.fu.tree', { - selected: selectedData, - item: $clickedElement, - eventType: eventType - }); }, selectedItems: function selectedItems() { From a1c0ffe632d91f4829f904eab96aca24075bba14 Mon Sep 17 00:00:00 2001 From: Stephen James Date: Sat, 9 May 2015 18:24:47 -0400 Subject: [PATCH 2/3] Increase variations in single select tree tests Many problems with this unit test: - Mouse clicks were not being tested, only API methods - Folder selectable tree needs to test items also. - Folder selectable tree needs to test items with folders previously selected, and folders with items previously selected - Markup for folder selectable tree did not include a `tree-item` template and therefore created only folders and did not create any items (hence 4 folders/items are returned instead of the folders/correct 8 items). - The selectItem method was being used to select folders. --- test/markup/tree-markup.html | 7 +++++++ test/tree-test.js | 25 ++++++++++++++++++++----- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/test/markup/tree-markup.html b/test/markup/tree-markup.html index d8330cb3c..cdfaaaf60 100644 --- a/test/markup/tree-markup.html +++ b/test/markup/tree-markup.html @@ -66,6 +66,13 @@
    + +
    diff --git a/test/tree-test.js b/test/tree-test.js index bd4d1740e..e51e8c619 100644 --- a/test/tree-test.js +++ b/test/tree-test.js @@ -182,12 +182,13 @@ define(function (require) { $selNode = $tree.find('.tree-branch:eq(1)'); $tree.tree('discloseFolder', $selNode.find('.tree-branch-header')); - equal($selNode.find('.tree-branch-children > li').length, 4, 'Folder has been populated with sub-folders'); + equal($selNode.find('.tree-branch-children > li').length, 8, 'Folder has been populated with sub-folders and items'); }); test("Single item/folder selection works as designed", function () { var $tree = $(html).find('#MyTree'); + // multiSelect: false is the default $tree.tree({ dataSource: this.dataSource }); @@ -204,10 +205,24 @@ define(function (require) { folderSelect: true }); - $tree.tree('selectItem', $tree.find('.tree-branch-name:eq(1)')); - equal($tree.tree('selectedItems').length, 1, 'Return single selected value'); - $tree.tree('selectItem', $tree.find('.tree-branch-name:eq(2)')); - equal($tree.tree('selectedItems').length, 1, 'Return new single selected value'); + $tree.tree('selectItem', $tree.find('.tree-item:eq(1)')); + equal($tree.tree('selectedItems').length, 1, 'Return single selected item (none previously selected, 1st programatic selection)'); + + $tree.tree('selectFolder', $tree.find('.tree-branch-name:eq(1)')); + equal($tree.tree('selectedItems').length, 1, 'Return single selected folder (item previously selected, 2nd programatic selection)'); + + $tree.tree('selectItem', $tree.find('.tree-item:eq(2)')); + equal($tree.tree('selectedItems').length, 1, 'Return single selected item (folder previously selected, 3rd programatic selection)'); + + $tree.find('.tree-item:eq(1)').click(); + equal($tree.tree('selectedItems').length, 1, 'Return single selected item (item previously selected, 1st click selection)'); + + $tree.find('.tree-branch-name:eq(1)').click(); + equal($tree.tree('selectedItems').length, 1, 'Return single selected folder (item previously selected, 2nd click selection)'); + + $tree.find('.tree-item:eq(2)').click(); + equal($tree.tree('selectedItems').length, 1, 'Return single selected item (folder previously selected, 3rd click selection)'); + }); test("Multiple item/folder selection works as designed", function () { From e54a9dc71fddc832d9bab7e9351099a5514ae5b6 Mon Sep 17 00:00:00 2001 From: Stephen James Date: Wed, 20 May 2015 00:55:46 -0400 Subject: [PATCH 3/3] Make selectTreeNode DRY, add deselectAll method --- index.js | 2 +- js/tree.js | 177 +++++++++++++++++++++++++++-------------------------- 2 files changed, 90 insertions(+), 89 deletions(-) diff --git a/index.js b/index.js index 719b270b8..4108254d0 100644 --- a/index.js +++ b/index.js @@ -993,7 +993,7 @@ myTreeInit(); }); $('#btnTreeClearSelected').click(function () { - log('Items/folders cleared: ', $('#myTree1').tree('clearSelected') ); + log('Items/folders cleared: ', $('#myTree1').tree('deselectAll') ); }); $('#btnTreeDiscloseVisible').click(function () { diff --git a/js/tree.js b/js/tree.js index 82ed5bb7a..d73a7368a 100644 --- a/js/tree.js +++ b/js/tree.js @@ -60,11 +60,14 @@ Tree.prototype = { constructor: Tree, - clearSelected: function clearSelected(nodes) { + deselectAll: function deselectAll(nodes) { + // clear all child tree nodes and style as deselected nodes = nodes || this.$element; - var selectedElements = $(nodes).find('.tree-selected'); - selectedElements.removeClass('tree-selected'); - return selectedElements; + var $selectedElements = $(nodes).find('.tree-selected'); + $selectedElements.each(function (index, element) { + styleNodeDeselected( $(element), $(element).find( '.glyphicon' ) ); + }); + return $selectedElements; }, destroy: function destroy() { @@ -167,104 +170,42 @@ }, selectTreeNode: function selectItem(clickedElement, nodeType) { - var $clickedElement = $(clickedElement); - var $clickedElementIcon; - var $selectedElements = this.$element.find('.tree-selected'); - var eventType; - var selectedDataForEvent = []; - var clickedElementData; + var clicked = {}; // object for clicked element + clicked.$element = $(clickedElement); + + var selected = {}; // object for selected elements + selected.$elements = this.$element.find('.tree-selected'); + selected.dataForEvent = []; + // determine clicked element and it's icon if (nodeType === 'folder') { - // make the $clickedElement the container branch - $clickedElement = $clickedElement.closest('.tree-branch'); - $clickedElementIcon = $clickedElement.find('.icon-folder'); + // make the clicked.$element the container branch + clicked.$element = clicked.$element.closest('.tree-branch'); + clicked.$icon = clicked.$element.find('.icon-folder'); } else { - $clickedElementIcon = $clickedElement.find('.icon-item'); - } - - clickedElementData = $clickedElement.data(); - - function multiSelectSyncNodes() { - // search for currently selected and add to selected data list if needed - $.each($selectedElements, function (index, element) { - var $element = $(element); - if ($element[0] !== $clickedElement[0]) { - selectedDataForEvent.push( $($element).data() ); - } - }); - - if ($clickedElement.hasClass('tree-selected')) { - // update styling of clicked element - $clickedElement.removeClass('tree-selected'); - if ( $clickedElement.data('type') === 'item' && $clickedElementIcon.hasClass('glyphicon-ok') ) { - $clickedElementIcon.removeClass('glyphicon-ok').addClass('fueluxicon-bullet'); // make bullet - } - // set event data - eventType = 'deselected'; - } - else { - // update styling of clicked element - $clickedElement.addClass('tree-selected'); - if ( $clickedElement.data('type') === 'item' && $clickedElementIcon.hasClass('fueluxicon-bullet') ) { - $clickedElementIcon.removeClass('fueluxicon-bullet').addClass('glyphicon-ok'); // make checkmark - } - // set event data - eventType = 'selected'; - selectedDataForEvent.push(clickedElementData); - } + clicked.$icon = clicked.$element.find('.icon-item'); } + clicked.elementData = clicked.$element.data(); - function singleSelectSyncNodes(self) { - // element is not currently selected - if ($selectedElements[0] !== $clickedElement[0]) { - var clearedElements = self.clearSelected(self.$element); - // change styling of deselected element - if ($(clearedElements[0]).data('type') === 'item' ) { - if( $(clearedElements[0]).find( '.glyphicon' ).hasClass('glyphicon-ok') ) { - $(clearedElements[0]).find( '.glyphicon' ).removeClass( 'glyphicon-ok' ).addClass( 'fueluxicon-bullet' ); - } - } - // update styling of clicked element - $clickedElement.addClass('tree-selected'); - if ( $clickedElement.data('type') === 'item' && $clickedElementIcon.hasClass('fueluxicon-bullet') ) { - $clickedElementIcon.removeClass('fueluxicon-bullet').addClass('glyphicon-ok'); // make checkmark - } - // set event data - eventType = 'selected'; - selectedDataForEvent = [clickedElementData]; - } - else { - // add styling to clicked element - $clickedElement.removeClass('tree-selected'); - if ( $clickedElement.data('type') === 'item' && $clickedElementIcon.hasClass('glyphicon-ok') ) { - $clickedElementIcon.removeClass('glyphicon-ok').addClass('fueluxicon-bullet'); // make bullet - } - - // set event data - eventType = 'deselected'; - selectedDataForEvent = []; - } - } - - // start here + // the below functions pass objects by copy/reference and use modified object in this function if ( this.options.multiSelect ) { - multiSelectSyncNodes(this); + multiSelectSyncNodes(this, clicked, selected); } else { - singleSelectSyncNodes(this); + singleSelectSyncNodes(this, clicked, selected); } // all done with the DOM, now fire events - this.$element.trigger(eventType + '.fu.tree', { - target: clickedElementData, - selected: selectedDataForEvent + this.$element.trigger(selected.eventType + '.fu.tree', { + target: clicked.elementData, + selected: selected.dataForEvent }); - $clickedElement.trigger('updated.fu.tree', { - selected: selectedDataForEvent, - item: $clickedElement, - eventType: eventType + clicked.$element.trigger('updated.fu.tree', { + selected: selected.dataForEvent, + item: clicked.$element, + eventType: selected.eventType }); }, @@ -482,11 +423,71 @@ } }; + + // ALIASES + //alias for collapse for consistency. "Collapse" is an ambiguous term (collapse what? All? One specific branch?) Tree.prototype.closeAll = Tree.prototype.collapse; //alias for backwards compatibility because there's no reason not to. Tree.prototype.openFolder = Tree.prototype.discloseFolder; + + // PRIVATE FUNCTIONS + + function styleNodeSelected ($element, $icon) { + $element.addClass('tree-selected'); + if ( $element.data('type') === 'item' && $icon.hasClass('fueluxicon-bullet') ) { + $icon.removeClass('fueluxicon-bullet').addClass('glyphicon-ok'); // make checkmark + } + } + + function styleNodeDeselected ($element, $icon) { + $element.removeClass('tree-selected'); + if ( $element.data('type') === 'item' && $icon.hasClass('glyphicon-ok') ) { + $icon.removeClass('glyphicon-ok').addClass('fueluxicon-bullet'); // make bullet + } + } + + function multiSelectSyncNodes (self, clicked, selected) { + // search for currently selected and add to selected data list if needed + $.each(selected.$elements, function (index, element) { + var $element = $(element); + if ($element[0] !== clicked.$element[0]) { + selected.dataForEvent.push( $($element).data() ); + } + }); + + if (clicked.$element.hasClass('tree-selected')) { + styleNodeDeselected (clicked.$element, clicked.$icon); + // set event data + selected.eventType = 'deselected'; + } + else { + styleNodeSelected(clicked.$element, clicked.$icon); + // set event data + selected.eventType = 'selected'; + selected.dataForEvent.push(clicked.elementData); + } + } + + function singleSelectSyncNodes(self, clicked, selected) { + // element is not currently selected + if (selected.$elements[0] !== clicked.$element[0]) { + var clearedElements = self.deselectAll(self.$element); + styleNodeSelected(clicked.$element, clicked.$icon); + // set event data + selected.eventType = 'selected'; + selected.dataForEvent = [clicked.elementData]; + } + else { + styleNodeDeselected(clicked.$element, clicked.$icon); + // set event data + selected.eventType = 'deselected'; + selected.dataForEvent = []; + } + } + + // TREE PLUGIN DEFINITION $.fn.tree = function tree(option) {