Skip to content
This repository was archived by the owner on Dec 28, 2024. It is now read-only.

Avoid to create a new array each selection#153

Open
jorgecasar wants to merge 1 commit intoPolymerElements:masterfrom
jorgecasar:fix/152-selectedItems-avoid-new-array
Open

Avoid to create a new array each selection#153
jorgecasar wants to merge 1 commit intoPolymerElements:masterfrom
jorgecasar:fix/152-selectedItems-avoid-new-array

Conversation

@jorgecasar
Copy link

Solve #152
Signed-off-by: Jorge del Casar jorge.casar@gmail.com

selector.multi = true;
selector.selectedValues = ['non-existing-value'];
assert.deepEqual(selector.selectedItems, [undefined]);
assert.deepEqual(selector.selectedItems, []);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you select a non existing value the selectedItems must be an empty array

@jorgecasar jorgecasar force-pushed the fix/152-selectedItems-avoid-new-array branch 2 times, most recently from f90849f to d29ebb7 Compare April 28, 2017 15:01
if (this.multi) {
this._setSelectedItems(s);
if (isSelected) {
this.splice('selectedItems', this.selectedItems.length, 0, item);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not simply this.push('selectedItems', item);?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, have sense. Changed.

} else {
this._setSelectedItems([s]);
this._setSelectedItem(s);
this._setSelectedItems(isSelected?[item]:[]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces: this._setSelectedItems(isSelected ? [item] : []);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

this._setSelectedItems([s]);
this._setSelectedItem(s);
this._setSelectedItems(isSelected?[item]:[]);
this._setSelectedItem(isSelected?item:undefined);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces: this._setSelectedItem(isSelected ? item : undefined);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

_selectionChange: function() {
this._setSelectedItem(this._selection.get());
_selectionChange: function(isSelected, item) {
this._setSelectedItem(isSelected?item:undefined);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces: this._setSelectedItem(isSelected ? item : undefined);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@jorgecasar jorgecasar force-pushed the fix/152-selectedItems-avoid-new-array branch from d29ebb7 to 1750695 Compare April 29, 2017 19:05
Copy link
Member

@abdonrd abdonrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jorgecasar 👏

LGTM! 😄

@jorgecasar jorgecasar force-pushed the fix/152-selectedItems-avoid-new-array branch from 7aee6b4 to 1750695 Compare May 13, 2017 07:15
@jorgecasar jorgecasar force-pushed the fix/152-selectedItems-avoid-new-array branch from 1750695 to d4d03e9 Compare June 5, 2017 17:18
Signed-off-by: Jorge del Casar <jorge.casar@gmail.com>
@jorgecasar jorgecasar force-pushed the fix/152-selectedItems-avoid-new-array branch from d4d03e9 to f10c99c Compare June 25, 2017 10:20
@abdonrd
Copy link
Member

abdonrd commented Mar 11, 2018

Friendly ping to @bicknellr (code owner).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants