dom-repeat is not properly updating elements on filter/sort

[jshcrowthe]

When you utilize the filter/sort attributes the repeated nodes are not updating properly. This is the same behavior that was exhibited in the Polycast on using core-list.

Rob gave a workaround for this by using data-index="{{index}}". This seemed to fix the issue for his use case but when using the filter and sort attributes things get weird really quick.

I threw together a quick demo of this (it utilizes iron-image and iron-ajax so I opted to just give you a repo instead of a jsbin) it is here (Pardon the whimsical API). A link to the running demo: http://jshcrowthe.github.io/dom-repeat-bug/ (After the data loads note the position and color/images utilized then mess with the filters).

[andrepl]

+1
I think I'm seeing something similar, I have a nested dom-repeat rendered as a list of tables, my sort function is descending on a numeric value, and 2 of the source tables are already sorted in ascending order. The rendered dom for those 2 tables includes an duplicate bottom row, the remain table shows no issues.

[jshcrowthe]

I have seen the same issue while using the sort attribute. I have updated the demo with an example. Note: The buttons can be clicked more than once to toggle ascending vs. descending order

[arthurevans]

Can you describe the issue you're seeing, because I'm playing with the demo and I'm not seeing anything obvious. This could be a result of my ignorance of Pokemon, but if you could give a specific set of steps that produce a specific incorrect list, that would be helpful.

[jshcrowthe]

Again I apologize for the Pokemon, it's the most easy to use free API I've come across so it's kinda my default for tinkering that requires an API.

The easiest way to see this is by using the sort buttons. Load up the page and note the initial background of the 5th pokemon-card element. Click the name sort button and note that the 5th element has been sorted to the 2nd element (as it should) however the associated background (which is applied as a CSS class in the ready function) does not follow the rest of the data.

This has caused me to think that the DOM itself is not being sorted but rather the data inside of the DOM and then the rendered elements are simply being reused to display the remaining data (similar to how core-list functioned before). Did you guys merge the functionality of core-list into the dom-repeat template?

[arthurevans]

I think you're on to something there, although I don't think the dom elements are actually re-used in this instance, it may have something to do with the way the elements are initialized. @kevinpschaaf would know for sure.

In this case for example, using a computed binding in place of the ready method fixes the background:

 <div id="wrap" class$="{{computeClass(pokemon)}}">

...

computeClass: function(pokemon) {
  var baseClass = "layout horizontal center justified ";
  return baseClass + this.getType(pokemon) + '-gradient';
},

[andrepl]

I was able to reproduce my issue and now I'm not sure it's directly related to this one, but here it is.
I was only able to reproduce it in a nested repeat, and it requires adding items one by one to a sorted repeat.

http://jsbin.com/goxaharici/4/edit

[kevinpschaaf]

@jshcrowthe The issue you're seeing is indeed due to dom-repeat's reuse of DOM for efficiency.

The pokemon-card locks in state for the element at ready time, rather than when the pokemon property changes: https://github.com/jshcrowthe/dom-repeat-bug/blob/gh-pages/components/pokemon-card/pokemon-card.html#L124

We will likely add an option for dom-repeat to re-create DOM whenever the model would need to change for cases when users can't avoid using elements whose internal state can't be dynamically controlled via bindings to its properties, at the cost of less-efficient updates.

However, note that in general we consider it an anti-pattern to create elements that do one-shot state setup, such as inspecting attributes/properties at created or ready time only, as opposed to using observers on those properties, since this is how all native HTML elements behave (consider if you could only set a checkbox's checked value once at startup...). So I'd encourage you to refactor your pokemon-card element to use observers/computed properties for the properties you use to set the class, such that the state of the element can be controlled dynamically via its API.

Something like...

...
  <div id="wrap" class$="computeClass(pokemon, 'layout horizontal justified center')">
...

    properties: {
      pokemon: Object,
      value: null
    },
    computeClass: function(pokemon, staticClasses) {
      var c;
      if (!pokemon || !Array.isArray(pokemon.types)) {
        c = 'normal-gradient';
      } else {
        c = pokemon.types[0].name + '-gradient';
      }
      return c + ' ' + staticClasses;
    }

[kevinpschaaf]

@andrepl Looks like you had a couple of typos in your sample that I corrected here, which I think matches your intent? http://jsbin.com/cubaru/2/edit For each click to go, it was creating a new category, but then pushing items onto categories[0] & [1] (which was wrong for subsequent calls to go, should have been 3 & 4, 5 & 6, etc.); I fixed that one way, but just creating the new items array and pushing that onto categories in one go.

If you can reproduce another issue, it'd be great if you could open a separate issue so we can track these separately.

[kevinpschaaf]

Keeping this issue open to consider adding a restamp option as a fallback for users when DOM reuse is not possible.

[andrepl]

@kevinpschaaf the changes you made to my example do prevent the bug from occurring, but it isn't always possible to build up the second collection BEFORE pushing the parent item as you've done. the bug is exposed when items are pushed onto the nested collection. I've re-opened a new issue as this definitely doesn't seem related: #1744. more info on that ticket.

[jshcrowthe]

@kevinpschaaf I updated my internal project using computed classes and that solved one of my two issues. I seem to also be having an issue using an iron-image inside of a dom-repeat nested in another dom-repeat. I will work on reproducing the issue for you and will let you know when I've got it up and running.

[ryanwtyler]

Think my issue is related. I get an "orphaned" user input after Dom-repeat sorts. Enter 111 in first input and click button. The 111 is left in the first position but the item it belongs to has been resorted

http://codepen.io/arthurevans/pen/QNEPZb

[arthurevans]

@kevinpschaaf Don't know if we're still considering adding a restamp option to dom-repeat. I've been under the (incorrect) assumption that sort was re-ordering the DOM nodes, as the 0.5 code did.

This is problematic for stateful elements like inputs, as shown in @ryanwtyler's example, and the doc doesn't currently provide any insight or warnings here. (Ref. incorrect assumptions, above.)

There are a couple of workarounds I can see here for @ryanwtyler's use case.

1. Data bind the input values. This works, but looks a little weird in this case.

2. Have the parent component manage the input's state. This makes a certain amount of sense, because after the sort, you probably want to manually set the focus and/or scroll state. Here's one example: http://codepen.io/anon/pen/PNGLxJ ... In this particular use case, I'm not sure what the correct behavior is -- you might want to focus and/or scroll to the item you've just updated, or you might want to focus and/or scroll to the new top item. In either case, this is an application-level decision that the dom-repeat can't make for you. The tricky case here would be retaining focus and/or scrolling to the "current" item... Because the elements have changed sort order, you'd actually need to:

   1. Update the data.
   2. Force the list to render (dom-repeat.render for instance).
   3. Find the new position for the item by iterating through the inputs looking for the one for which dom-repeat.itemForElement(input) == currentItem.

   (I still think we could use some kind of "instanceForItem" API for these cases.)

3. Instead of having inputs for each line item, redesign the UI so the user taps to edit an individual line, bringing up a single edit dialog. (Probably works better on mobile, if that's a concern.) Example: http://codepen.io/anon/pen/bpwJdx (Note that you have to click the save or cancel buttons to close the dialog. No enter key handling.)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed after being marked stale. If you're still facing this problem with the above solution, please comment and we'll reopen!