Mooeypoo has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/372428 )

Change subject: RCFilters: Fix validation for single_option groups
......................................................................

RCFilters: Fix validation for single_option groups

The single_option groups have a base definition that means they
always must have a value, and must never have more than one value.
The previous way we attempted to do that was confusing and missed
a case where after resetting filters, two values were selected before
one could be unselected, which then broke the behavior in the
entire group.

This fix reorganizes the validation when an item in the group is
selected or unselected to make sure the group retains its promised
behavior.

Bug: T173303
Change-Id: I5758ec324a26c0e5e6f5c473d206e818a1d22523
---
M resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
1 file changed, 27 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/28/372428/1

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 6acc44d..23ceffe 100644
--- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
+++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
@@ -224,32 +224,33 @@
                var changed = false,
                        active = this.areAnySelected();
 
-               if (
-                       item.isSelected() &&
-                       this.getType() === 'single_option' &&
-                       this.currSelected &&
-                       this.currSelected !== item
-               ) {
-                       this.currSelected.toggleSelected( false );
-               }
-
-               // For 'single_option' groups, check if we just unselected all
-               // items. This should never be the result. If we did unselect
-               // all (like resetting all filters to false) then this group
-               // must choose its default item or the first item in the group
-               if (
-                       this.getType() === 'single_option' &&
-                       !this.getItems().some( function ( filterItem ) {
-                               return filterItem.isSelected();
-                       } )
-               ) {
-                       // Single option means there must be a single option
-                       // selected, so we have to either select the default
-                       // or select the first option
-                       this.currSelected = this.getItemByParamName( 
this.defaultParams[ this.getName() ] ) ||
-                               this.getItems()[ 0 ];
-                       this.currSelected.toggleSelected( true );
-                       changed = true;
+               if ( this.getType() === 'single_option' ) {
+                       // This group must have one item selected always
+                       // and must never have more than one item selected at a 
time
+                       if ( this.getSelectedItems().length === 0 ) {
+                               // Nothing is selected anymore
+                               // Select the default or the first item
+                               this.currSelected = this.getItemByParamName( 
this.defaultParams[ this.getName() ] ) ||
+                                       this.getItems()[ 0 ];
+                               this.currSelected.toggleSelected( true );
+                               changed = true;
+                       } else if ( this.getSelectedItems().length > 1 ) {
+                               // There is more than one item selected
+                               // This should only happen if the item given
+                               // is the one that is selected, so unselect
+                               // all items that is not it
+                               this.getSelectedItems().forEach( function ( 
itemModel ) {
+                                       // Note that in case the given item is 
actually
+                                       // not selected, this loop will end up 
unselecting
+                                       // all items, which would trigger the 
case above
+                                       // when the last item is unselected 
anyways
+                                       itemModel.toggleSelected(
+                                               itemModel.getName() === 
item.getName() &&
+                                               item.isSelected()
+                                       );
+                               } );
+                               changed = true;
+                       }
                }
 
                if (

-- 
To view, visit https://gerrit.wikimedia.org/r/372428
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5758ec324a26c0e5e6f5c473d206e818a1d22523
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Mooeypoo <mor...@gmail.com>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to