From: Moriel Schottlender Date: Fri, 9 Dec 2016 01:06:23 +0000 (-0800) Subject: RCFilters UI: Read default states of filters X-Git-Tag: 1.31.0-rc.0~4238^2 X-Git-Url: http://git.cyclocoop.org/%24image?a=commitdiff_plain;h=0972ef625d9fc8e077aeb70255ee11106893488a;p=lhc%2Fweb%2Fwiklou.git RCFilters UI: Read default states of filters Add the functionality to read the default state of filters and preserve them, so that they are considered when either base state is requested or when filters are explicitly told to revert to default. Bug: T149391 Bug: T144448 Change-Id: I9e8e3430ca2f80d3f67422681e8fb69a43ee4bef --- diff --git a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterItem.js b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterItem.js index f6fef5b714..5dfb68df99 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterItem.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterItem.js @@ -10,10 +10,10 @@ * @cfg {string} [group] The group this item belongs to * @cfg {string} [label] The label for the filter * @cfg {string} [description] The description of the filter - * @cfg {boolean} [selected] Filter is selected * @cfg {boolean} [active=true] The filter is active and affecting the result * @cfg {string[]} [excludes=[]] A list of filter names this filter, if * selected, makes inactive. + * @cfg {boolean} [default] The default state of this filter */ mw.rcfilters.dm.FilterItem = function MwRcfiltersDmFilterItem( name, config ) { config = config || {}; @@ -25,10 +25,11 @@ this.group = config.group || ''; this.label = config.label || this.name; this.description = config.description; + this.default = !!config.default; - this.selected = !!config.selected; this.active = config.active === undefined ? true : !!config.active; this.excludes = config.excludes || []; + this.selected = this.default; }; /* Initialization */ @@ -82,6 +83,15 @@ return this.description; }; + /** + * Get the default value of this filter + * + * @return {boolean} Filter default + */ + mw.rcfilters.dm.FilterItem.prototype.getDefault = function () { + return this.default; + }; + /** * Get the selected state of this filter * 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 59a34f429e..2496961de0 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js @@ -14,6 +14,7 @@ this.groups = {}; this.excludedByMap = {}; + this.defaultParams = {}; // Events this.aggregate( { update: 'filterItemUpdate' } ); @@ -139,7 +140,7 @@ * @param {Object} filters Filter group definition */ mw.rcfilters.dm.FiltersViewModel.prototype.initializeFilters = function ( filters ) { - var i, filterItem, excludedFilters, + var i, filterItem, selectedFilterNames, excludedFilters, model = this, items = [], addToMap = function ( excludedFilters ) { @@ -163,6 +164,7 @@ model.groups[ group ].separator = data.separator || '|'; model.groups[ group ].exclusionType = data.exclusionType || 'default'; + selectedFilterNames = []; for ( i = 0; i < data.filters.length; i++ ) { excludedFilters = data.filters[ i ].excludes || []; @@ -171,18 +173,38 @@ label: data.filters[ i ].label, description: data.filters[ i ].description, selected: data.filters[ i ].selected, - excludes: excludedFilters + excludes: excludedFilters, + 'default': data.filters[ i ].default } ); // Map filters and what excludes them addToMap( excludedFilters ); + if ( data.type === 'send_unselected_if_any' ) { + // Store the default parameter state + // For this group type, parameter values are direct + model.defaultParams[ data.filters[ i ].name ] = Number( !!data.filters[ i ].default ); + } else if ( + data.type === 'string_options' && + data.filters[ i ].default + ) { + selectedFilterNames.push( data.filters[ i ].name ); + } + model.groups[ group ].filters.push( filterItem ); items.push( filterItem ); } + + if ( data.type === 'string_options' ) { + // Store the default parameter group state + // For this group, the parameter is group name and value is the names + // of selected items + model.defaultParams[ group ] = model.sanitizeStringOptionGroup( group, selectedFilterNames ).join( model.groups[ group ].separator ); + } } ); this.addItems( items ); + this.emit( 'initialize' ); }; @@ -302,16 +324,37 @@ return result; }; + /** + * Get the default parameters object + * + * @return {Object} Default parameter values + */ + mw.rcfilters.dm.FiltersViewModel.prototype.getDefaultParams = function () { + return this.defaultParams; + }; + + /** + * Set all filter states to default values + */ + mw.rcfilters.dm.FiltersViewModel.prototype.setFiltersToDefaults = function () { + var defaultFilterStates = this.getFiltersFromParameters( this.getDefaultParams() ); + + this.updateFilters( defaultFilterStates ); + }; + /** * Analyze the groups and their filters and output an object representing * the state of the parameters they represent. * + * @param {Object} [filterGroups] An object defining the filter groups to + * translate to parameters. Its structure must follow that of this.groups + * see #getFilterGroups * @return {Object} Parameter state object */ - mw.rcfilters.dm.FiltersViewModel.prototype.getParametersFromFilters = function () { + mw.rcfilters.dm.FiltersViewModel.prototype.getParametersFromFilters = function ( filterGroups ) { var i, filterItems, anySelected, values, result = {}, - groupItems = this.getFilterGroups(); + groupItems = filterGroups || this.getFilterGroups(); $.each( groupItems, function ( group, data ) { filterItems = data.filters; @@ -353,6 +396,7 @@ * Remove duplicates and make sure to only use valid * values. * + * @private * @param {string} groupName Group name * @param {string[]} valueArray Array of values * @return {string[]} Array of valid values @@ -389,7 +433,7 @@ /** * This is the opposite of the #getParametersFromFilters method; this goes over - * the parameters and translates into a selected/unselected value in the filters. + * the given parameters and translates into a selected/unselected value in the filters. * * @param {Object} params Parameters query object * @return {Object} Filter state object @@ -398,9 +442,8 @@ var i, filterItem, groupMap = {}, model = this, - base = this.getParametersFromFilters(), - // Start with current state - result = this.getSelectedState(); + base = this.getDefaultParams(), + result = {}; params = $.extend( {}, base, params ); diff --git a/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js b/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js index ea44b8b962..025dc89bc3 100644 --- a/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js +++ b/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js @@ -6,7 +6,6 @@ */ mw.rcfilters.Controller = function MwRcfiltersController( model ) { this.model = model; - // TODO: When we are ready, update the URL when a filter is updated // this.model.connect( this, { itemUpdate: 'updateURL' } ); }; @@ -20,6 +19,8 @@ mw.rcfilters.Controller.prototype.initialize = function () { var uri = new mw.Uri(); + // Give the model a full parameter state from which to + // update the filters this.model.updateFilters( // Translate the url params to filter select states this.model.getFiltersFromParameters( uri.query ) diff --git a/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js b/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js index 34df2f5a5b..1f02bc45c4 100644 --- a/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js +++ b/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js @@ -88,12 +88,14 @@ { name: 'hidebots', label: mw.msg( 'rcfilters-filter-bots-label' ), - description: mw.msg( 'rcfilters-filter-bots-description' ) + description: mw.msg( 'rcfilters-filter-bots-description' ), + 'default': true }, { name: 'hidehumans', label: mw.msg( 'rcfilters-filter-humans-label' ), - description: mw.msg( 'rcfilters-filter-humans-description' ) + description: mw.msg( 'rcfilters-filter-humans-description' ), + 'default': false } ] }, @@ -120,22 +122,26 @@ { name: 'hidepageedits', label: mw.msg( 'rcfilters-filter-pageedits-label' ), - description: mw.msg( 'rcfilters-filter-pageedits-description' ) + description: mw.msg( 'rcfilters-filter-pageedits-description' ), + 'default': false }, { name: 'hidenewpages', label: mw.msg( 'rcfilters-filter-newpages-label' ), - description: mw.msg( 'rcfilters-filter-newpages-description' ) + description: mw.msg( 'rcfilters-filter-newpages-description' ), + 'default': false }, { name: 'hidecategorization', label: mw.msg( 'rcfilters-filter-categorization-label' ), - description: mw.msg( 'rcfilters-filter-categorization-description' ) + description: mw.msg( 'rcfilters-filter-categorization-description' ), + 'default': true }, { name: 'hidelog', label: mw.msg( 'rcfilters-filter-logactions-label' ), - description: mw.msg( 'rcfilters-filter-logactions-description' ) + description: mw.msg( 'rcfilters-filter-logactions-description' ), + 'default': false } ] } diff --git a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterWrapperWidget.js b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterWrapperWidget.js index 3353e54820..44531a136f 100644 --- a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterWrapperWidget.js +++ b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterWrapperWidget.js @@ -99,6 +99,7 @@ */ mw.rcfilters.ui.FilterWrapperWidget.prototype.onModelInitialize = function () { var items, + wrapper = this, filters = this.model.getItems(); // Reset @@ -113,6 +114,16 @@ } ); this.capsule.getMenu().addItems( items ); + + // Add defaults to capsule. We have to do this + // after we added to the capsule menu, since that's + // how the capsule multiselect widget knows which + // object to add + filters.forEach( function ( filterItem ) { + if ( filterItem.isSelected() ) { + wrapper.addCapsuleItemFromName( filterItem.getName() ); + } + } ); }; /** @@ -124,11 +135,7 @@ var widget = this; if ( item.isSelected() ) { - this.capsule.addItemsFromData( [ item.getName() ] ); - - // Deal with active/inactive capsule filter items - this.capsule.getItemFromData( item.getName() ).$element - .toggleClass( 'mw-rcfilters-ui-filterCapsuleMultiselectWidget-item-inactive', !item.isActive() ); + this.addCapsuleItemFromName( item.getName() ); } else { this.capsule.removeItemsFromData( [ item.getName() ] ); } @@ -140,4 +147,19 @@ } } ); }; + + /** + * Add a capsule item by its filter name + * + * @param {string} itemName Filter name + */ + mw.rcfilters.ui.FilterWrapperWidget.prototype.addCapsuleItemFromName = function ( itemName ) { + var item = this.model.getItemByName( itemName ); + + this.capsule.addItemsFromData( [ itemName ] ); + + // Deal with active/inactive capsule filter items + this.capsule.getItemFromData( itemName ).$element + .toggleClass( 'mw-rcfilters-ui-filterCapsuleMultiselectWidget-item-inactive', !item.isActive() ); + }; }( mediaWiki ) ); 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 09e0f07407..62d247aeba 100644 --- a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js +++ b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js @@ -420,7 +420,8 @@ { name: 'hidefilter1', label: 'Show filter 1', - description: 'Description of Filter 1 in Group 1' + description: 'Description of Filter 1 in Group 1', + default: true }, { name: 'hidefilter2', @@ -430,7 +431,8 @@ { name: 'hidefilter3', label: 'Show filter 3', - description: 'Description of Filter 3 in Group 1' + description: 'Description of Filter 3 in Group 1', + default: true } ] }, @@ -446,7 +448,8 @@ { name: 'hidefilter5', label: 'Show filter 5', - description: 'Description of Filter 2 in Group 2' + description: 'Description of Filter 2 in Group 2', + default: true }, { name: 'hidefilter6', @@ -468,7 +471,8 @@ { name: 'filter8', label: 'Group 3: Filter 2', - description: 'Description of Filter 2 in Group 3' + description: 'Description of Filter 2 in Group 3', + default: true }, { name: 'filter9', @@ -478,62 +482,40 @@ ] } }, + defaultFilterRepresentation = { + // Group 1 and 2, "send_unselected_if_any", the values of the filters are "flipped" from the values of the parameters + hidefilter1: false, + hidefilter2: true, + hidefilter3: false, + hidefilter4: true, + hidefilter5: false, + hidefilter6: true, + // Group 3, "string_options", default values correspond to parameters and filters + filter7: false, + filter8: true, + filter9: false + }, model = new mw.rcfilters.dm.FiltersViewModel(); model.initializeFilters( definition ); - // Empty query = empty filter definition + // Empty query = only default values assert.deepEqual( model.getFiltersFromParameters( {} ), - { - hidefilter1: false, // The text is "show filter 1" - hidefilter2: false, // The text is "show filter 2" - hidefilter3: false, // The text is "show filter 3" - hidefilter4: false, // The text is "show filter 4" - hidefilter5: false, // The text is "show filter 5" - hidefilter6: false, // The text is "show filter 6" - filter7: false, - filter8: false, - filter9: false - }, - 'Empty parameter query results in filters in initial state' + defaultFilterRepresentation, + 'Empty parameter query results in filters in initial default state' ); assert.deepEqual( model.getFiltersFromParameters( { - hidefilter1: '1' - } ), - { - hidefilter1: false, // The text is "show filter 1" - hidefilter2: true, // The text is "show filter 2" - hidefilter3: true, // The text is "show filter 3" - hidefilter4: false, // The text is "show filter 4" - hidefilter5: false, // The text is "show filter 5" - hidefilter6: false, // The text is "show filter 6" - filter7: false, - filter8: false, - filter9: false - }, - 'One falsey parameter in a group makes the rest of the filters in the group truthy (checked) in the interface' - ); - - assert.deepEqual( - model.getFiltersFromParameters( { - hidefilter1: '1', hidefilter2: '1' } ), - { + $.extend( {}, defaultFilterRepresentation, { hidefilter1: false, // The text is "show filter 1" hidefilter2: false, // The text is "show filter 2" - hidefilter3: true, // The text is "show filter 3" - hidefilter4: false, // The text is "show filter 4" - hidefilter5: false, // The text is "show filter 5" - hidefilter6: false, // The text is "show filter 6" - filter7: false, - filter8: false, - filter9: false - }, - 'Two falsey parameters in a \'send_unselected_if_any\' group makes the rest of the filters in the group truthy (checked) in the interface' + hidefilter3: false // The text is "show filter 3" + } ), + 'One truthy parameter in a group whose other parameters are true by default makes the rest of the filters in the group false (unchecked)' ); assert.deepEqual( @@ -542,23 +524,23 @@ hidefilter2: '1', hidefilter3: '1' } ), - { - // TODO: This will have to be represented as a different state, though. + $.extend( {}, defaultFilterRepresentation, { hidefilter1: false, // The text is "show filter 1" hidefilter2: false, // The text is "show filter 2" - hidefilter3: false, // The text is "show filter 3" - hidefilter4: false, // The text is "show filter 4" - hidefilter5: false, // The text is "show filter 5" - hidefilter6: false, // The text is "show filter 6" - filter7: false, - filter8: false, - filter9: false - }, + hidefilter3: false // The text is "show filter 3" + } ), 'All paremeters in the same \'send_unselected_if_any\' group false is equivalent to none are truthy (checked) in the interface' ); // The ones above don't update the model, so we have a clean state. - + // getFiltersFromParameters is stateless; any change is unaffected by the current state + // This test is demonstrating wrong usage of the method; + // We should be aware that getFiltersFromParameters is stateless, + // so each call gives us a filter state that only reflects the query given. + // This means that the two calls to updateFilters() below collide. + // The result of the first is overridden by the result of the second, + // since both get a full state object from getFiltersFromParameters that **only** relates + // to the input it receives. model.updateFilters( model.getFiltersFromParameters( { hidefilter1: '1' @@ -567,28 +549,19 @@ model.updateFilters( model.getFiltersFromParameters( { - hidefilter3: '1' + hidefilter6: '1' } ) ); - // 1 and 3 are separately unchecked via hide parameters, 2 should still be - // checked. - // This can simulate separate filters in the same group being hidden different - // ways (e.g. preferences and URL). + // The result here is ignoring the first updateFilters call + // We should receive default values + hidefilter6 as false assert.deepEqual( model.getSelectedState(), - { - hidefilter1: false, // The text is "show filter 1" - hidefilter2: true, // The text is "show filter 2" - hidefilter3: false, // The text is "show filter 3" - hidefilter4: false, // The text is "show filter 4" - hidefilter5: false, // The text is "show filter 5" - hidefilter6: false, // The text is "show filter 6" - filter7: false, - filter8: false, - filter9: false - }, - 'After unchecking 2 of 3 \'send_unselected_if_any\' filters via separate updateFilters calls, only the remaining one is still checked.' + $.extend( {}, defaultFilterRepresentation, { + hidefilter5: false, + hidefilter6: false + } ), + 'getFiltersFromParameters does not care about previous or existing state.' ); // Reset @@ -597,12 +570,12 @@ model.updateFilters( model.getFiltersFromParameters( { - hidefilter1: '1' + hidefilter1: '0' } ) ); model.updateFilters( model.getFiltersFromParameters( { - hidefilter1: '0' + hidefilter1: '1' } ) ); @@ -610,18 +583,8 @@ // override. assert.deepEqual( model.getSelectedState(), - { - hidefilter1: false, // The text is "show filter 1" - hidefilter2: false, // The text is "show filter 2" - hidefilter3: false, // The text is "show filter 3" - hidefilter4: false, // The text is "show filter 4" - hidefilter5: false, // The text is "show filter 5" - hidefilter6: false, // The text is "show filter 6" - filter7: false, - filter8: false, - filter9: false - }, - 'After unchecking then checking a \'send_unselected_if_any\' filter (without touching other filters in that group), all are checked' + defaultFilterRepresentation, + 'After checking and then unchecking a \'send_unselected_if_any\' filter (without touching other filters in that group), results are default' ); model.updateFilters( @@ -631,17 +594,11 @@ ); assert.deepEqual( model.getSelectedState(), - { - hidefilter1: false, // The text is "show filter 1" - hidefilter2: false, // The text is "show filter 2" - hidefilter3: false, // The text is "show filter 3" - hidefilter4: false, // The text is "show filter 4" - hidefilter5: false, // The text is "show filter 5" - hidefilter6: false, // The text is "show filter 6" + $.extend( {}, defaultFilterRepresentation, { filter7: true, filter8: false, filter9: false - }, + } ), 'A \'string_options\' parameter containing 1 value, results in the corresponding filter as checked' ); @@ -652,17 +609,11 @@ ); assert.deepEqual( model.getSelectedState(), - { - hidefilter1: false, // The text is "show filter 1" - hidefilter2: false, // The text is "show filter 2" - hidefilter3: false, // The text is "show filter 3" - hidefilter4: false, // The text is "show filter 4" - hidefilter5: false, // The text is "show filter 5" - hidefilter6: false, // The text is "show filter 6" + $.extend( {}, defaultFilterRepresentation, { filter7: true, filter8: true, filter9: false - }, + } ), 'A \'string_options\' parameter containing 2 values, results in both corresponding filters as checked' ); @@ -673,38 +624,26 @@ ); assert.deepEqual( model.getSelectedState(), - { - hidefilter1: false, // The text is "show filter 1" - hidefilter2: false, // The text is "show filter 2" - hidefilter3: false, // The text is "show filter 3" - hidefilter4: false, // The text is "show filter 4" - hidefilter5: false, // The text is "show filter 5" - hidefilter6: false, // The text is "show filter 6" + $.extend( {}, defaultFilterRepresentation, { filter7: false, filter8: false, filter9: false - }, + } ), 'A \'string_options\' parameter containing all values, results in all filters of the group as unchecked.' ); model.updateFilters( model.getFiltersFromParameters( { - group3: 'filter7,filter8,filter9' + group3: 'filter7,all,filter9' } ) ); assert.deepEqual( model.getSelectedState(), - { - hidefilter1: false, // The text is "show filter 1" - hidefilter2: false, // The text is "show filter 2" - hidefilter3: false, // The text is "show filter 3" - hidefilter4: false, // The text is "show filter 4" - hidefilter5: false, // The text is "show filter 5" - hidefilter6: false, // The text is "show filter 6" + $.extend( {}, defaultFilterRepresentation, { filter7: false, filter8: false, filter9: false - }, + } ), 'A \'string_options\' parameter containing the value \'all\', results in all filters of the group as unchecked.' ); @@ -715,17 +654,11 @@ ); assert.deepEqual( model.getSelectedState(), - { - hidefilter1: false, // The text is "show filter 1" - hidefilter2: false, // The text is "show filter 2" - hidefilter3: false, // The text is "show filter 3" - hidefilter4: false, // The text is "show filter 4" - hidefilter5: false, // The text is "show filter 5" - hidefilter6: false, // The text is "show filter 6" + $.extend( {}, defaultFilterRepresentation, { filter7: true, filter8: false, filter9: true - }, + } ), 'A \'string_options\' parameter containing an invalid value, results in the invalid value ignored and the valid corresponding filters checked.' ); } ); @@ -777,7 +710,7 @@ ); } ); - QUnit.test( 'reapplyActiveFilters - "default" exclusion rules', function ( assert ) { + QUnit.test( 'setFiltersToDefaults', function ( assert ) { var definition = { group1: { title: 'Group 1', @@ -787,7 +720,8 @@ { name: 'hidefilter1', label: 'Show filter 1', - description: 'Description of Filter 1 in Group 1' + description: 'Description of Filter 1 in Group 1', + default: true }, { name: 'hidefilter2', @@ -797,7 +731,8 @@ { name: 'hidefilter3', label: 'Show filter 3', - description: 'Description of Filter 3 in Group 1' + description: 'Description of Filter 3 in Group 1', + default: true } ] }, @@ -813,7 +748,8 @@ { name: 'hidefilter5', label: 'Show filter 5', - description: 'Description of Filter 2 in Group 2' + description: 'Description of Filter 2 in Group 2', + default: true }, { name: 'hidefilter6', @@ -831,15 +767,15 @@ model.getFullState(), { // Group 1 - hidefilter1: { selected: false, active: true }, + hidefilter1: { selected: true, active: true }, hidefilter2: { selected: false, active: true }, - hidefilter3: { selected: false, active: true }, + hidefilter3: { selected: true, active: true }, // Group 2 hidefilter4: { selected: false, active: true }, - hidefilter5: { selected: false, active: true }, + hidefilter5: { selected: true, active: true }, hidefilter6: { selected: false, active: true }, }, - 'Initial state: all filters are active.' + 'Initial state: all filters are active, and select states are default.' ); // Default behavior for 'exclusion' type with only 1 item selected, means that: @@ -956,6 +892,19 @@ ] } }, + defaultFilterRepresentation = { + // Group 1 and 2, "send_unselected_if_any", the values of the filters are "flipped" from the values of the parameters + hidefilter1: false, + hidefilter2: true, + hidefilter3: false, + hidefilter4: true, + hidefilter5: false, + hidefilter6: true, + // Group 3, "string_options", default values correspond to parameters and filters + filter7: false, + filter8: true, + filter9: false + }, model = new mw.rcfilters.dm.FiltersViewModel(); model.initializeFilters( definition );