From f255df5070dbac4b0bf54341a17463b8790b111a Mon Sep 17 00:00:00 2001 From: Stephane Bisson Date: Wed, 1 Mar 2017 07:04:05 -0500 Subject: [PATCH] RC filters: update the state of the app on popstate. Also re-fetch changes list. Bug: T153949 Change-Id: Id3d4ea2a4de6074ae1c15cadb74e7a324a39e7ff --- .../dm/mw.rcfilters.dm.FilterItem.js | 9 +++ .../dm/mw.rcfilters.dm.FiltersViewModel.js | 41 +++++------ .../mw.rcfilters.Controller.js | 73 +++++++++++++++---- .../mediawiki.rcfilters/mw.rcfilters.init.js | 3 + .../ui/mw.rcfilters.ui.CapsuleItemWidget.js | 3 +- .../dm.FiltersViewModel.test.js | 60 +++++++-------- 6 files changed, 119 insertions(+), 70 deletions(-) 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 18f1299377..0df34f8f16 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterItem.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterItem.js @@ -378,4 +378,13 @@ mw.rcfilters.dm.FilterItem.prototype.isHighlightSupported = function () { return !!this.getCssClass(); }; + + /** + * Check if the filter is currently highlighted + * + * @return {boolean} + */ + mw.rcfilters.dm.FilterItem.prototype.isHighlighted = function () { + return this.isHighlightEnabled() && !!this.getHighlightColor(); + }; }( mediaWiki ) ); 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 2afe286095..5be3656988 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js @@ -359,7 +359,7 @@ mw.rcfilters.dm.FiltersViewModel.prototype.setFiltersToDefaults = function () { var defaultFilterStates = this.getFiltersFromParameters( this.getDefaultParams() ); - this.updateFilters( defaultFilterStates ); + this.toggleFiltersSelected( defaultFilterStates ); }; /** @@ -418,7 +418,7 @@ * are the selected highlight colors. */ mw.rcfilters.dm.FiltersViewModel.prototype.getHighlightParameters = function () { - var result = { highlight: this.isHighlightEnabled() }; + var result = { highlight: Number( this.isHighlightEnabled() ) }; this.getItems().forEach( function ( filterItem ) { result[ filterItem.getName() + '_color' ] = filterItem.getHighlightColor(); @@ -472,15 +472,10 @@ * @return {boolean} Current filters are all empty */ mw.rcfilters.dm.FiltersViewModel.prototype.areCurrentFiltersEmpty = function () { - var model = this; - // Check if there are either any selected items or any items // that have highlight enabled return !this.getItems().some( function ( filterItem ) { - return ( - filterItem.isSelected() || - ( model.isHighlightEnabled() && filterItem.getHighlightColor() ) - ); + return filterItem.isSelected() || filterItem.isHighlighted(); } ); }; @@ -602,14 +597,19 @@ * This is equivalent to display all. */ mw.rcfilters.dm.FiltersViewModel.prototype.emptyAllFilters = function () { - var filters = {}; - this.getItems().forEach( function ( filterItem ) { - filters[ filterItem.getName() ] = false; - } ); + this.toggleFilterSelected( filterItem.getName(), false ); + }.bind( this ) ); + }; - // Update filters - this.updateFilters( filters ); + /** + * Toggle selected state of one item + * + * @param {string} name Name of the filter item + * @param {boolean} [isSelected] Filter selected state + */ + mw.rcfilters.dm.FiltersViewModel.prototype.toggleFilterSelected = function ( name, isSelected ) { + this.getItemByName( name ).toggleSelected( isSelected ); }; /** @@ -617,13 +617,10 @@ * * @param {Object} filterDef Filter definitions */ - mw.rcfilters.dm.FiltersViewModel.prototype.updateFilters = function ( filterDef ) { - var name, filterItem; - - for ( name in filterDef ) { - filterItem = this.getItemByName( name ); - filterItem.toggleSelected( filterDef[ name ] ); - } + mw.rcfilters.dm.FiltersViewModel.prototype.toggleFiltersSelected = function ( filterDef ) { + Object.keys( filterDef ).forEach( function ( name ) { + this.toggleFilterSelected( name, filterDef[ name ] ); + }.bind( this ) ); }; /** @@ -726,7 +723,7 @@ * @return {boolean} */ mw.rcfilters.dm.FiltersViewModel.prototype.isHighlightEnabled = function () { - return this.highlightEnabled; + return !!this.highlightEnabled; }; /** diff --git a/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js b/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js index 1c0590901d..4c3d35f5b6 100644 --- a/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js +++ b/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js @@ -20,13 +20,20 @@ * @param {Object} filterStructure Filter definition and structure for the model */ mw.rcfilters.Controller.prototype.initialize = function ( filterStructure ) { - var uri = new mw.Uri(); - // Initialize the model this.filtersModel.initializeFilters( filterStructure ); + this.updateStateBasedOnUrl(); + }; + + /** + * Update filter state (selection and highlighting) based + * on current URL and default values. + */ + mw.rcfilters.Controller.prototype.updateStateBasedOnUrl = function () { + var uri = new mw.Uri(); // Set filter states based on defaults and URL params - this.filtersModel.updateFilters( + this.filtersModel.toggleFiltersSelected( this.filtersModel.getFiltersFromParameters( // Merge defaults with URL params for initialization $.extend( @@ -43,11 +50,11 @@ this.filtersModel.toggleHighlight( !!uri.query.highlight ); this.filtersModel.getItems().forEach( function ( filterItem ) { var color = uri.query[ filterItem.getName() + '_color' ]; - if ( !color ) { - return; + if ( color ) { + filterItem.setHighlightColor( color ); + } else { + filterItem.clearHighlightColor(); } - - filterItem.setHighlightColor( color ); } ); // Check all filter interactions @@ -84,19 +91,17 @@ * @param {boolean} [isSelected] Filter selected state */ mw.rcfilters.Controller.prototype.toggleFilterSelect = function ( filterName, isSelected ) { - var obj = {}, - filterItem = this.filtersModel.getItemByName( filterName ); + var filterItem = this.filtersModel.getItemByName( filterName ); isSelected = isSelected === undefined ? !filterItem.isSelected() : isSelected; if ( filterItem.isSelected() !== isSelected ) { - obj[ filterName ] = isSelected; - this.filtersModel.updateFilters( obj ); + this.filtersModel.toggleFilterSelected( filterName, isSelected ); this.updateChangesList(); // Check filter interactions - this.filtersModel.reassessFilterInteractions( this.filtersModel.getItemByName( filterName ) ); + this.filtersModel.reassessFilterInteractions( filterItem ); } }; @@ -112,14 +117,22 @@ * @param {Object} [params] Extra parameters to add to the API call */ mw.rcfilters.Controller.prototype.updateURL = function ( params ) { - var uri; + var updatedUri, + notEquivalent = function ( obj1, obj2 ) { + var keys = Object.keys( obj1 ).concat( Object.keys( obj2 ) ); + return keys.some( function ( key ) { + return obj1[ key ] != obj2[ key ]; // eslint-disable-line eqeqeq + } ); + }; params = params || {}; - uri = this.getUpdatedUri(); - uri.extend( params ); + updatedUri = this.getUpdatedUri(); + updatedUri.extend( params ); - window.history.pushState( { tag: 'rcfilters' }, document.title, uri.toString() ); + if ( notEquivalent( updatedUri.query, new mw.Uri().query ) ) { + window.history.pushState( { tag: 'rcfilters' }, document.title, updatedUri.toString() ); + } }; /** @@ -247,4 +260,32 @@ this.filtersModel.clearHighlightColor( filterName ); this.updateURL(); }; + + /** + * Clear both highlight and selection of a filter + * + * @param {string} filterName Name of the filter item + */ + mw.rcfilters.Controller.prototype.clearFilter = function ( filterName ) { + var filterItem = this.filtersModel.getItemByName( filterName ); + + if ( filterItem.isSelected() || filterItem.isHighlighted() ) { + this.filtersModel.clearHighlightColor( filterName ); + this.filtersModel.toggleFilterSelected( filterName, false ); + this.updateChangesList(); + this.filtersModel.reassessFilterInteractions( filterItem ); + } + }; + + /** + * Synchronize the URL with the current state of the filters + * without adding an history entry. + */ + mw.rcfilters.Controller.prototype.replaceUrl = function () { + window.history.replaceState( + { tag: 'rcfilters' }, + document.title, + this.getUpdatedUri().toString() + ); + }; }( mediaWiki, jQuery ) ); diff --git a/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js b/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js index 255d93b692..a0b785d32c 100644 --- a/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js +++ b/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js @@ -193,6 +193,7 @@ $( '.rcfilters-head' ).addClass( 'mw-rcfilters-ui-ready' ); window.addEventListener( 'popstate', function () { + controller.updateStateBasedOnUrl(); controller.updateChangesList(); } ); @@ -200,6 +201,8 @@ 'href', 'https://www.mediawiki.org/wiki/Special:MyLanguage/Help:New_filters_for_edit_review' ); + + controller.replaceUrl(); } }; diff --git a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.CapsuleItemWidget.js b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.CapsuleItemWidget.js index 05f2f66639..e7e3751dec 100644 --- a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.CapsuleItemWidget.js +++ b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.CapsuleItemWidget.js @@ -111,8 +111,7 @@ } // Respond to user removing the filter - this.controller.toggleFilterSelect( this.model.getName(), false ); - this.controller.clearHighlightColor( this.model.getName() ); + this.controller.clearFilter( this.model.getName() ); }; mw.rcfilters.ui.CapsuleItemWidget.prototype.setHighlightColor = function () { 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 3a940d081e..ad0ed54484 100644 --- a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js +++ b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js @@ -79,7 +79,7 @@ 'Initial state of filters' ); - model.updateFilters( { + model.toggleFiltersSelected( { group1filter1: true, group2filter2: true, group3filter1: true @@ -276,7 +276,7 @@ ); // Select 1 filter - model.updateFilters( { + model.toggleFiltersSelected( { hidefilter1: true, hidefilter2: false, hidefilter3: false, @@ -302,7 +302,7 @@ ); // Select 2 filters - model.updateFilters( { + model.toggleFiltersSelected( { hidefilter1: true, hidefilter2: true, hidefilter3: false, @@ -328,7 +328,7 @@ ); // Select 3 filters - model.updateFilters( { + model.toggleFiltersSelected( { hidefilter1: true, hidefilter2: true, hidefilter3: true, @@ -354,7 +354,7 @@ ); // Select 1 filter from string_options - model.updateFilters( { + model.toggleFiltersSelected( { filter7: true, filter8: false, filter9: false @@ -377,7 +377,7 @@ ); // Select 2 filters from string_options - model.updateFilters( { + model.toggleFiltersSelected( { filter7: true, filter8: true, filter9: false @@ -400,7 +400,7 @@ ); // Select 3 filters from string_options - model.updateFilters( { + model.toggleFiltersSelected( { filter7: true, filter8: true, filter9: true @@ -550,23 +550,23 @@ // 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. + // This means that the two calls to toggleFiltersSelected() 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.toggleFiltersSelected( model.getFiltersFromParameters( { hidefilter1: '1' } ) ); - model.updateFilters( + model.toggleFiltersSelected( model.getFiltersFromParameters( { hidefilter6: '1' } ) ); - // The result here is ignoring the first updateFilters call + // The result here is ignoring the first toggleFiltersSelected call // We should receive default values + hidefilter6 as false assert.deepEqual( model.getSelectedState(), @@ -581,12 +581,12 @@ model = new mw.rcfilters.dm.FiltersViewModel(); model.initializeFilters( definition ); - model.updateFilters( + model.toggleFiltersSelected( model.getFiltersFromParameters( { hidefilter1: '0' } ) ); - model.updateFilters( + model.toggleFiltersSelected( model.getFiltersFromParameters( { hidefilter1: '1' } ) @@ -600,7 +600,7 @@ 'After checking and then unchecking a \'send_unselected_if_any\' filter (without touching other filters in that group), results are default' ); - model.updateFilters( + model.toggleFiltersSelected( model.getFiltersFromParameters( { group3: 'filter7' } ) @@ -615,7 +615,7 @@ 'A \'string_options\' parameter containing 1 value, results in the corresponding filter as checked' ); - model.updateFilters( + model.toggleFiltersSelected( model.getFiltersFromParameters( { group3: 'filter7,filter8' } ) @@ -630,7 +630,7 @@ 'A \'string_options\' parameter containing 2 values, results in both corresponding filters as checked' ); - model.updateFilters( + model.toggleFiltersSelected( model.getFiltersFromParameters( { group3: 'filter7,filter8,filter9' } ) @@ -645,7 +645,7 @@ 'A \'string_options\' parameter containing all values, results in all filters of the group as unchecked.' ); - model.updateFilters( + model.toggleFiltersSelected( model.getFiltersFromParameters( { group3: 'filter7,all,filter9' } ) @@ -660,7 +660,7 @@ 'A \'string_options\' parameter containing the value \'all\', results in all filters of the group as unchecked.' ); - model.updateFilters( + model.toggleFiltersSelected( model.getFiltersFromParameters( { group3: 'filter7,foo,filter9' } ) @@ -797,7 +797,7 @@ 'Initial state: default filters are not selected (controller selects defaults explicitly).' ); - model.updateFilters( { + model.toggleFiltersSelected( { hidefilter1: false, hidefilter3: false } ); @@ -870,7 +870,7 @@ model.initializeFilters( definition ); // Select a filter that has subset with another filter - model.updateFilters( { + model.toggleFiltersSelected( { filter1: true } ); @@ -886,7 +886,7 @@ ); // Select another filter that has a subset with the same previous filter - model.updateFilters( { + model.toggleFiltersSelected( { filter4: true } ); model.reassessFilterInteractions( model.getItemByName( 'filter4' ) ); @@ -903,7 +903,7 @@ ); // Remove one filter (but leave the other) that affects filter2 - model.updateFilters( { + model.toggleFiltersSelected( { filter1: false } ); model.reassessFilterInteractions( model.getItemByName( 'filter1' ) ); @@ -918,7 +918,7 @@ 'Removing a filter only un-includes its subset if there is no other filter affecting.' ); - model.updateFilters( { + model.toggleFiltersSelected( { filter4: false } ); model.reassessFilterInteractions( model.getItemByName( 'filter4' ) ); @@ -994,7 +994,7 @@ ); // Select most (but not all) items in each group - model.updateFilters( { + model.toggleFiltersSelected( { filter1: true, filter2: true, filter4: true, @@ -1009,7 +1009,7 @@ ); // Select all items in 'fullCoverage' group (group2) - model.updateFilters( { + model.toggleFiltersSelected( { filter6: true } ); @@ -1025,7 +1025,7 @@ ); // Select all items in non 'fullCoverage' group (group1) - model.updateFilters( { + model.toggleFiltersSelected( { filter3: true } ); @@ -1041,7 +1041,7 @@ ); // Uncheck an item from each group - model.updateFilters( { + model.toggleFiltersSelected( { filter3: false, filter5: false } ); @@ -1107,7 +1107,7 @@ ); // Select a filter that has a conflict with another - model.updateFilters( { + model.toggleFiltersSelected( { filter1: true // conflicts: filter2, filter4 } ); @@ -1124,7 +1124,7 @@ ); // Select one of the conflicts (both filters are now conflicted and selected) - model.updateFilters( { + model.toggleFiltersSelected( { filter4: true // conflicts: filter 1 } ); model.reassessFilterInteractions( model.getItemByName( 'filter4' ) ); @@ -1141,7 +1141,7 @@ // Select another filter from filter4 group, meaning: // now filter1 no longer conflicts with filter4 - model.updateFilters( { + model.toggleFiltersSelected( { filter6: true // conflicts: filter2 } ); model.reassessFilterInteractions( model.getItemByName( 'filter6' ) ); -- 2.20.1