PolymerElements / iron-list

Element for a virtual, "infinite" list
https://www.webcomponents.org/element/PolymerElements/iron-list
219 stars 131 forks source link

iron-list not maintaining bindings in items array #300

Open tstewart15 opened 8 years ago

tstewart15 commented 8 years ago

Description

I'm attempting to use iron-lists to display 2 arrays: 1 array being a filtered subset of the other array. The linked demo illustrates this example; the iron-list on the left is read-only and shows the unfiltered data array, while the iron-list on the right is editable and shows the filtered data. The filtering is based on whether the "Filter Checked Items" switch is on. Each time the filter switch is toggled, the data-binding between the left and right arrays is maintained using Polymer.Base.linkPaths - you can see that the data-binding works by checking and unchecking items in the right list (with the filter switch on or off) and seeing the changes on the corresponding items in the left list.

The problem is that iron-list is incorrectly updating the data bindings for rows that are hidden / not-being-used... then, when the hidden row comes back into view, it doesn't update correctly because it thinks that it is already up-to-date. This is what the "Steps to reproduce" scenario below is showing. You can see the problem occur in the code by setting a breakpoint on iron-list.html:1182 before Step 3, performing Step 3 (turn filter off), resuming script execution (F8 in Chrome Dev Tools) until item is the "Item3" object, stepping into the function call (F11) twice (getting to polymer.html:1416), then advancing one line (F10). Here, you can see that the old variable is already equal to the new value (notice both have selected: false), therefore no update occurs. After letting the code execution run to completion, however, the display shows selected as true. Based on this, I have 2 issues/questions:

  1. Why is the template instance for the 3rd row element in the iron-list being updated when performing Step 2 (de-selecting the item in the 2nd row element). This is why the old variable has selected: false and is preventing the correct update.
  2. Clearly the template instance for the 3rd row element is being updated correctly at some point (selected: false is the desired data-binding that we want). Why isn't the checkbox reflecting that value?

Note that this issue does not occur when replacing the iron-list with a dom-repeat template.

Live Demo

http://jsbin.com/rugajexeqo/edit?html,console,output

Steps to reproduce

  1. Turn "Checked Items" filter ON.
  2. De-select Item3. Notice that the binding is reflected in unfiltered array on the left.
  3. Turn "Checked Items" filter OFF.
  4. ISSUE: Item3 appears selected in the list. Notice in the console that the filteredItems array shows Item3 with selected: false

    Browsers Tested

    • Chrome (v52)
    • Firefox (v45.3)
akc42 commented 8 years ago

I have exactly the same problem - it appears that although the underlying model is updated by all the methods that are supposed to properly notify the changes, the display on screen doesn't change.

I have constructed a js bin demo of exactly this happening. I have a variety of ways trying to update the underlying data - including via the model and the selection - also showing what happens on clicking an item.

In all cases I prove that the underlying data has updated, but the view in screen doesn't change The jsbin will update every 5 seconds, and output info on the conssole. Also it shows what happens in the user clicks on an item. Note that the Icon marking the selected row DOES Change.

http://jsbin.com/bizizat/edit?html,console,output

akc42 commented 8 years ago

I have done a preliminary investigation with the debugger, and there is something strange in the way the _itemsChanged seems to work.

Having worked out the path of the change it calls

this._forwardItemPath(change.path.split('.').slice(1).join('.'), change.value);

This is turn then calls

el._templateInstance.notifyPath(path, value, true);

Unfortunately since the model has already been updated before _itemsChanged get called, it does nothing as the values haven't changed.

What I am still not sure I understand is why setting the items via the model didn't already change the screen. I will have a deeper look later today when I get some time.

43081j commented 8 years ago

FYI it looks like this is because here, we only unset the key but not the value.

So...

When you filter items:

When you unfiltered items:

To fix this, you could forcefully set inst[this.as] = null in the line linked above, however you'd probably want to test the performance of such a change.

Reason being, it means you will be re-propagating data every time an item comes back from being hidden. Maybe the perf loss is insignificant, though.

cc @blasten

blasten commented 8 years ago

@43081j Thanks a lot for looking at this issue and sending a PR! Just for history, the line number is 1259

blasten commented 8 years ago

This issue is also related to: https://github.com/PolymerElements/iron-list/issues/212

atl3tico commented 7 years ago

My first Polymer app implements iron-list & toggling checkboxes. So lucky. =(

keanulee commented 7 years ago

As @43081j mentioned above, this is due to 2 reasons: 1) the recycled element(s) are not removed (just hidden) and 2) when the element is unhidden, the dirty check determines the object is equal so no updates are done.

As an update, by default this behavior is still present in Polymer 2.0 and iron-list 2.0 (http://jsbin.com/hukugi/1/edit?html,output). However, with Polymer 2.0 and iron-list 2.0, you can now add the mutable-data attribute to <iron-list> which will skip dirty checking and always trigger side effects (http://jsbin.com/hukugi/2/edit?html,output). For more details on this, see https://www.polymer-project.org/2.0/docs/devguide/data-system#mutable-data.

davidmaxwaterman commented 7 years ago

I failed to make this work, so switched back to using dom-repeat, which is a shame. I wonder if there is any work being done to fix this problem. Would someone mind updating us? Thanks :)

danbeam commented 7 years ago

fwiw: as a workaround, you can probably change the data in a way that doesn't notify, then call notifySplices(). but this is kind of a lame hack.

It worked for me: https://chromium-review.googlesource.com/c/chromium/src/+/688814

danbeam commented 7 years ago

/cc @freshp86