From 5ed72ed54cbf7cd1880bacea7e2e11ef8e9b096c Mon Sep 17 00:00:00 2001 From: Moriel Schottlender Date: Fri, 23 Jun 2017 13:30:42 -0700 Subject: [PATCH] RCFilters: Add 'single_option' group type Group type that only allows a single option to be selected from its range of items. Bonus: Add the ability to have a view that is hidden from the menus Bug: T162784 Bug: T162786 Change-Id: Ide93491a49c1405926ac171c7924a469e94c0e0a --- .../dm/mw.rcfilters.dm.FilterGroup.js | 68 +++++++- .../dm/mw.rcfilters.dm.FiltersViewModel.js | 15 +- .../ui/mw.rcfilters.ui.MenuSelectWidget.js | 70 ++++---- .../dm.FiltersViewModel.test.js | 159 +++++++++++++++--- 4 files changed, 245 insertions(+), 67 deletions(-) diff --git a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js index c1a936d3fd..2307f301cf 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js @@ -12,6 +12,7 @@ * @cfg {string} [view='default'] Name of the display group this group * is a part of. * @cfg {string} [title] Group title + * @cfg {boolean} [hidden] This group is hidden from the regular menu views * @cfg {string} [separator='|'] Value separator for 'string_options' groups * @cfg {boolean} [active] Group is active * @cfg {boolean} [fullCoverage] This filters in this group collectively cover all results @@ -36,6 +37,7 @@ this.type = config.type || 'send_unselected_if_any'; this.view = config.view || 'default'; this.title = config.title || name; + this.hidden = !!config.hidden; this.separator = config.separator || '|'; this.labelPrefixKey = config.labelPrefixKey; @@ -156,17 +158,34 @@ return item.getParamName(); } ) ).join( this.getSeparator() ); + } else if ( this.getType() === 'single_option' ) { + // For this group, the parameter is the group name, + // and a single item can be selected, or none at all + // The item also must be recognized or none is set as + // default + model.defaultParams[ this.getName() ] = this.getItemByParamName( groupDefault ) ? groupDefault : ''; } }; /** * Respond to filterItem update event * + * @param {mw.rcfilters.dm.FilterItem} item Updated filter item * @fires update */ - mw.rcfilters.dm.FilterGroup.prototype.onFilterItemUpdate = function () { + mw.rcfilters.dm.FilterGroup.prototype.onFilterItemUpdate = function ( item ) { // Update state - var active = this.areAnySelected(); + var active = this.areAnySelected(), + itemName = item && item.getName(); + + if ( item.isSelected() && this.getType() === 'single_option' ) { + // Change the selection to only be the newly selected item + this.getItems().forEach( function ( filterItem ) { + if ( filterItem.getName() !== itemName ) { + filterItem.toggleSelected( false ); + } + } ); + } if ( this.active !== active ) { this.active = active; @@ -183,6 +202,15 @@ return this.active; }; + /** + * Get group hidden state + * + * @return {boolean} Hidden state + */ + mw.rcfilters.dm.FilterGroup.prototype.isHidden = function () { + return this.hidden; + }; + /** * Get group name * @@ -388,7 +416,22 @@ areAnySelected = false, buildFromCurrentState = !filterRepresentation, result = {}, - filterParamNames = {}; + model = this, + filterParamNames = {}, + getSelectedParameter = function ( filters ) { + var item, + selected = []; + + // Find if any are selected + $.each( filters, function ( name, value ) { + if ( value ) { + selected.push( name ); + } + } ); + + item = model.getItemByName( selected[ 0 ] ); + return ( item && item.getParamName() ) || ''; + }; filterRepresentation = filterRepresentation || {}; @@ -438,6 +481,8 @@ result[ this.getName() ] = ( values.length === Object.keys( filterRepresentation ).length ) ? 'all' : values.join( this.getSeparator() ); + } else if ( this.getType() === 'single_option' ) { + result[ this.getName() ] = getSelectedParameter( filterRepresentation ); } return result; @@ -513,6 +558,11 @@ // Otherwise, the filter is selected only if it appears in the parameter values paramValues.indexOf( filterItem.getParamName() ) > -1; } ); + } else if ( this.getType() === 'single_option' ) { + // There is parameter that fits a single filter, or none at all + this.getItems().forEach( function ( filterItem ) { + result[ filterItem.getName() ] = filterItem.getParamName() === paramRepresentation; + } ); } // Go over result and make sure all filters are represented. @@ -524,6 +574,18 @@ return result; }; + /** + * Get item by its filter name + * + * @param {string} filterName Filter name + * @return {mw.rcfilters.dm.FilterItem} Filter item + */ + mw.rcfilters.dm.FilterGroup.prototype.getItemByName = function ( filterName ) { + return this.getItems().filter( function ( item ) { + return item.getName() === filterName; + } )[ 0 ]; + }; + /** * Get item by its parameter name * diff --git a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js index 37cf4ddf05..eb52efb478 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js @@ -349,10 +349,12 @@ viewData.groups.forEach( function ( groupData ) { var group = groupData.name; - model.groups[ group ] = new mw.rcfilters.dm.FilterGroup( - group, - $.extend( true, {}, groupData, { view: viewName } ) - ); + if ( !model.groups[ group ] ) { + model.groups[ group ] = new mw.rcfilters.dm.FilterGroup( + group, + $.extend( true, {}, groupData, { view: viewName } ) + ); + } model.groups[ group ].initializeFilters( groupData.filters, groupData.default ); items = items.concat( model.groups[ group ].getItems() ); @@ -399,7 +401,10 @@ groupModel.getItems().forEach( function ( filterItem ) { model.parameterMap[ filterItem.getParamName() ] = filterItem; } ); - } else if ( groupModel.getType() === 'string_options' ) { + } else if ( + groupModel.getType() === 'string_options' || + groupModel.getType() === 'single_option' + ) { // Group model.parameterMap[ groupModel.getName() ] = groupModel; } diff --git a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.MenuSelectWidget.js b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.MenuSelectWidget.js index dda4ac5004..8357317548 100644 --- a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.MenuSelectWidget.js +++ b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.MenuSelectWidget.js @@ -133,46 +133,50 @@ // Count groups per view $.each( groups, function ( groupName, groupModel ) { - viewGroupCount[ groupModel.getView() ] = viewGroupCount[ groupModel.getView() ] || 0; - viewGroupCount[ groupModel.getView() ]++; + if ( !groupModel.isHidden() ) { + viewGroupCount[ groupModel.getView() ] = viewGroupCount[ groupModel.getView() ] || 0; + viewGroupCount[ groupModel.getView() ]++; + } } ); $.each( groups, function ( groupName, groupModel ) { var currentItems = [], view = groupModel.getView(); - if ( viewGroupCount[ view ] > 1 ) { - // Only add a section header if there is more than - // one group - currentItems.push( - // Group section - new mw.rcfilters.ui.FilterMenuSectionOptionWidget( - widget.controller, - groupModel, - { - $overlay: widget.$overlay - } - ) - ); - } + if ( !groupModel.isHidden() ) { + if ( viewGroupCount[ view ] > 1 ) { + // Only add a section header if there is more than + // one group + currentItems.push( + // Group section + new mw.rcfilters.ui.FilterMenuSectionOptionWidget( + widget.controller, + groupModel, + { + $overlay: widget.$overlay + } + ) + ); + } - // Add items - widget.model.getGroupFilters( groupName ).forEach( function ( filterItem ) { - currentItems.push( - new mw.rcfilters.ui.FilterMenuOptionWidget( - widget.controller, - filterItem, - { - $overlay: widget.$overlay - } - ) - ); - } ); - - // Cache the items per view, so we can switch between them - // without rebuilding the widgets each time - widget.views[ view ] = widget.views[ view ] || []; - widget.views[ view ] = widget.views[ view ].concat( currentItems ); + // Add items + widget.model.getGroupFilters( groupName ).forEach( function ( filterItem ) { + currentItems.push( + new mw.rcfilters.ui.FilterMenuOptionWidget( + widget.controller, + filterItem, + { + $overlay: widget.$overlay + } + ) + ); + } ); + + // Cache the items per view, so we can switch between them + // without rebuilding the widgets each time + widget.views[ view ] = widget.views[ view ] || []; + widget.views[ view ] = widget.views[ view ].concat( currentItems ); + } } ); this.switchView( this.model.getCurrentView() ); diff --git a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js index e801e8c5f7..5212ee9ce6 100644 --- a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js +++ b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js @@ -54,6 +54,15 @@ { name: 'filter8', label: 'group3filter8-label', description: 'group3filter8-desc' }, { name: 'filter9', label: 'group3filter9-label', description: 'group3filter9-desc' } ] + }, { + name: 'group4', + type: 'single_option', + default: 'option1', + filters: [ + { name: 'option1', label: 'group4option1-label', description: 'group4option1-desc' }, + { name: 'option2', label: 'group4option2-label', description: 'group4option2-desc' }, + { name: 'option3', label: 'group4option3-label', description: 'group4option3-desc' } + ] } ], viewsDefinition = { namespaces: { @@ -81,6 +90,7 @@ filter5: '1', filter6: '0', group3: 'filter8', + group4: 'option1', namespace: '' }, baseParamRepresentation = { @@ -91,6 +101,7 @@ filter5: '0', filter6: '0', group3: '', + group4: '', namespace: '' }, baseFilterRepresentation = { @@ -103,6 +114,9 @@ group3__filter7: false, group3__filter8: false, group3__filter9: false, + group4__option1: false, + group4__option2: false, + group4__option3: false, namespace__0: false, namespace__1: false, namespace__2: false, @@ -118,6 +132,9 @@ group3__filter7: { selected: false, conflicted: false, included: false }, group3__filter8: { selected: false, conflicted: false, included: false }, group3__filter9: { selected: false, conflicted: false, included: false }, + group4__option1: { selected: false, conflicted: false, included: false }, + group4__option2: { selected: false, conflicted: false, included: false }, + group4__option3: { selected: false, conflicted: false, included: false }, namespace__0: { selected: false, conflicted: false, included: false }, namespace__1: { selected: false, conflicted: false, included: false }, namespace__2: { selected: false, conflicted: false, included: false }, @@ -360,6 +377,36 @@ } ), 'All filters selected in "string_option" group returns \'all\'.' ); + + // Reset + model = new mw.rcfilters.dm.FiltersViewModel(); + model.initializeFilters( filterDefinition, viewsDefinition ); + + // Select an option from single_option group + model.toggleFiltersSelected( { + group4__option2: true + } ); + // All filters of the group are selected == this is the same as not selecting any + assert.deepEqual( + model.getParametersFromFilters(), + $.extend( true, {}, baseParamRepresentation, { + group4: 'option2' + } ), + 'Selecting an option from "single_option" group returns that option as a value.' + ); + + // Select a different option from single_option group + model.toggleFiltersSelected( { + group4__option3: true + } ); + // All filters of the group are selected == this is the same as not selecting any + assert.deepEqual( + model.getParametersFromFilters(), + $.extend( true, {}, baseParamRepresentation, { + group4: 'option3' + } ), + 'Selecting a different option from "single_option" group changes the selection.' + ); } ); QUnit.test( 'getParametersFromFilters (custom object)', function ( assert ) { @@ -396,7 +443,26 @@ { name: 'filter8', label: 'Hide filter 8', description: '' }, { name: 'filter9', label: 'Hide filter 9', description: '' } ] + }, { + name: 'group4', + title: 'Group 4', + type: 'single_option', + filters: [ + { name: 'filter10', label: 'Hide filter 10', description: '' }, + { name: 'filter11', label: 'Hide filter 11', description: '' }, + { name: 'filter12', label: 'Hide filter 12', description: '' } + ] } ], + baseResult = { + hidefilter1: '0', + hidefilter2: '0', + hidefilter3: '0', + hidefilter4: '0', + hidefilter5: '0', + hidefilter6: '0', + group3: '', + group4: '' + }, cases = [ { // This is mocking the cases above, both @@ -413,17 +479,12 @@ group3__filter8: true, group3__filter9: false }, - expected: { + expected: $.extend( true, {}, baseResult, { // Group 1 (two selected, the others are true) - hidefilter1: '0', - hidefilter2: '0', hidefilter3: '1', - // Group 2 (nothing is selected, all false) - hidefilter4: '0', - hidefilter5: '0', - hidefilter6: '0', + // Group 3 (two selected) group3: 'filter7,filter8' - }, + } ), msg: 'Given an explicit (complete) filter state object, the result is the same as if the object given represented the model state.' }, { @@ -432,30 +493,35 @@ input: { group1__hidefilter1: 1 }, - expected: { + expected: $.extend( true, {}, baseResult, { // Group 1 (one selected, the others are true) - hidefilter1: '0', hidefilter2: '1', - hidefilter3: '1', - // Group 2 (nothing is selected, all false) - hidefilter4: '0', - hidefilter5: '0', - hidefilter6: '0', - group3: '' - }, + hidefilter3: '1' + } ), msg: 'Given an explicit (incomplete) filter state object, the result is the same as if the object give represented the model state.' }, { - input: {}, - expected: { - hidefilter1: '0', - hidefilter2: '0', - hidefilter3: '0', - hidefilter4: '0', - hidefilter5: '0', - hidefilter6: '0', - group3: '' + input: { + group4__filter10: true }, + expected: $.extend( true, {}, baseResult, { + group4: 'filter10' + } ), + msg: 'Given a single value for "single_option" that option is represented in the result.' + }, + { + input: { + group4__filter10: true, + group4__filter11: true + }, + expected: $.extend( true, {}, baseResult, { + group4: 'filter10' + } ), + msg: 'Given more than one true value for "single_option" (which should not happen!) only the first value counts, and the second is ignored.' + }, + { + input: {}, + expected: baseResult, msg: 'Given an explicit empty object, the result is all filters set to their falsey unselected value.' } ]; @@ -630,6 +696,47 @@ } ), 'A \'string_options\' parameter containing an invalid value, results in the invalid value ignored and the valid corresponding filters checked.' ); + + model.toggleFiltersSelected( + model.getFiltersFromParameters( { + group4: 'option1' + } ) + ); + assert.deepEqual( + model.getSelectedState(), + $.extend( {}, baseFilterRepresentation, { + group4__option1: true + } ), + 'A \'single_option\' parameter reflects a single selected value.' + ); + + assert.deepEqual( + model.getFiltersFromParameters( { + group4: 'option1,option2' + } ), + baseFilterRepresentation, + 'An invalid \'single_option\' parameter is ignored.' + ); + + // Change to one value + model.toggleFiltersSelected( + model.getFiltersFromParameters( { + group4: 'option1' + } ) + ); + // Change again to another value + model.toggleFiltersSelected( + model.getFiltersFromParameters( { + group4: 'option2' + } ) + ); + assert.deepEqual( + model.getSelectedState(), + $.extend( {}, baseFilterRepresentation, { + group4__option2: true + } ), + 'A \'single_option\' parameter always reflects the latest selected value.' + ); } ); QUnit.test( 'sanitizeStringOptionGroup', function ( assert ) { -- 2.20.1