From: jenkins-bot Date: Thu, 19 Oct 2017 19:39:04 +0000 (+0000) Subject: Merge "RCFilters: refactor highlight state" X-Git-Tag: 1.31.0-rc.0~1714 X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=20fcae98034d264bdc1979d543c9ebf1f1b4506b;hp=605d3add72cbaae042209c17634783452c224414 Merge "RCFilters: refactor highlight state" --- 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 b17355f5ba..d20e2e7f83 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js @@ -93,7 +93,6 @@ */ mw.rcfilters.dm.FilterGroup.prototype.initializeFilters = function ( filterDefinition, groupDefault ) { var defaultParam, - anyHighlighted, supersetMap = {}, model = this, items = []; @@ -191,16 +190,11 @@ } // add highlights to defaultParams - anyHighlighted = false; this.getItems().forEach( function ( filterItem ) { if ( filterItem.isHighlighted() ) { - anyHighlighted = true; this.defaultParams[ filterItem.getName() + '_color' ] = filterItem.getHighlightColor(); } }.bind( this ) ); - if ( anyHighlighted ) { - this.defaultParams.highlight = '1'; - } // Store default filter state based on default params this.defaultFilters = this.getFilterRepresentation( this.getDefaultParams() ); 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 b8e112913a..3a6efe2b1d 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js @@ -403,9 +403,7 @@ this.currentView = 'default'; - if ( this.getHighlightedItems().length > 0 ) { - this.toggleHighlight( true ); - } + this.updateHighlightedState(); // Finish initialization this.emit( 'initialize' ); @@ -439,7 +437,7 @@ filterItem.clearHighlightColor(); } } ); - this.toggleHighlight( !!Number( params.highlight ) ); + this.updateHighlightedState(); // Check all filter interactions this.reassessFilterInteractions(); @@ -456,8 +454,7 @@ true, {}, this.getParametersFromFilters( {} ), - this.getEmptyHighlightParameters(), - { highlight: '0' } + this.getEmptyHighlightParameters() ); } return this.emptyParameterState; @@ -484,11 +481,9 @@ // Highlights Object.keys( this.getEmptyHighlightParameters() ).forEach( function ( param ) { - if ( param !== 'highlight' && parameters[ param ] ) { + if ( parameters[ param ] ) { // If a highlight parameter is not undefined and not null // add it to the result - // Ignore "highlight" parameter because that, we checked already with - // the empty parameter state (and this soon changes to an implicit value) result[ param ] = parameters[ param ]; } } ); @@ -535,12 +530,7 @@ true, {}, this.getParametersFromFilters( this.getSelectedState() ), - this.getHighlightParameters(), - { - // HACK: Add highlight. This is only needed while it's - // stored as an outside state - highlight: String( Number( this.isHighlightEnabled() ) ) - } + this.getHighlightParameters() ) ); if ( removeExcludedParams ) { @@ -614,6 +604,13 @@ return this.getItems().map( function ( item ) { return item.getName(); } ); }; + /** + * Turn the highlight feature on or off + */ + mw.rcfilters.dm.FiltersViewModel.prototype.updateHighlightedState = function () { + this.toggleHighlight( this.getHighlightedItems().length > 0 ); + }; + /** * Get the object that defines groups by their name. * @@ -955,14 +952,16 @@ * are the selected highlight colors. */ mw.rcfilters.dm.FiltersViewModel.prototype.getHighlightParameters = function () { - var result = {}; + var highlightEnabled = this.isHighlightEnabled(), + result = {}; this.getItems().forEach( function ( filterItem ) { if ( filterItem.isHighlightSupported() ) { - result[ filterItem.getName() + '_color' ] = filterItem.getHighlightColor() || null; + result[ filterItem.getName() + '_color' ] = highlightEnabled && filterItem.isHighlighted() ? + filterItem.getHighlightColor() : + null; } } ); - result.highlight = String( Number( this.isHighlightEnabled() ) ); return result; }; @@ -980,31 +979,6 @@ result[ filterItem.getName() + '_color' ] = null; } } ); - result.highlight = '0'; - - return result; - }; - - /** - * Extract the highlight values from given object. Since highlights are - * the same for filter and parameters, it doesn't matter which one is - * given; values will be returned with a full list of the highlights - * with colors or null values. - * - * @param {Object} representation Object containing representation of - * some or all highlight values - * @return {Object} Object where keys are `_color` and values - * are the selected highlight colors. The returned object - * contains all available filters either with a color value - * or with null. - */ - mw.rcfilters.dm.FiltersViewModel.prototype.extractHighlightValues = function ( representation ) { - var result = {}; - - this.getItems().forEach( function ( filterItem ) { - var highlightName = filterItem.getName() + '_color'; - result[ highlightName ] = representation[ highlightName ] || null; - } ); return result; }; @@ -1017,13 +991,15 @@ mw.rcfilters.dm.FiltersViewModel.prototype.getCurrentlyUsedHighlightColors = function () { var result = []; - this.getHighlightedItems().forEach( function ( filterItem ) { - var color = filterItem.getHighlightColor(); + if ( this.isHighlightEnabled() ) { + this.getHighlightedItems().forEach( function ( filterItem ) { + var color = filterItem.getHighlightColor(); - if ( result.indexOf( color ) === -1 ) { - result.push( color ); - } - } ); + if ( result.indexOf( color ) === -1 ) { + result.push( color ); + } + } ); + } return result; }; @@ -1311,13 +1287,6 @@ enable = enable === undefined ? !this.highlightEnabled : enable; if ( this.highlightEnabled !== enable ) { - // HACK make sure highlights are disabled globally while we toggle on the items, - // otherwise we'll call clearHighlight() and applyHighlight() many many times - this.highlightEnabled = false; - this.getItems().forEach( function ( filterItem ) { - filterItem.toggleHighlight( enable ); - } ); - this.highlightEnabled = enable; this.emit( 'highlightChange', this.highlightEnabled ); } diff --git a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.ItemModel.js b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.ItemModel.js index 4a8869a17d..d940321342 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.ItemModel.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.ItemModel.js @@ -181,6 +181,10 @@ * @param {string|null} highlightColor */ mw.rcfilters.dm.ItemModel.prototype.setHighlightColor = function ( highlightColor ) { + if ( !this.isHighlightSupported() ) { + return; + } + if ( this.highlightColor !== highlightColor ) { this.highlightColor = highlightColor; this.emit( 'update' ); @@ -222,36 +226,6 @@ return this.identifiers; }; - /** - * Toggle the highlight feature on and off for this filter. - * It only works if highlight is supported for this filter. - * - * @param {boolean} enable Highlight should be enabled - */ - mw.rcfilters.dm.ItemModel.prototype.toggleHighlight = function ( enable ) { - enable = enable === undefined ? !this.highlightEnabled : enable; - - if ( !this.isHighlightSupported() ) { - return; - } - - if ( enable === this.highlightEnabled ) { - return; - } - - this.highlightEnabled = enable; - this.emit( 'update' ); - }; - - /** - * Check if the highlight feature is currently enabled for this filter - * - * @return {boolean} - */ - mw.rcfilters.dm.ItemModel.prototype.isHighlightEnabled = function () { - return !!this.highlightEnabled; - }; - /** * Check if the highlight feature is supported for this filter * @@ -267,6 +241,6 @@ * @return {boolean} */ mw.rcfilters.dm.ItemModel.prototype.isHighlighted = function () { - return this.isHighlightEnabled() && !!this.getHighlightColor(); + return !!this.getHighlightColor(); }; }( mediaWiki ) ); diff --git a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.SavedQueriesModel.js b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.SavedQueriesModel.js index 29585e902d..23f600739f 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.SavedQueriesModel.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.SavedQueriesModel.js @@ -123,6 +123,8 @@ // the given data, if they exist normalizedData.params = model.filtersModel.removeExcludedParams( normalizedData.params ); + model.cleanupHighlights( normalizedData ); + id = String( id ); // Skip the addNewQuery method because we don't want to unnecessarily manipulate @@ -149,6 +151,23 @@ this.emit( 'initialize' ); }; + /** + * Clean up highlight parameters. + * 'highlight' used to be stored, it's not inferred based on the presence of absence of + * filter colors. + * + * @param {Object} data Saved query data + */ + mw.rcfilters.dm.SavedQueriesModel.prototype.cleanupHighlights = function ( data ) { + if ( + data.params.highlight === '0' && + data.highlights && Object.keys( data.highlights ).length + ) { + data.highlights = {}; + } + delete data.params.highlight; + }; + /** * Convert from representation of filters to representation of parameters * @@ -168,7 +187,7 @@ this.filtersModel.getParametersFromFilters( fullFilterRepresentation ) ); - // Highlights (taking out 'highlight' itself, appending _color to keys) + // Highlights: appending _color to keys newData.highlights = {}; $.each( data.highlights, function ( highlightedFilterName, value ) { if ( value ) { diff --git a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.SavedQueryItemModel.js b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.SavedQueryItemModel.js index a6ff9a1c25..c74648e8eb 100644 --- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.SavedQueryItemModel.js +++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.SavedQueryItemModel.js @@ -10,7 +10,7 @@ * @param {string} label Saved query label * @param {Object} data Saved query data * @param {Object} [config] Configuration options - * @param {boolean} [default] This item is the default + * @cfg {boolean} [default] This item is the default */ mw.rcfilters.dm.SavedQueryItemModel = function MwRcfiltersDmSavedQueriesModel( id, label, data, config ) { config = config || {}; diff --git a/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js b/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js index 0b2dd8d381..ac998d7680 100644 --- a/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js +++ b/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js @@ -690,7 +690,7 @@ * Check whether the current filter and highlight state exists * in the saved queries model. * - * @return {boolean} Query exists + * @return {mw.rcfilters.dm.SavedQueryItemModel} Matching item model */ mw.rcfilters.Controller.prototype.findQueryMatchingCurrentState = function () { return this.savedQueriesModel.findMatchingQuery( diff --git a/resources/src/mediawiki.rcfilters/mw.rcfilters.UriProcessor.js b/resources/src/mediawiki.rcfilters/mw.rcfilters.UriProcessor.js index 044712c6bf..53557f641f 100644 --- a/resources/src/mediawiki.rcfilters/mw.rcfilters.UriProcessor.js +++ b/resources/src/mediawiki.rcfilters/mw.rcfilters.UriProcessor.js @@ -208,13 +208,7 @@ */ mw.rcfilters.UriProcessor.prototype.doesQueryContainRecognizedParams = function ( uriQuery ) { var anyValidInUrl, - validParameterNames = Object.keys( this.filtersModel.getEmptyParameterState() ) - .filter( function ( param ) { - // Remove 'highlight' parameter from this check; - // if it's the only parameter in the URL we still - // want to consider the URL 'empty' for defaults to load - return param !== 'highlight'; - } ); + validParameterNames = Object.keys( this.filtersModel.getEmptyParameterState() ); uriQuery = uriQuery || new mw.Uri().query; diff --git a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterMenuOptionWidget.js b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterMenuOptionWidget.js index 1292901bf0..926502d2fc 100644 --- a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterMenuOptionWidget.js +++ b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterMenuOptionWidget.js @@ -6,19 +6,22 @@ * * @constructor * @param {mw.rcfilters.Controller} controller RCFilters controller + * @param {mw.rcfilters.dm.FiltersViewModel} filtersViewModel * @param {mw.rcfilters.dm.FilterItem} invertModel - * @param {mw.rcfilters.dm.FilterItem} model Filter item model + * @param {mw.rcfilters.dm.FilterItem} itemModel Filter item model * @param {Object} config Configuration object */ - mw.rcfilters.ui.FilterMenuOptionWidget = function MwRcfiltersUiFilterMenuOptionWidget( controller, invertModel, model, config ) { + mw.rcfilters.ui.FilterMenuOptionWidget = function MwRcfiltersUiFilterMenuOptionWidget( + controller, filtersViewModel, invertModel, itemModel, config + ) { config = config || {}; this.controller = controller; this.invertModel = invertModel; - this.model = model; + this.model = itemModel; // Parent - mw.rcfilters.ui.FilterMenuOptionWidget.parent.call( this, controller, this.invertModel, model, config ); + mw.rcfilters.ui.FilterMenuOptionWidget.parent.call( this, controller, filtersViewModel, this.invertModel, itemModel, config ); // Event this.model.getGroupModel().connect( this, { update: 'onGroupModelUpdate' } ); @@ -40,9 +43,9 @@ /** * @inheritdoc */ - mw.rcfilters.ui.FilterMenuOptionWidget.prototype.onModelUpdate = function () { + mw.rcfilters.ui.FilterMenuOptionWidget.prototype.updateUiBasedOnState = function () { // Parent - mw.rcfilters.ui.FilterMenuOptionWidget.parent.prototype.onModelUpdate.call( this ); + mw.rcfilters.ui.FilterMenuOptionWidget.parent.prototype.updateUiBasedOnState.call( this ); this.setCurrentMuteState(); }; diff --git a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterTagItemWidget.js b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterTagItemWidget.js index 43a301f0f9..41c7bae48c 100644 --- a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterTagItemWidget.js +++ b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterTagItemWidget.js @@ -7,14 +7,17 @@ * * @constructor * @param {mw.rcfilters.Controller} controller + * @param {mw.rcfilters.dm.FiltersViewModel} filtersViewModel * @param {mw.rcfilters.dm.FilterItem} invertModel - * @param {mw.rcfilters.dm.FilterItem} model Item model + * @param {mw.rcfilters.dm.FilterItem} itemModel Item model * @param {Object} config Configuration object */ - mw.rcfilters.ui.FilterTagItemWidget = function MwRcfiltersUiFilterTagItemWidget( controller, invertModel, model, config ) { + mw.rcfilters.ui.FilterTagItemWidget = function MwRcfiltersUiFilterTagItemWidget( + controller, filtersViewModel, invertModel, itemModel, config + ) { config = config || {}; - mw.rcfilters.ui.FilterTagItemWidget.parent.call( this, controller, invertModel, model, config ); + mw.rcfilters.ui.FilterTagItemWidget.parent.call( this, controller, filtersViewModel, invertModel, itemModel, config ); this.$element .addClass( 'mw-rcfilters-ui-filterTagItemWidget' ); @@ -32,11 +35,11 @@ mw.rcfilters.ui.FilterTagItemWidget.prototype.setCurrentMuteState = function () { this.setFlags( { muted: ( - !this.model.isSelected() || - this.model.isIncluded() || - this.model.isFullyCovered() + !this.itemModel.isSelected() || + this.itemModel.isIncluded() || + this.itemModel.isFullyCovered() ), - invalid: this.model.isSelected() && this.model.isConflicted() + invalid: this.itemModel.isSelected() && this.itemModel.isConflicted() } ); }; }( mediaWiki, jQuery ) ); diff --git a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterTagMultiselectWidget.js b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterTagMultiselectWidget.js index ef95f2f243..404cb981a4 100644 --- a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterTagMultiselectWidget.js +++ b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.FilterTagMultiselectWidget.js @@ -416,6 +416,7 @@ /** * Respond to model itemUpdate event + * fixme: when a new state is applied to the model this function is called 60+ times in a row * * @param {mw.rcfilters.dm.FilterItem} item Filter item model */ @@ -425,7 +426,6 @@ item.isSelected() || ( this.model.isHighlightEnabled() && - item.isHighlightSupported() && item.getHighlightColor() ) ) { @@ -484,6 +484,8 @@ } }.bind( this ) ); } + + this.setSavedQueryVisibility(); }; /** @@ -615,6 +617,7 @@ if ( filterItem ) { return new mw.rcfilters.ui.FilterTagItemWidget( this.controller, + this.model, this.model.getInvertModel(), filterItem, { diff --git a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.ItemMenuOptionWidget.js b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.ItemMenuOptionWidget.js index 36bc6cb9fe..db43a5376c 100644 --- a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.ItemMenuOptionWidget.js +++ b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.ItemMenuOptionWidget.js @@ -6,11 +6,14 @@ * * @constructor * @param {mw.rcfilters.Controller} controller RCFilters controller + * @param {mw.rcfilters.dm.FiltersViewModel} filtersViewModel * @param {mw.rcfilters.dm.ItemModel} invertModel - * @param {mw.rcfilters.dm.ItemModel} model Item model + * @param {mw.rcfilters.dm.ItemModel} itemModel Item model * @param {Object} config Configuration object */ - mw.rcfilters.ui.ItemMenuOptionWidget = function MwRcfiltersUiItemMenuOptionWidget( controller, invertModel, model, config ) { + mw.rcfilters.ui.ItemMenuOptionWidget = function MwRcfiltersUiItemMenuOptionWidget( + controller, filtersViewModel, invertModel, itemModel, config + ) { var layout, classes = [], $label = $( '
' ) @@ -19,20 +22,21 @@ config = config || {}; this.controller = controller; + this.filtersViewModel = filtersViewModel; this.invertModel = invertModel; - this.model = model; + this.itemModel = itemModel; // Parent mw.rcfilters.ui.ItemMenuOptionWidget.parent.call( this, $.extend( { // Override the 'check' icon that OOUI defines icon: '', - data: this.model.getName(), - label: this.model.getLabel() + data: this.itemModel.getName(), + label: this.itemModel.getLabel() }, config ) ); this.checkboxWidget = new mw.rcfilters.ui.CheckboxInputWidget( { - value: this.model.getName(), - selected: this.model.isSelected() + value: this.itemModel.getName(), + selected: this.itemModel.isSelected() } ); $label.append( @@ -40,28 +44,28 @@ .addClass( 'mw-rcfilters-ui-itemMenuOptionWidget-label-title' ) .append( this.$label ) ); - if ( this.model.getDescription() ) { + if ( this.itemModel.getDescription() ) { $label.append( $( '
' ) .addClass( 'mw-rcfilters-ui-itemMenuOptionWidget-label-desc' ) - .text( this.model.getDescription() ) + .text( this.itemModel.getDescription() ) ); } this.highlightButton = new mw.rcfilters.ui.FilterItemHighlightButton( this.controller, - this.model, + this.itemModel, { $overlay: config.$overlay || this.$element, title: mw.msg( 'rcfilters-highlightmenu-help' ) } ); - this.highlightButton.toggle( this.model.isHighlightEnabled() ); + this.highlightButton.toggle( this.filtersViewModel.isHighlightEnabled() ); this.excludeLabel = new OO.ui.LabelWidget( { label: mw.msg( 'rcfilters-filter-excluded' ) } ); - this.excludeLabel.toggle( this.model.isSelected() && this.invertModel.isSelected() ); + this.excludeLabel.toggle( this.itemModel.isSelected() && this.invertModel.isSelected() ); layout = new OO.ui.FieldLayout( this.checkboxWidget, { label: $label, @@ -69,8 +73,9 @@ } ); // Events - this.invertModel.connect( this, { update: 'onModelUpdate' } ); - this.model.connect( this, { update: 'onModelUpdate' } ); + this.filtersViewModel.connect( this, { highlightChange: 'updateUiBasedOnState' } ); + this.invertModel.connect( this, { update: 'updateUiBasedOnState' } ); + this.itemModel.connect( this, { update: 'updateUiBasedOnState' } ); // HACK: Prevent defaults on 'click' for the label so it // doesn't steal the focus away from the input. This means // we can continue arrow-movement after we click the label @@ -80,7 +85,7 @@ this.$element .addClass( 'mw-rcfilters-ui-itemMenuOptionWidget' ) - .addClass( 'mw-rcfilters-ui-itemMenuOptionWidget-view-' + this.model.getGroupModel().getView() ) + .addClass( 'mw-rcfilters-ui-itemMenuOptionWidget-view-' + this.itemModel.getGroupModel().getView() ) .append( $( '
' ) .addClass( 'mw-rcfilters-ui-table' ) @@ -101,8 +106,8 @@ ) ); - if ( this.model.getIdentifiers() ) { - this.model.getIdentifiers().forEach( function ( ident ) { + if ( this.itemModel.getIdentifiers() ) { + this.itemModel.getIdentifiers().forEach( function ( ident ) { classes.push( 'mw-rcfilters-ui-itemMenuOptionWidget-identifier-' + ident ); } ); @@ -124,11 +129,11 @@ /** * Respond to item model update event */ - mw.rcfilters.ui.ItemMenuOptionWidget.prototype.onModelUpdate = function () { - this.checkboxWidget.setSelected( this.model.isSelected() ); + mw.rcfilters.ui.ItemMenuOptionWidget.prototype.updateUiBasedOnState = function () { + this.checkboxWidget.setSelected( this.itemModel.isSelected() ); - this.highlightButton.toggle( this.model.isHighlightEnabled() ); - this.excludeLabel.toggle( this.model.isSelected() && this.invertModel.isSelected() ); + this.highlightButton.toggle( this.filtersViewModel.isHighlightEnabled() ); + this.excludeLabel.toggle( this.itemModel.isSelected() && this.invertModel.isSelected() ); }; /** @@ -137,11 +142,11 @@ * @return {string} Filter name */ mw.rcfilters.ui.ItemMenuOptionWidget.prototype.getName = function () { - return this.model.getName(); + return this.itemModel.getName(); }; mw.rcfilters.ui.ItemMenuOptionWidget.prototype.getModel = function () { - return this.model; + return this.itemModel; }; }( mediaWiki ) ); 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 63a563c011..22c176f282 100644 --- a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.MenuSelectWidget.js +++ b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.MenuSelectWidget.js @@ -186,6 +186,7 @@ currentItems.push( new mw.rcfilters.ui.FilterMenuOptionWidget( widget.controller, + widget.model, widget.model.getInvertModel(), filterItem, { diff --git a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.TagItemWidget.js b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.TagItemWidget.js index cc314ac66a..7e324b6946 100644 --- a/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.TagItemWidget.js +++ b/resources/src/mediawiki.rcfilters/ui/mw.rcfilters.ui.TagItemWidget.js @@ -8,22 +8,26 @@ * * @constructor * @param {mw.rcfilters.Controller} controller + * @param {mw.rcfilters.dm.FiltersViewModel} filtersViewModel * @param {mw.rcfilters.dm.FilterItem} invertModel - * @param {mw.rcfilters.dm.FilterItem} model Item model + * @param {mw.rcfilters.dm.FilterItem} itemModel Item model * @param {Object} config Configuration object * @cfg {jQuery} [$overlay] A jQuery object serving as overlay for popups */ - mw.rcfilters.ui.TagItemWidget = function MwRcfiltersUiTagItemWidget( controller, invertModel, model, config ) { + mw.rcfilters.ui.TagItemWidget = function MwRcfiltersUiTagItemWidget( + controller, filtersViewModel, invertModel, itemModel, config + ) { // Configuration initialization config = config || {}; this.controller = controller; this.invertModel = invertModel; - this.model = model; + this.filtersViewModel = filtersViewModel; + this.itemModel = itemModel; this.selected = false; mw.rcfilters.ui.TagItemWidget.parent.call( this, $.extend( { - data: this.model.getName() + data: this.itemModel.getName() }, config ) ); this.$overlay = config.$overlay || this.$element; @@ -50,11 +54,12 @@ .addClass( 'mw-rcfilters-ui-tagItemWidget-highlight' ); // Add title attribute with the item label to 'x' button - this.closeButton.setTitle( mw.msg( 'rcfilters-tag-remove', this.model.getLabel() ) ); + this.closeButton.setTitle( mw.msg( 'rcfilters-tag-remove', this.itemModel.getLabel() ) ); // Events + this.filtersViewModel.connect( this, { highlightChange: 'updateUiBasedOnState' } ); this.invertModel.connect( this, { update: 'updateUiBasedOnState' } ); - this.model.connect( this, { update: 'updateUiBasedOnState' } ); + this.itemModel.connect( this, { update: 'updateUiBasedOnState' } ); // Initialization this.$overlay.append( this.popup.$element ); @@ -82,13 +87,18 @@ this.setCurrentMuteState(); // Update label if needed - this.setLabel( $( '
' ).html( this.model.getPrefixedLabel( this.invertModel.isSelected() ) ).contents() ); + this.setLabel( $( '
' ).html( this.itemModel.getPrefixedLabel( this.invertModel.isSelected() ) ).contents() ); this.setHighlightColor(); }; + /** + * Set the current highlight color for this item + */ mw.rcfilters.ui.TagItemWidget.prototype.setHighlightColor = function () { - var selectedColor = this.model.isHighlightEnabled() ? this.model.getHighlightColor() : null; + var selectedColor = this.filtersViewModel.isHighlightEnabled() && this.itemModel.isHighlighted ? + this.itemModel.getHighlightColor() : + null; this.$highlight .attr( 'data-color', selectedColor ) @@ -107,7 +117,7 @@ * Respond to mouse enter event */ mw.rcfilters.ui.TagItemWidget.prototype.onMouseEnter = function () { - var labelText = this.model.getStateMessage(); + var labelText = this.itemModel.getStateMessage(); if ( labelText ) { this.popupLabel.setLabel( labelText ); @@ -166,7 +176,7 @@ * @return {string} Filter name */ mw.rcfilters.ui.TagItemWidget.prototype.getName = function () { - return this.model.getName(); + return this.itemModel.getName(); }; /** @@ -175,7 +185,7 @@ * @return {string} Filter model */ mw.rcfilters.ui.TagItemWidget.prototype.getModel = function () { - return this.model; + return this.itemModel; }; /** @@ -184,7 +194,7 @@ * @return {string} Filter view */ mw.rcfilters.ui.TagItemWidget.prototype.getView = function () { - return this.model.getGroupModel().getView(); + return this.itemModel.getGroupModel().getView(); }; /** @@ -195,7 +205,7 @@ this.popup.$element.detach(); // Disconnect events - this.model.disconnect( this ); + this.itemModel.disconnect( this ); this.closeButton.disconnect( this ); }; }( mediaWiki, jQuery ) ); diff --git a/tests/qunit/suites/resources/mediawiki.rcfilters/UriProcessor.test.js b/tests/qunit/suites/resources/mediawiki.rcfilters/UriProcessor.test.js index 291d5c747c..534af86d64 100644 --- a/tests/qunit/suites/resources/mediawiki.rcfilters/UriProcessor.test.js +++ b/tests/qunit/suites/resources/mediawiki.rcfilters/UriProcessor.test.js @@ -114,16 +114,6 @@ $.extend( true, {}, { filter1: '1' } ), 'Parameters in Uri query set parameter value in the model' ); - - uriProcessor.updateModelBasedOnQuery( { highlight: '1', group1__filter1_color: 'c1', urlversion: '2' } ); - assert.deepEqual( - filtersModel.getCurrentParameterState(), - { - highlight: '1', - group1__filter1_color: 'c1' - }, - 'Highlight parameters in Uri query set highlight state in the model' - ); } ); QUnit.test( 'isNewState', function ( assert ) { 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 dde49ba00b..a700e30aa9 100644 --- a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js +++ b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js @@ -161,7 +161,6 @@ group6option3: '0', group7: '', namespace: '', - highlight: '0', // Null highlights group1__filter1_color: null, group1__filter2_color: null, diff --git a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.SavedQueriesModel.test.js b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.SavedQueriesModel.test.js index 539bab4f19..58524ecf91 100644 --- a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.SavedQueriesModel.test.js +++ b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.SavedQueriesModel.test.js @@ -83,10 +83,8 @@ // in param representation filter2: '1', filter3: '1', // Group type string_options - group2: 'filter4', + group2: 'filter4' // Note - Group3 is sticky, so it won't show in output - // highlight toggle - highlight: '1' }, highlights: { group1__filter1_color: 'c5', @@ -95,6 +93,11 @@ } } } + }, + removeHighlights = function ( data ) { + var copy = $.extend( true, {}, data ); + copy.queries[ 1234 ].data.highlights = {}; + return copy; }; QUnit.module( 'mediawiki.rcfilters - SavedQueriesModel' ); @@ -158,6 +161,12 @@ finalState: $.extend( true, { default: '1234' }, queriesParamRepresentation ), msg: 'Conversion from filter representation to parameters, with default set up, retains data.' }, + { + // Converting from old structure and cleaning up highlights + input: $.extend( true, queriesFilterRepresentation, { queries: { 1234: { data: { highlights: { highlight: false } } } } } ), + finalState: removeHighlights( queriesParamRepresentation ), + msg: 'Conversion from filter representation to parameters and highlight cleanup' + }, { // New structure input: $.extend( true, {}, queriesParamRepresentation ), @@ -170,6 +179,18 @@ input: $.extend( true, { queries: { 1234: { data: { highlights: { group2__filter5_color: 'c2' } } } } }, exampleQueryStructure ), finalState: $.extend( true, { queries: { 1234: { data: { highlights: { group2__filter5_color: 'c2' } } } } }, exampleQueryStructure ), msg: 'Structure that contains invalid highlights remains the same in initialization' + }, + { + // Trim colors when highlight=false is stored + input: $.extend( true, { queries: { 1234: { data: { params: { highlight: '0' } } } } }, queriesParamRepresentation ), + finalState: removeHighlights( queriesParamRepresentation ), + msg: 'Colors are removed when highlight=false' + }, + { + // Remove highlight when it is true but no colors are specified + input: $.extend( true, { queries: { 1234: { data: { params: { highlight: '1' } } } } }, removeHighlights( queriesParamRepresentation ) ), + finalState: removeHighlights( queriesParamRepresentation ), + msg: 'remove highlight when it is true but there is no colors' } ]; @@ -298,7 +319,6 @@ 'New query 1', { group2: 'filter5', - highlight: '1', group1__filter1_color: 'c5', group3__group3option1_color: 'c1' } @@ -327,8 +347,7 @@ label: 'New query 1', data: { params: { - group2: 'filter5', - highlight: '1' + group2: 'filter5' }, highlights: { group1__filter1_color: 'c5', @@ -385,7 +404,6 @@ // Find matching query matchingItem = queriesModel.findMatchingQuery( { - highlight: '1', group2: 'filter5', group1__filter1_color: 'c5', group3__group3option1_color: 'c1' @@ -403,7 +421,6 @@ group2: 'filter5', filter1: '0', filter2: '0', - highlight: '1', invert: '0', group1__filter1_color: 'c5', group3__group3option1_color: 'c1' diff --git a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.SavedQueryItemModel.test.js b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.SavedQueryItemModel.test.js index a91dff99a1..181e9925dc 100644 --- a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.SavedQueryItemModel.test.js +++ b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.SavedQueryItemModel.test.js @@ -4,7 +4,6 @@ params: { param1: '1', param2: 'foo|bar', - highlight: '1', invert: '0' }, highlights: {