From 75c73c4ccdd4223c5a1dd0d9036870f21329dac3 Mon Sep 17 00:00:00 2001 From: user378230 Date: Sat, 9 Jul 2016 22:21:43 +0100 Subject: [PATCH] feat(groupBy): Define group by expression in repeat Changes the method for defining group by to simplify the codebase, improve performance and better support changes going forward. BREAKING CHANGE: Removes support for group-by and group-filter attributes. Instead you should now use an `ng-repeat` *like* syntax: `... group by x | ...` For example: ``` ``` Now becomes: ``` ``` Note that in order support filter groups based on an array with more than one value (eg.`group-filter="['A', 'B'...]"`) you will need to use the new `uisGroupFilter` filter. Example: ``` ``` --- docs/assets/demo.js | 10 +- docs/examples/demo-bootstrap.html | 2 +- docs/examples/demo-group-by.html | 10 +- docs/examples/demo-group-filter.html | 26 +++- docs/examples/demo-multiple-selection.html | 2 +- .../examples/demo-select2-with-bootstrap.html | 2 +- .../demo-selectize-with-bootstrap.html | 2 +- docs/examples/demo-tagging.html | 2 +- src/common.js | 19 +++ src/uiSelectChoicesDirective.js | 13 +- src/uiSelectController.js | 70 ++++----- src/uisRepeatParserService.js | 42 ++++-- test/select.spec.js | 136 +++++++++++++++--- 13 files changed, 247 insertions(+), 89 deletions(-) diff --git a/docs/assets/demo.js b/docs/assets/demo.js index 9d6702c4c..c6580aafc 100644 --- a/docs/assets/demo.js +++ b/docs/assets/demo.js @@ -39,7 +39,13 @@ app.filter('propsFilter', function() { return out; }; }); +app.filter('reverseOrderFilterFn', function() { + return function(items) { + if(!angular.isArray(items)) return items; + return items.slice().reverse(); + }; +}); app.controller('DemoCtrl', function ($scope, $http, $timeout, $interval) { var vm = this; @@ -86,10 +92,6 @@ app.controller('DemoCtrl', function ($scope, $http, $timeout, $interval) { return item.name[0]; }; - vm.reverseOrderFilterFn = function(groups) { - return groups.reverse(); - }; - vm.personAsync = {selected : "wladimir@email.com"}; vm.peopleAsync = []; diff --git a/docs/examples/demo-bootstrap.html b/docs/examples/demo-bootstrap.html index 0785a3007..5d966f735 100644 --- a/docs/examples/demo-bootstrap.html +++ b/docs/examples/demo-bootstrap.html @@ -25,7 +25,7 @@ {{$select.selected.name}} - + diff --git a/docs/examples/demo-group-by.html b/docs/examples/demo-group-by.html index 986bec15c..303aab607 100644 --- a/docs/examples/demo-group-by.html +++ b/docs/examples/demo-group-by.html @@ -5,10 +5,10 @@

Group By

Selected: {{ctrl.person.selected}}

-

Grouped using a string group-by="'country'"

+

Grouped using a string "group by person.country"

{{$select.selected.name}} - +
email: {{person.email}} @@ -17,17 +17,17 @@

Grouped using a string group-by="'country'"

 -

Grouped using a function group-by="ctrl.someGroupFn"

+

Grouped using a function "group by ctrl.someGroupFn(person)"

{{$select.selected.name}} - +
email: {{person.email}} age: - +

Regular

diff --git a/docs/examples/demo-group-filter.html b/docs/examples/demo-group-filter.html index c3e727fd1..bd864e893 100644 --- a/docs/examples/demo-group-filter.html +++ b/docs/examples/demo-group-filter.html @@ -7,19 +7,37 @@

Group Filtering

Selected: {{country.selected}}

-

Filter groups by array group-filter="['Z','B','C']"

+

Filter groups by name group by ctrl.firstLetterGroupFn(country) | filter:{name: 'Z'}

{{$select.selected.name}} - + -

Filter groups using a function group-filter="reverseOrderFilterFn"

+

Filter groups by string group by ctrl.firstLetterGroupFn(country) | uisGroupFilter:'Z'

{{$select.selected.name}} - + + + + + + +

Filter groups by array group by ctrl.firstLetterGroupFn(country) | uisGroupFilter:['A', 'B', 'C']

+ + {{$select.selected.name}} + + + + + + +

Filter groups using a function group by ctrl.firstLetterGroupFn(country) | reverseOrderFilterFn

+ + {{$select.selected.name}} + diff --git a/docs/examples/demo-multiple-selection.html b/docs/examples/demo-multiple-selection.html index 799c8430d..39ea0232b 100644 --- a/docs/examples/demo-multiple-selection.html +++ b/docs/examples/demo-multiple-selection.html @@ -62,7 +62,7 @@

Array of objects with single property binding

Array of objects (with groupBy)

{{$item.name}} <{{$item.email}}> - +
email: {{person.email}} diff --git a/docs/examples/demo-select2-with-bootstrap.html b/docs/examples/demo-select2-with-bootstrap.html index 98602e846..48ccb09e2 100644 --- a/docs/examples/demo-select2-with-bootstrap.html +++ b/docs/examples/demo-select2-with-bootstrap.html @@ -40,7 +40,7 @@ {{$select.selected.name}} - + diff --git a/docs/examples/demo-selectize-with-bootstrap.html b/docs/examples/demo-selectize-with-bootstrap.html index cf7528c59..ff2e2b4aa 100644 --- a/docs/examples/demo-selectize-with-bootstrap.html +++ b/docs/examples/demo-selectize-with-bootstrap.html @@ -45,7 +45,7 @@ {{$select.selected.name}} - + diff --git a/docs/examples/demo-tagging.html b/docs/examples/demo-tagging.html index a6ae2eb6c..3b963af84 100644 --- a/docs/examples/demo-tagging.html +++ b/docs/examples/demo-tagging.html @@ -31,7 +31,7 @@

Simple String Tags (Predictive Search Model & No Labels)<

Object Tags (with grouping)

{{$item.name}} <{{$item.email}}> - +
diff --git a/src/common.js b/src/common.js index 45d88acfc..3ad8beab4 100644 --- a/src/common.js +++ b/src/common.js @@ -128,7 +128,26 @@ var uis = angular.module('ui.select', []) } }; }) +.filter('uisGroupFilter', ['$filter', function($filter) { + return function(groups, definition) { + if(!angular.isArray(groups)) return groups; + + var filterDefintion = []; + + if(angular.isString(definition)) { + filterDefintion = [definition]; + } else if(angular.isArray(definition)) { + filterDefintion = definition; + } + + if(filterDefintion.length === 0) return groups; + + return $filter('filter')(groups, function(group) { + return filterDefintion.indexOf(group.name) > -1; + }); + }; +}]) /** * Highlights text that matches $select.search. * diff --git a/src/uiSelectChoicesDirective.js b/src/uiSelectChoicesDirective.js index a1c3967c7..50067d192 100644 --- a/src/uiSelectChoicesDirective.js +++ b/src/uiSelectChoicesDirective.js @@ -21,23 +21,20 @@ uis.directive('uiSelectChoices', if (!tAttrs.repeat) throw uiSelectMinErr('repeat', "Expected 'repeat' expression."); // var repeat = RepeatParser.parse(attrs.repeat); - var groupByExp = tAttrs.groupBy; - var groupFilterExp = tAttrs.groupFilter; + var parserResult = RepeatParser.parse(tAttrs.repeat); - if (groupByExp) { + if (parserResult.groupByExp) { var groups = tElement.querySelectorAll('.ui-select-choices-group'); if (groups.length !== 1) throw uiSelectMinErr('rows', "Expected 1 .ui-select-choices-group but got '{0}'.", groups.length); - groups.attr('ng-repeat', RepeatParser.getGroupNgRepeatExpression()); + groups.attr('ng-repeat', '$group in $select.groups ' + ( parserResult.groupByFilter || '')); } - var parserResult = RepeatParser.parse(tAttrs.repeat); - var choices = tElement.querySelectorAll('.ui-select-choices-row'); if (choices.length !== 1) { throw uiSelectMinErr('rows', "Expected 1 .ui-select-choices-row but got '{0}'.", choices.length); } - choices.attr('ng-repeat', parserResult.repeatExpression(groupByExp)) + choices.attr('ng-repeat', parserResult.repeatExpression(parserResult.groupByExp)) .attr('ng-if', '$select.open'); //Prevent unnecessary watches when dropdown is closed @@ -54,7 +51,7 @@ uis.directive('uiSelectChoices', return function link(scope, element, attrs, $select) { - $select.parseRepeatAttr(attrs.repeat, groupByExp, groupFilterExp); //Result ready at $select.parserResult + $select.parseRepeatAttr(attrs.repeat); //Result ready at $select.parserResult $select.disableChoiceExpression = attrs.uiDisableChoice; $select.onHighlightCallback = attrs.onHighlight; diff --git a/src/uiSelectController.js b/src/uiSelectController.js index b7dc2801c..f810b18dc 100644 --- a/src/uiSelectController.js +++ b/src/uiSelectController.js @@ -94,17 +94,17 @@ uis.controller('uiSelectCtrl', } } - function _groupsFilter(groups, groupNames) { - var i, j, result = []; - for(i = 0; i < groupNames.length ;i++){ - for(j = 0; j < groups.length ;j++){ - if(groups[j].name == [groupNames[i]]){ - result.push(groups[j]); - } - } - } - return result; - } + // function _groupsFilter(groups, groupNames) { + // var i, j, result = []; + // for(i = 0; i < groupNames.length ;i++){ + // for(j = 0; j < groups.length ;j++){ + // if(groups[j].name == [groupNames[i]]){ + // result.push(groups[j]); + // } + // } + // } + // return result; + // } // When the user clicks on ui-select, displays the dropdown list ctrl.activate = function(initSearchValue, avoidReset) { @@ -169,43 +169,43 @@ uis.controller('uiSelectCtrl', })[0]; }; - ctrl.parseRepeatAttr = function(repeatAttr, groupByExp, groupFilterExp) { + ctrl.parseRepeatAttr = function(repeatAttr) { + function updateGroups(items) { - var groupFn = $scope.$eval(groupByExp); ctrl.groups = []; - angular.forEach(items, function(item) { - var groupName = angular.isFunction(groupFn) ? groupFn(item) : item[groupFn]; - var group = ctrl.findGroupByName(groupName); - if(group) { - group.items.push(item); - } - else { - ctrl.groups.push({name: groupName, items: [item]}); + + var groupingFn = ctrl.parserResult.getGroupingFn($scope); + + angular.forEach(items, function (item) { + var itemGroupName = groupingFn(item); + var group = ctrl.findGroupByName(itemGroupName); + if (!group) { + group = { name: itemGroupName, items: [] }; + ctrl.groups.push(group); } + + group.items.push(item); }); - if(groupFilterExp){ - var groupFilterFn = $scope.$eval(groupFilterExp); - if( angular.isFunction(groupFilterFn)){ - ctrl.groups = groupFilterFn(ctrl.groups); - } else if(angular.isArray(groupFilterFn)){ - ctrl.groups = _groupsFilter(ctrl.groups, groupFilterFn); - } + + if(ctrl.parserResult.groupByFilter) { + // Prevent filtered groups from adding to ctrl.items + var filteredGroups = $scope.$eval('$select.groups ' + ctrl.parserResult.groupByFilter); + var filteredItems = filteredGroups.map(function(g) { return g.items; }); + ctrl.items = [].concat.apply([], filteredItems); + } else { + ctrl.items = items; } - ctrl.items = []; - ctrl.groups.forEach(function(group) { - ctrl.items = ctrl.items.concat(group.items); - }); } function setPlainItems(items) { ctrl.items = items; } - ctrl.setItemsFn = groupByExp ? updateGroups : setPlainItems; - ctrl.parserResult = RepeatParser.parse(repeatAttr); + ctrl.isGrouped = !!ctrl.parserResult.groupByExp; + + ctrl.setItemsFn = ctrl.isGrouped ? updateGroups : setPlainItems; - ctrl.isGrouped = !!groupByExp; ctrl.itemProperty = ctrl.parserResult.itemName; //If collection is an Object, convert it to Array diff --git a/src/uisRepeatParserService.js b/src/uisRepeatParserService.js index 58ffa19c6..7c8e70871 100644 --- a/src/uisRepeatParserService.js +++ b/src/uisRepeatParserService.js @@ -26,8 +26,8 @@ uis.service('uisRepeatParser', ['uiSelectMinErr','$parse', function(uiSelectMinE // If an array is used as collection // if (isObjectCollection){ - // 000000000000000000000000000000111111111000000000000000222222222222220033333333333333333333330000444444444444444444000000000000000055555555555000000000000000000000066666666600000000 - match = expression.match(/^\s*(?:([\s\S]+?)\s+as\s+)?(?:([\$\w][\$\w]*)|(?:\(\s*([\$\w][\$\w]*)\s*,\s*([\$\w][\$\w]*)\s*\)))\s+in\s+(\s*[\s\S]+?)?(?:\s+track\s+by\s+([\s\S]+?))?\s*$/); + // 0000000000000000000000000000001111111110000000000000002222222222222200333333333333333333333300004444444444444444440000000000000000555555555550000000000000000000000666666666000000000000000000000777777777000 + match = expression.match(/^\s*(?:([\s\S]+?)\s+as\s+)?(?:([\$\w][\$\w]*)|(?:\(\s*([\$\w][\$\w]*)\s*,\s*([\$\w][\$\w]*)\s*\)))\s+in\s+(\s*[\s\S]+?)?(?:\s+track\s+by\s+([\s\S]+?))?(?:\s+group\s+by\s+([\s\S]+?))?\s*$/); // 1 Alias // 2 Item @@ -35,9 +35,10 @@ uis.service('uisRepeatParser', ['uiSelectMinErr','$parse', function(uiSelectMinE // 4 Value on (key,value) // 5 Source expression (including filters) // 6 Track by + // 7 Group by expression (including filters) if (!match) { - throw uiSelectMinErr('iexp', "Expected expression in form of '_item_ in _collection_[ track by _id_]' but got '{0}'.", + throw uiSelectMinErr('iexp', "Expected expression in form of '_item_ in _collection_[ track by _id_][ group by _property_or_func_[ |_group_filter_exp]] ]' but got '{0}'.", expression); } @@ -58,12 +59,40 @@ uis.service('uisRepeatParser', ['uiSelectMinErr','$parse', function(uiSelectMinE } } + var itemName = match[4] || match[2]; + + var groupByExp = match[7], + groupByFilters = '', + getGroupingFn; + + if(match[7]) { + // match all after | but not after || + var groupFilterMatch = match[7].match(/^\s*(?:[\s\S]+?)(?:[^\|]|\|\|)+([\s\S]*)\s*$/); + if(groupFilterMatch && groupFilterMatch[1].trim()) { + // TODO: Consider checking for enclosing parenthesis? + groupByFilters = groupFilterMatch[1]; + groupByExp = groupByExp.replace(groupByFilters, '').trim(); + } + var parsedGroupingExp = $parse(groupByExp); + // Creates a getter function that can run against an arbitary item + getGroupingFn = function($scope) { + return function(item) { + var locals = {}; + locals[itemName] = item; + return parsedGroupingExp($scope, locals); + }; + }; + } + return { - itemName: match[4] || match[2], // (lhs) Left-hand side, + itemName: itemName, // (lhs) Left-hand side, keyName: match[3], //for (key, value) syntax source: $parse(source), filters: filters, trackByExp: match[6], + groupByExp: groupByExp, + groupByFilter: groupByFilters, + getGroupingFn: getGroupingFn, modelMapper: $parse(match[1] || match[4] || match[2]), repeatExpression: function (grouped) { var expression = this.itemName + ' in ' + (grouped ? '$group.items' : '$select.items'); @@ -75,9 +104,4 @@ uis.service('uisRepeatParser', ['uiSelectMinErr','$parse', function(uiSelectMinE }; }; - - self.getGroupNgRepeatExpression = function() { - return '$group in $select.groups'; - }; - }]); diff --git a/test/select.spec.js b/test/select.spec.js index 5b735689f..9cf8e4f9c 100644 --- a/test/select.spec.js +++ b/test/select.spec.js @@ -69,12 +69,21 @@ describe('ui-select tests', function() { beforeEach(module('ngSanitize', 'ui.select', 'wrapperDirective', 'testValidator')); beforeEach(function() { - module(function($provide) { + module(function($provide, $filterProvider) { $provide.factory('uisOffset', function() { return function(el) { return {top: 100, left: 200, width: 300, height: 400}; }; }); + + $filterProvider.register('invertOrder', function() { + return function(groups) { + if(!angular.isArray(groups)) return; + return groups.slice().sort(function(groupA, groupB){ + return groupA.name.toLocaleLowerCase() < groupB.name.toLocaleLowerCase(); + }); + }; + }); }); }); @@ -87,17 +96,10 @@ describe('ui-select tests', function() { uisRepeatParser = _uisRepeatParser_; scope.selection = {}; - scope.getGroupLabel = function(person) { + scope.getGroupByLabel = function(person) { return person.age % 2 ? 'even' : 'odd'; }; - scope.filterInvertOrder = function(groups) { - return groups.sort(function(groupA, groupB){ - return groupA.name.toLocaleLowerCase() < groupB.name.toLocaleLowerCase(); - }); - }; - - scope.people = [ { name: 'Adam', email: 'adam@email.com', group: 'Foo', age: 12 }, { name: 'Amalie', email: 'amalie@email.com', group: 'Foo', age: 12 }, @@ -384,6 +386,70 @@ describe('ui-select tests', function() { }); + describe("parsing group by expresson", function() { + it('should parse a simple expression', function() { + + var parserResult = uisRepeatParser.parse('person in people group by person.country'); + expect(parserResult.itemName).toBe('person'); + expect(parserResult.groupByExp).toBe('person.country'); + }); + + it('should parse simple grouping function expression', function() { + + var parserResult = uisRepeatParser.parse('person in people group by someGroupingFn(person)'); + expect(parserResult.itemName).toBe('person'); + expect(parserResult.groupByExp).toBe('someGroupingFn(person)'); + }); + + it('should parse any grouping filters', function() { + + var parserResult = uisRepeatParser.parse('person in people group by person.country | filter:{name: "usa"}'); + expect(parserResult.itemName).toBe('person'); + expect(parserResult.groupByExp).toBe('person.country'); + expect(parserResult.groupByFilter).toBe('| filter:{name: "usa"}'); + }); + + it('should parse fallback grouping expression', function() { + + var parserResult = uisRepeatParser.parse('person in people group by (person.firstName || person.lastName)'); + expect(parserResult.itemName).toBe('person'); + expect(parserResult.groupByExp).toBe('(person.firstName || person.lastName)'); + }); + + it('should parse group by in a complex expression', function() { + + var parserResult = uisRepeatParser.parse('person.name as person in (peopleNothing || people) track by person.id group by person.country'); + expect(parserResult.itemName).toBe('person'); + expect(parserResult.groupByExp).toBe('person.country'); + }); + + it('should get group name using simple group by property', function() { + + var person = {name: 'Wladimir', country: 'usa'}; + + var parserResult = uisRepeatParser.parse('person in people group by person.country'); + var groupingFn = parserResult.getGroupingFn({}); + expect(groupingFn(person)).toBe('usa'); + }); + + it('should call grouping function if specified', function() { + var repeatScope = { someGroupingFn: function(item) { + // group by reversed country (just so we know the function has been called) + return item.country.split('').reverse().join(''); + }}; + + var person = {name: 'Wladimir', country: 'usa'}; + + var parserResult = uisRepeatParser.parse('person in people group by someGroupingFn(person)'); + var groupingFn = parserResult.getGroupingFn(repeatScope); + expect(groupingFn(person)).toBe('asu'); + }); + }); + + + + + it('should not leak memory', function() { var cacheLenght = Object.keys(angular.element.cache).length; createUiSelect().remove(); @@ -974,7 +1040,7 @@ describe('ui-select tests', function() { return compileTemplate( ' \ {{$select.selected.name}} \ - \ + \
\
\ \ @@ -1026,7 +1092,7 @@ describe('ui-select tests', function() { return compileTemplate( ' \ {{$select.selected.name}} \ - \ + \
\ \ ' @@ -1045,7 +1111,7 @@ describe('ui-select tests', function() { return compileTemplate('\ \ {{$select.selected.name}} \ - \ + \
\ \ ' @@ -1064,8 +1130,7 @@ describe('ui-select tests', function() { return compileTemplate('\ \ {{$select.selected.name}} \ - \ + \
\ \ ' @@ -1109,7 +1174,7 @@ describe('ui-select tests', function() { \ ' ); - }).toThrow(new Error('[ui.select:iexp] Expected expression in form of \'_item_ in _collection_[ track by _id_]\' but got \'incorrect format people\'.')); + }).toThrow(); }); it('should throw when no ui-select-match found', function() { @@ -1745,6 +1810,7 @@ describe('ui-select tests', function() { function createUiSelectMultiple(attrs) { var attrsHtml = '', + groupByHtml = '', choicesAttrsHtml = '', matchesAttrsHtml = ''; if (attrs !== undefined) { @@ -1756,14 +1822,14 @@ describe('ui-select tests', function() { if (attrs.taggingTokens !== undefined) { attrsHtml += ' tagging-tokens="' + attrs.taggingTokens + '"'; } if (attrs.taggingLabel !== undefined) { attrsHtml += ' tagging-label="' + attrs.taggingLabel + '"'; } if (attrs.inputId !== undefined) { attrsHtml += ' input-id="' + attrs.inputId + '"'; } - if (attrs.groupBy !== undefined) { choicesAttrsHtml += ' group-by="' + attrs.groupBy + '"'; } + if (attrs.groupBy !== undefined) { groupByHtml += ' group by ' + attrs.groupBy; } if (attrs.lockChoice !== undefined) { matchesAttrsHtml += ' ui-lock-choice="' + attrs.lockChoice + '"'; } } return compileTemplate( ' \ {{$item.name}} <{{$item.email}}> \ - \ + \
\
\ \ @@ -2509,7 +2575,7 @@ describe('ui-select tests', function() { expect(el.find('.ui-select-choices-row-inner').size()).toBe(0); }); - it('should allow creating tag in multi select mode with tagging and group-by enabled', function() { + it('should allow creating tag in multi select mode with tagging and group by enabled', function() { scope.taggingFunc = function (name) { return { name: name, @@ -2519,7 +2585,7 @@ describe('ui-select tests', function() { }; }; - var el = createUiSelectMultiple({tagging: 'taggingFunc', groupBy: "'age'"}); + var el = createUiSelectMultiple({tagging: 'taggingFunc', groupBy: "person.age"}); showChoicesForSearch(el, 'amal'); expect(el.find('.ui-select-choices-row-inner').size()).toBe(2); @@ -2880,4 +2946,36 @@ describe('ui-select tests', function() { }); }); + describe('uisGroupFilter filter', function() { + var groupFilter; + var testArray = [ + {name: 'group1'}, + {name: 'group2'}, + {name: 'group3'}, + {name: 'group4'}, + {name: 'group5'} + ]; + + beforeEach(function() { + groupFilter = $injector.get('uisGroupFilterFilter'); + }); + + it('should return all items if no filter arguments are give', function() { + expect(groupFilter(testArray)).toEqual(testArray); + }); + + it('should, if given a string, return only groups with that matching name property', function () { + expect(groupFilter(testArray, 'group1')).toEqual([{ name: 'group1' }]); + }); + + it('should, if given an array, return any groups with a name property specified in the array', function () { + expect(groupFilter(testArray, ['group1', 'group2'])).toEqual([{ name: 'group1' }, { name: 'group2' }]); + }); + + it('should, if given an array, return any groups with a name property specified in the array (incl. dupes)', function () { + var testArray2 = testArray.slice(); + testArray2.push({ name: 'group2' }); + expect(groupFilter(testArray2, ['group1', 'group2'])).toEqual([{ name: 'group1' }, { name: 'group2' }, { name: 'group2' }]); + }); + }); });