PolymerElements / iron-selector

Manages a list of elements that can be selected
32 stars 55 forks source link

Changes to selectedValues not reflected in display #112

Closed andrewmwhite closed 8 years ago

andrewmwhite commented 8 years ago

For instance, removing an element from selectedValues does not result in it's "selected" appearance changing to deselected. See JS Bin here.

The tests appear to cover cases where the selectedValues array is overwritten, not modified (pushed, shifted, spliced, etc., by the Polymer array mutation methods).

bicknellr commented 8 years ago

Thanks for the repro; my first thought / hope is that this might be as simple as changing this observer to '_updateSelected(selectedValues.*)'.

andrewmwhite commented 8 years ago

Neither using 'selectedValues.*' nor 'selectedValues.splices' works, at least according to this test case: https://github.com/andrewmwhite/iron-selector/commit/64f80e86a63bb656e76b0e2ea8861d4d448d565c

bicknellr commented 8 years ago

@andrewmwhite, I talked with @cdata yesterday about this and he mentioned that selectedValues.* provides both the full array and the splices object at different times. Also, it doesn't seem like your branch actually changes the observer - am I missing something?

andrewmwhite commented 8 years ago

My branch doesn't -- it just provides the test case. I tried both 'selectedValues.*' and 'selectedValues.splices' offline and neither worked.

bicknellr commented 8 years ago

Oh, I just noticed that your tests use the native array methods; unfortunately, these won't trigger updates to bindings / observers / computed properties. Switching s.selectedValues.arrayFn(...args) to s.arrayFn('selectedValues', ...args) for these functions should help.

andrewmwhite commented 8 years ago

Apparently my brain was checked out yesterday -- thanks for pointing that out. I've fixed the test case and implemented your suggested change, which passes the (fixed) tests. I'll submit a PR once I verify this fixes the downstream problem I was encountering.

bicknellr commented 8 years ago

Awesome!