Trevor Parscal has uploaded a new change for review.
https://gerrit.wikimedia.org/r/174886
Change subject: [BREAKING CHANGE] Allow options with similar data
......................................................................
[BREAKING CHANGE] Allow options with similar data
Problems:
1. OptionWidget's were required to have unique data within a select, which can
cause problems and confusion, especially since conflicts were handled silently
in unintuitive ways.
2. Item data was hashed when the item was added, essentially freezing the value
at the time of adding. If the item's data changes, the hash is invalid and
stops working.
Thoughts:
1. It's actually not nessecary for options to have unique data, and it may be
generally useful for other kinds of elements to have data, and other kinds of
groups to be able to access elements by that data.
2. While thus far we've not had an issue with items having unique data, it
seems that we are destined to need this eventually, especially with drag and
drop between groups on the horizon.
Solution:
1. Add 'data' config option to Element, stored as 'data' property, accessed
using 'getData' method and modified using 'setData' method
Store data property in Element
2. Remove data argument, data property and 'getData' method from OptionWidget
3. Move getItemFromData from SelectWidget to GroupElement - make it return
first matching item
4. Add getItemsFromData method to GroupElement, which returns all matching items
5. Remove logic in SelectWidget that indexed item data and enforced unique item
data constraints
6. Update subclasses of OptionWidget to no longer pass through the data argument
7. Update callers to use the 'data' config option instead of an initial argument
Change-Id: I7ee78b6d0b734c32fc6dd3760c6ec45362ef8571
---
M demos/demo.js
M demos/pages/dialogs.js
M demos/pages/widgets.js
M src/Element.js
M src/elements/GroupElement.js
M src/layouts/BookletLayout.js
M src/widgets/ButtonOptionWidget.js
M src/widgets/DecoratedOptionWidget.js
M src/widgets/MenuOptionWidget.js
M src/widgets/MenuSectionOptionWidget.js
M src/widgets/OptionWidget.js
M src/widgets/OutlineOptionWidget.js
M src/widgets/SelectWidget.js
13 files changed, 160 insertions(+), 122 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/oojs/ui refs/changes/86/174886/1
diff --git a/demos/demo.js b/demos/demo.js
index ce36e15..7349673 100644
--- a/demos/demo.js
+++ b/demos/demo.js
@@ -19,26 +19,26 @@
$: this.$,
menu: {
items: [
- new OO.ui.MenuOptionWidget( 'dialogs', { $:
this.$, label: 'Dialogs' } ),
- new OO.ui.MenuOptionWidget( 'icons', { $:
this.$, label: 'Icons' } ),
- new OO.ui.MenuOptionWidget( 'toolbars', { $:
this.$, label: 'Toolbars' } ),
- new OO.ui.MenuOptionWidget( 'widgets', { $:
this.$, label: 'Widgets' } )
+ new OO.ui.MenuOptionWidget( { $: this.$, data:
'dialogs', label: 'Dialogs' } ),
+ new OO.ui.MenuOptionWidget( { $: this.$, data:
'icons', label: 'Icons' } ),
+ new OO.ui.MenuOptionWidget( { $: this.$, data:
'toolbars', label: 'Toolbars' } ),
+ new OO.ui.MenuOptionWidget( { $: this.$, data:
'widgets', label: 'Widgets' } )
]
},
classes: [ 'oo-ui-demo-pageDropdown' ]
} );
this.pageMenu = this.pageDropdown.getMenu();
this.themeSelect = new OO.ui.ButtonSelectWidget( { $: this.$ }
).addItems( [
- new OO.ui.ButtonOptionWidget( 'apex', { $: this.$, label:
'Apex' } ),
- new OO.ui.ButtonOptionWidget( 'mediawiki', { $: this.$, label:
'MediaWiki' } )
+ new OO.ui.ButtonOptionWidget( { $: this.$, data: 'apex', label:
'Apex' } ),
+ new OO.ui.ButtonOptionWidget( { $: this.$, data: 'mediawiki',
label: 'MediaWiki' } )
] );
this.graphicsSelect = new OO.ui.ButtonSelectWidget( { $: this.$ }
).addItems( [
- new OO.ui.ButtonOptionWidget( 'vector', { $: this.$, label:
'Vector' } ),
- new OO.ui.ButtonOptionWidget( 'raster', { $: this.$, label:
'Raster' } )
+ new OO.ui.ButtonOptionWidget( { $: this.$, data: 'vector',
label: 'Vector' } ),
+ new OO.ui.ButtonOptionWidget( { $: this.$, data: 'raster',
label: 'Raster' } )
] );
this.directionSelect = new OO.ui.ButtonSelectWidget( { $: this.$ }
).addItems( [
- new OO.ui.ButtonOptionWidget( 'ltr', { $: this.$, label: 'LTR'
} ),
- new OO.ui.ButtonOptionWidget( 'rtl', { $: this.$, label: 'RTL'
} )
+ new OO.ui.ButtonOptionWidget( { $: this.$, data: 'ltr', label:
'LTR' } ),
+ new OO.ui.ButtonOptionWidget( { $: this.$, data: 'rtl', label:
'RTL' } )
] );
// Events
diff --git a/demos/pages/dialogs.js b/demos/pages/dialogs.js
index ceeb1e7..689bf6d 100644
--- a/demos/pages/dialogs.js
+++ b/demos/pages/dialogs.js
@@ -74,7 +74,7 @@
SearchWidgetDialog.super.prototype.initialize.apply( this,
arguments );
var i, items = [], searchWidget = new OO.ui.SearchWidget( { $:
this.$ } );
for ( i = 1; i <= 20; i++ ) {
- items.push( new OO.ui.OptionWidget( i, { $: this.$,
label: 'Item ' + i } ) );
+ items.push( new OO.ui.OptionWidget( { $: this.$, data:
i, label: 'Item ' + i } ) );
}
searchWidget.results.addItems( items );
searchWidget.onQueryChange = function () {};
diff --git a/demos/pages/widgets.js b/demos/pages/widgets.js
index 402456c..bf7682d 100644
--- a/demos/pages/widgets.js
+++ b/demos/pages/widgets.js
@@ -297,18 +297,23 @@
new OO.ui.FieldLayout(
new OO.ui.ButtonSelectWidget( {
items: [
- new OO.ui.ButtonOptionWidget(
'a', {
- icon: 'picture',
indicator: 'down'
+ new OO.ui.ButtonOptionWidget( {
+ data: 'a',
+ icon: 'picture',
+ indicator: 'down'
} ),
- new OO.ui.ButtonOptionWidget(
'b', {
+ new OO.ui.ButtonOptionWidget( {
+ data: 'b',
label: 'One',
flags: [ 'primary' ]
} ),
- new OO.ui.ButtonOptionWidget(
'c', {
+ new OO.ui.ButtonOptionWidget( {
+ data: 'c',
label: 'Two',
flags: [ 'constructive'
]
} ),
- new OO.ui.ButtonOptionWidget(
'd', {
+ new OO.ui.ButtonOptionWidget( {
+ data: 'd',
label: 'Three',
flags: [ 'destructive' ]
} )
@@ -323,16 +328,20 @@
new OO.ui.ButtonSelectWidget( {
disabled: true,
items: [
- new OO.ui.ButtonOptionWidget(
'a', {
+ new OO.ui.ButtonOptionWidget( {
+ data: 'a',
icon: 'picture',
indicator: 'down'
} ),
- new OO.ui.ButtonOptionWidget(
1, {
+ new OO.ui.ButtonOptionWidget( {
+ data: 1,
label: 'One'
} ),
- new OO.ui.ButtonOptionWidget(
2, {
+ new OO.ui.ButtonOptionWidget( {
+ data: 2,
label: 'Two'
} ),
- new OO.ui.ButtonOptionWidget(
3, {
+ new OO.ui.ButtonOptionWidget( {
+ data: 3,
label: 'Three'
} )
]
@@ -345,17 +354,21 @@
new OO.ui.FieldLayout(
new OO.ui.ButtonSelectWidget( {
items: [
- new OO.ui.ButtonOptionWidget(
'a', {
+ new OO.ui.ButtonOptionWidget( {
+ data: 'a',
icon: 'picture',
indicator: 'down'
} ),
- new OO.ui.ButtonOptionWidget(
1, {
+ new OO.ui.ButtonOptionWidget( {
+ data: 1,
label: 'One',
disabled: true
} ),
- new OO.ui.ButtonOptionWidget(
2, {
+ new OO.ui.ButtonOptionWidget( {
+ data: 2,
label: 'Two'
} ),
- new OO.ui.ButtonOptionWidget(
3, {
+ new OO.ui.ButtonOptionWidget( {
+ data: 3,
label: 'Three',
disabled: true
} )
@@ -465,19 +478,24 @@
align: 'top',
menu: {
items: [
- new
OO.ui.MenuOptionWidget( 'a', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'a',
label: 'First'
} ),
- new
OO.ui.MenuOptionWidget( 'b', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'b',
label: 'Second'
} ),
- new
OO.ui.MenuOptionWidget( 'c', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'c',
label: 'Third'
} ),
- new
OO.ui.MenuOptionWidget( 'c', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'c',
label: 'The
fourth option has a long label'
} ),
- new
OO.ui.MenuOptionWidget( 'd', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'd',
label: 'Fifth'
} )
]
@@ -493,21 +511,26 @@
label: 'Select one',
menu: {
items: [
- new
OO.ui.MenuOptionWidget( 'a', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'a',
label: 'First'
} ),
- new
OO.ui.MenuOptionWidget( 'b', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'b',
label:
'Disabled second option',
disabled: true
} ),
- new
OO.ui.MenuOptionWidget( 'c', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'c',
label: 'Third'
} ),
- new
OO.ui.MenuOptionWidget( 'd', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'd',
label:
'Disabled fourth option with long label',
disabled: true
} ),
- new
OO.ui.MenuOptionWidget( 'c', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'c',
label: 'Third'
} )
]
@@ -524,16 +547,20 @@
disabled: true,
menu: {
items: [
- new
OO.ui.MenuOptionWidget( 'a', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'a',
label: 'First'
} ),
- new
OO.ui.MenuOptionWidget( 'b', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'b',
label: 'Second'
} ),
- new
OO.ui.MenuOptionWidget( 'c', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'c',
label: 'Third'
} ),
- new
OO.ui.MenuOptionWidget( 'd', {
+ new
OO.ui.MenuOptionWidget( {
+ data: 'd',
label: 'Fourth'
} )
]
@@ -548,11 +575,11 @@
new OO.ui.ComboBoxWidget( {
menu: {
items: [
- new
OO.ui.MenuOptionWidget( 'asd', { label: 'Label for asd' } ),
- new
OO.ui.MenuOptionWidget( 'fgh', { label: 'Label for fgh' } ),
- new
OO.ui.MenuOptionWidget( 'jkl', { label: 'Label for jkl' } ),
- new
OO.ui.MenuOptionWidget( 'zxc', { label: 'Label for zxc' } ),
- new
OO.ui.MenuOptionWidget( 'vbn', { label: 'Label for vbn' } )
+ new
OO.ui.MenuOptionWidget( { data: 'asd', label: 'Label for asd' } ),
+ new
OO.ui.MenuOptionWidget( { data: 'fgh', label: 'Label for fgh' } ),
+ new
OO.ui.MenuOptionWidget( { data: 'jkl', label: 'Label for jkl' } ),
+ new
OO.ui.MenuOptionWidget( { data: 'zxc', label: 'Label for zxc' } ),
+ new
OO.ui.MenuOptionWidget( { data: 'vbn', label: 'Label for vbn' } )
]
}
} ),
@@ -566,11 +593,11 @@
disabled: true,
menu: {
items: [
- new
OO.ui.MenuOptionWidget( 'asd', { label: 'Label for asd' } ),
- new
OO.ui.MenuOptionWidget( 'fgh', { label: 'Label for fgh' } ),
- new
OO.ui.MenuOptionWidget( 'jkl', { label: 'Label for jkl' } ),
- new
OO.ui.MenuOptionWidget( 'zxc', { label: 'Label for zxc' } ),
- new
OO.ui.MenuOptionWidget( 'vbn', { label: 'Label for vbn' } )
+ new
OO.ui.MenuOptionWidget( { data: 'asd', label: 'Label for asd' } ),
+ new
OO.ui.MenuOptionWidget( { data: 'fgh', label: 'Label for fgh' } ),
+ new
OO.ui.MenuOptionWidget( { data: 'jkl', label: 'Label for jkl' } ),
+ new
OO.ui.MenuOptionWidget( { data: 'zxc', label: 'Label for zxc' } ),
+ new
OO.ui.MenuOptionWidget( { data: 'vbn', label: 'Label for vbn' } )
]
}
} ),
diff --git a/src/Element.js b/src/Element.js
index 1505a22..6dc4fb6 100644
--- a/src/Element.js
+++ b/src/Element.js
@@ -10,6 +10,7 @@
* @cfg {string[]} [classes] CSS class names to add
* @cfg {string} [text] Text to insert
* @cfg {jQuery} [$content] Content elements to append (after text)
+ * @cfg {Mixed} [data] Element data
*/
OO.ui.Element = function OoUiElement( config ) {
// Configuration initialization
@@ -17,6 +18,7 @@
// Properties
this.$ = config.$ || OO.ui.Element.getJQuery( document );
+ this.data = config.data;
this.$element = this.$( this.$.context.createElement( this.getTagName()
) );
this.elementGroup = null;
this.debouncedUpdateThemeClassesHandler =
this.debouncedUpdateThemeClasses.bind( this );
@@ -423,6 +425,26 @@
/* Methods */
/**
+ * Get element data.
+ *
+ * @return {Mixed} Element data
+ */
+OO.ui.Element.prototype.getData = function () {
+ return this.data;
+};
+
+/**
+ * Set element data.
+ *
+ * @param {Mixed} Element data
+ * @chainable
+ */
+OO.ui.Element.prototype.setData = function ( data ) {
+ this.data = data;
+ return this;
+};
+
+/**
* Check if element supports one or more methods.
*
* @param {string|string[]} methods Method or list of methods to check
diff --git a/src/elements/GroupElement.js b/src/elements/GroupElement.js
index 5d97be2..8150765 100644
--- a/src/elements/GroupElement.js
+++ b/src/elements/GroupElement.js
@@ -58,6 +58,51 @@
};
/**
+ * Get an item by its data.
+ *
+ * Data is compared by value. Only the first item with matching data will be
returned.
+ *
+ * @param {Object} data Item data to search for
+ * @return {OO.ui.Element|null} Item with equivalent data, `null` if none
exists
+ */
+OO.ui.GroupElement.prototype.getItemFromData = function ( data ) {
+ var i, len, item,
+ hash = OO.getHash( data );
+
+ for ( i = 0, len = this.items.length; i < len; i++ ) {
+ item = this.items[i];
+ if ( hash === OO.getHash( item.getData() ) ) {
+ return item;
+ }
+ }
+
+ return null;
+};
+
+/**
+ * Get items by their data.
+ *
+ * Data is compared by value. All items with matching data will be returned.
+ *
+ * @param {Object} data Item data to search for
+ * @return {OO.ui.Element[]} Items with equivalent data
+ */
+OO.ui.GroupElement.prototype.getItemsFromData = function ( data ) {
+ var i, len, item,
+ hash = OO.getHash( data ),
+ items = [];
+
+ for ( i = 0, len = this.items.length; i < len; i++ ) {
+ item = this.items[i];
+ if ( hash === OO.getHash( item.getData() ) ) {
+ items.push( item );
+ }
+ }
+
+ return items;
+};
+
+/**
* Add an aggregate item event.
*
* Aggregated events are listened to on each item and then emitted by the
group under a new name,
diff --git a/src/layouts/BookletLayout.js b/src/layouts/BookletLayout.js
index 16a7af1..4d36ebf 100644
--- a/src/layouts/BookletLayout.js
+++ b/src/layouts/BookletLayout.js
@@ -302,7 +302,7 @@
name = page.getName();
this.pages[page.getName()] = page;
if ( this.outlined ) {
- item = new OO.ui.OutlineOptionWidget( name, page, { $:
this.$ } );
+ item = new OO.ui.OutlineOptionWidget( { $: this.$,
data: name } );
page.setOutlineItem( item );
items.push( item );
}
diff --git a/src/widgets/ButtonOptionWidget.js
b/src/widgets/ButtonOptionWidget.js
index f97ae89..207e044 100644
--- a/src/widgets/ButtonOptionWidget.js
+++ b/src/widgets/ButtonOptionWidget.js
@@ -8,12 +8,11 @@
* @mixins OO.ui.ButtonElement
*
* @constructor
- * @param {Mixed} data Option data
* @param {Object} [config] Configuration options
*/
-OO.ui.ButtonOptionWidget = function OoUiButtonOptionWidget( data, config ) {
+OO.ui.ButtonOptionWidget = function OoUiButtonOptionWidget( config ) {
// Parent constructor
- OO.ui.ButtonOptionWidget.super.call( this, data, config );
+ OO.ui.ButtonOptionWidget.super.call( this, config );
// Mixin constructors
OO.ui.ButtonElement.call( this, config );
diff --git a/src/widgets/DecoratedOptionWidget.js
b/src/widgets/DecoratedOptionWidget.js
index 433f0af..9fd8ee8 100644
--- a/src/widgets/DecoratedOptionWidget.js
+++ b/src/widgets/DecoratedOptionWidget.js
@@ -9,12 +9,11 @@
* @mixins OO.ui.IndicatorElement
*
* @constructor
- * @param {Mixed} data Option data
* @param {Object} [config] Configuration options
*/
-OO.ui.DecoratedOptionWidget = function OoUiDecoratedOptionWidget( data, config
) {
+OO.ui.DecoratedOptionWidget = function OoUiDecoratedOptionWidget( config ) {
// Parent constructor
- OO.ui.DecoratedOptionWidget.super.call( this, data, config );
+ OO.ui.DecoratedOptionWidget.super.call( this, config );
// Mixin constructors
OO.ui.IconElement.call( this, config );
diff --git a/src/widgets/MenuOptionWidget.js b/src/widgets/MenuOptionWidget.js
index b2a2f85..1a6ed4c 100644
--- a/src/widgets/MenuOptionWidget.js
+++ b/src/widgets/MenuOptionWidget.js
@@ -5,15 +5,14 @@
* @extends OO.ui.DecoratedOptionWidget
*
* @constructor
- * @param {Mixed} data Item data
* @param {Object} [config] Configuration options
*/
-OO.ui.MenuOptionWidget = function OoUiMenuOptionWidget( data, config ) {
+OO.ui.MenuOptionWidget = function OoUiMenuOptionWidget( config ) {
// Configuration initialization
config = $.extend( { icon: 'check' }, config );
// Parent constructor
- OO.ui.MenuOptionWidget.super.call( this, data, config );
+ OO.ui.MenuOptionWidget.super.call( this, config );
// Initialization
this.$element
diff --git a/src/widgets/MenuSectionOptionWidget.js
b/src/widgets/MenuSectionOptionWidget.js
index f678987..8986711 100644
--- a/src/widgets/MenuSectionOptionWidget.js
+++ b/src/widgets/MenuSectionOptionWidget.js
@@ -5,12 +5,11 @@
* @extends OO.ui.DecoratedOptionWidget
*
* @constructor
- * @param {Mixed} data Item data
* @param {Object} [config] Configuration options
*/
-OO.ui.MenuSectionOptionWidget = function OoUiMenuSectionOptionWidget( data,
config ) {
+OO.ui.MenuSectionOptionWidget = function OoUiMenuSectionOptionWidget( config )
{
// Parent constructor
- OO.ui.MenuSectionOptionWidget.super.call( this, data, config );
+ OO.ui.MenuSectionOptionWidget.super.call( this, config );
// Initialization
this.$element.addClass( 'oo-ui-menuSectionOptionWidget' );
diff --git a/src/widgets/OptionWidget.js b/src/widgets/OptionWidget.js
index 8f66604..8314b32 100644
--- a/src/widgets/OptionWidget.js
+++ b/src/widgets/OptionWidget.js
@@ -7,10 +7,9 @@
* @mixins OO.ui.FlaggedElement
*
* @constructor
- * @param {Mixed} data Option data
* @param {Object} [config] Configuration options
*/
-OO.ui.OptionWidget = function OoUiOptionWidget( data, config ) {
+OO.ui.OptionWidget = function OoUiOptionWidget( config ) {
// Configuration initialization
config = config || {};
@@ -23,7 +22,6 @@
OO.ui.FlaggedElement.call( this, config );
// Properties
- this.data = data;
this.selected = false;
this.highlighted = false;
this.pressed = false;
@@ -185,13 +183,4 @@
}
return deferred.promise();
-};
-
-/**
- * Get option data.
- *
- * @return {Mixed} Option data
- */
-OO.ui.OptionWidget.prototype.getData = function () {
- return this.data;
};
diff --git a/src/widgets/OutlineOptionWidget.js
b/src/widgets/OutlineOptionWidget.js
index aa60ef8..941c481 100644
--- a/src/widgets/OutlineOptionWidget.js
+++ b/src/widgets/OutlineOptionWidget.js
@@ -5,17 +5,16 @@
* @extends OO.ui.DecoratedOptionWidget
*
* @constructor
- * @param {Mixed} data Item data
* @param {Object} [config] Configuration options
* @cfg {number} [level] Indentation level
* @cfg {boolean} [movable] Allow modification from outline controls
*/
-OO.ui.OutlineOptionWidget = function OoUiOutlineOptionWidget( data, config ) {
+OO.ui.OutlineOptionWidget = function OoUiOutlineOptionWidget( config ) {
// Configuration initialization
config = config || {};
// Parent constructor
- OO.ui.OutlineOptionWidget.super.call( this, data, config );
+ OO.ui.OutlineOptionWidget.super.call( this, config );
// Properties
this.level = 0;
diff --git a/src/widgets/SelectWidget.js b/src/widgets/SelectWidget.js
index 55c45c1..b11d863 100644
--- a/src/widgets/SelectWidget.js
+++ b/src/widgets/SelectWidget.js
@@ -1,8 +1,8 @@
/**
* Generic selection of options.
*
- * Items can contain any rendering, and are uniquely identified by a hash of
their data. Any widget
- * that provides options, from which the user must choose one, should be built
on this class.
+ * Items can contain any rendering. Any widget that provides options, from
which the user must
+ * choose one, should be built on this class.
*
* Use together with OO.ui.OptionWidget.
*
@@ -27,7 +27,6 @@
// Properties
this.pressed = false;
this.selecting = null;
- this.hashes = {};
this.onMouseUpHandler = this.onMouseUp.bind( this );
this.onMouseMoveHandler = this.onMouseMove.bind( this );
@@ -250,22 +249,6 @@
};
/**
- * Get an existing item with equivilant data.
- *
- * @param {Object} data Item data to search for
- * @return {OO.ui.OptionWidget|null} Item with equivilent value, `null` if
none exists
- */
-OO.ui.SelectWidget.prototype.getItemFromData = function ( data ) {
- var hash = OO.getHash( data );
-
- if ( Object.prototype.hasOwnProperty.call( this.hashes, hash ) ) {
- return this.hashes[hash];
- }
-
- return null;
-};
-
-/**
* Toggle pressed state.
*
* @param {boolean} pressed An option is being pressed
@@ -430,31 +413,12 @@
/**
* Add items.
*
- * When items are added with the same values as existing items, the existing
items will be
- * automatically removed before the new items are added.
- *
* @param {OO.ui.OptionWidget[]} items Items to add
* @param {number} [index] Index to insert items after
* @fires add
* @chainable
*/
OO.ui.SelectWidget.prototype.addItems = function ( items, index ) {
- var i, len, item, hash,
- remove = [];
-
- for ( i = 0, len = items.length; i < len; i++ ) {
- item = items[i];
- hash = OO.getHash( item.getData() );
- if ( Object.prototype.hasOwnProperty.call( this.hashes, hash )
) {
- // Remove item with same value
- remove.push( this.hashes[hash] );
- }
- this.hashes[hash] = item;
- }
- if ( remove.length ) {
- this.removeItems( remove );
- }
-
// Mixin method
OO.ui.GroupWidget.prototype.addItems.call( this, items, index );
@@ -474,15 +438,11 @@
* @chainable
*/
OO.ui.SelectWidget.prototype.removeItems = function ( items ) {
- var i, len, item, hash;
+ var i, len, item;
+ // Deselect items being removed
for ( i = 0, len = items.length; i < len; i++ ) {
item = items[i];
- hash = OO.getHash( item.getData() );
- if ( Object.prototype.hasOwnProperty.call( this.hashes, hash )
) {
- // Remove existing item
- delete this.hashes[hash];
- }
if ( item.isSelected() ) {
this.selectItem( null );
}
@@ -507,10 +467,10 @@
OO.ui.SelectWidget.prototype.clearItems = function () {
var items = this.items.slice();
- // Clear all items
- this.hashes = {};
// Mixin method
OO.ui.GroupWidget.prototype.clearItems.call( this );
+
+ // Clear selection
this.selectItem( null );
this.emit( 'remove', items );
--
To view, visit https://gerrit.wikimedia.org/r/174886
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7ee78b6d0b734c32fc6dd3760c6ec45362ef8571
Gerrit-PatchSet: 1
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Trevor Parscal <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
