dom-repeat: Instance to item binding

[mattpalermo]

Description

If important instance state is not handled by property binding,
using dom-repeat currently leads to unexpected behaviour when the
items list is re-arranged. A good example of this is input focus
which will not behave as the user expects. By optionally binding
instances to items, the developer can ensure that unmanaged
instance state will follow the items as the user would expect.

Possible solution

I have implemented a possible solution to this problem which can be found here: 1d07acb.

The basic idea is that you can elect a property of the item object to be used as a key like this:

<dom-repeat ref-prop='key'>
  <template>
    <span>My key is:</span><span>{{item.key}}</span>
  </template>
</dom-repeat>

and then a stable instance will always be used to represent that item (even if the items array is changed or the sort and filter functions change). The only time the instance identity will change is if the dom-repeat is ever rendered and the key doesn't exist (the instance will be discarded).

I haven't created a PR yet because I would like to get some feedback about this. If feedback is positive I'll write some unit tests in test/unit/dom-repeat.html and submit a PR.

All existing tests seem to be passing so I don't think I have broken anything 🥇

[kevinpschaaf]

Hi @mattpalermo, I have a PR of a restamp option for dom-repeat with support for an optional restampKey, which is very much in the same spirit as the solution you proposed: #4363

I think it definitely needs to be an option because performance-wise, maintaining instance↔item mapping requires disconnecting and re-connecting an arbitrarily large tree of DOM per instance, potentially for all instances, potentially containing an arbitrary number of custom elements whose disconnectedCallback and connectedCallback will be spammed due to the move. It is difficult to say whether that would be better or worse performance than keeping them in place and re-running binding work, so I think it's probably good to leave it as a choice.

I made that PR in response to a similar user issue, but never got feedback from the original reporter as to whether it solved his problem, so it'd be really great if you could try the branch and the new options and see if it addresses your issue.

The __renderRestamp impl is slightly more sophisticated than what you did, in that it pools as it does one iteration over the items, which will perform less work in some cases, e.g. push to end (but still degrades to all elements being disconnected/reconnected for e.g. unshift to start).

All that said, I don't think this will solve the "focus gets lost upon array mutation" since simply disconnecting and reconnecting a focused item causes it to lose focus: http://jsbin.com/xibove/edit?html,console,output. I believe the only way to solve this is what we've seen similar frameworks do, which is detect which instance is focused and move other items around it. I think this could probably be fit into this algorithm as well.

[mattpalermo]

Hi @kevinpschaaf. Thank you, I will definitely check out your restamp and restampKey option. I see what you are saying about the focus. That is a shame, I assumed the focus would move with the element. Since focus is my major motivation for this work, I would love to see/get your focus suggestion implemented. I'd hate to have to pull in some huge UI rendering framework (React) in order to solve this.

If your restamp solution re-uses instances (and therefore individual elements persist) then perhaps there is an easier way to solve the focus problem. If you let focusedEl = Document.activeElement before render, then after render you could simply call focusedEl.focus(). What do you think about this solution @kevinpschaaf? I admit it seems a little dodge since we could be 'touching' an element in areas of the dom tree that we shouldn't be touching.

[mattpalermo]

A JSbin of the quick fix mentioned above: http://jsbin.com/buhosayesi/edit?html,console,output

[mattpalermo]

+1 Brilliant! The restamping feature solved my focus problem. Well at least on firefox and chrome. I will have to write some tests and run it through saucelabs to check if it works for all browsers. https://github.com/mattpalermo/PolyList/commit/2f247a728e9f17faa9a8f33096bffad02f5525c8

For reference, the specific problem I was having was that if the user removes all the contents of a note (an input element), then that note should be deleted when the user moves away (a blur event). If the blur event occurred because the user selected another note (another input element), then the deletion of the note would make the focus shift to the note below what the user wanted. Using your restamp feature solves the problem. Focus shifts when the items array is spliced.

+1 for the feature where it will use the item object ID as a key by default. That makes it so simple to use.

[TimvdLippe]

#4363 has been merged and is scheduled to be published in the next release.

[njbartlett]

#4363 was reverted so this bug needs to be reopened.

[TimvdLippe]

We are now tracking all issues related to dom-repeat's restamp in #1713, so please subscribe to that issue.