Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

changes to data model not detected #2

Closed
jmesserly opened this issue Jun 12, 2014 · 5 comments
Closed

changes to data model not detected #2

jmesserly opened this issue Jun 12, 2014 · 5 comments

Comments

@jmesserly
Copy link

core-list creates a shallow copy of each item's data, so it can recycle the DOM node when it scrolls out. One issue is it never observes the original data items, so changes to them aren't detected. For example, modify demo.html to have this "ready" method:

    ready: function() {
      this.data = this.generateData();

      function updateTime() {
        console.log('update times!');
        var now = new Date().toString();
        for (var i=0; i<this.count; i++) {
          this.data[i].time = now;
        }

        this.myJob = this.job(this.myJob, updateTime, 1000);
      }

      updateTime.call(this);
    },

until you scroll, items don't update.

This could be fixed by either a periodic dirty-check or by Object.observe on the original model items and proxy changes back to the shallow copies. Personally I'd lean towards the second approach (using O.o via observe-js).

Of course, if TemplateBinding gets built-in DOM recycling, this will be a non-issue.

@debianw
Copy link

debianw commented Aug 4, 2014

One workaround I found is to append the original object to the physical object inside pushItemData method (https://github.com/Polymer/core-list/blob/master/core-list.html#L211)

Like this:

pushItemData: function(source, dest) {
  for (var i = 0; i < this._propertyNames.length; ++i) {
    var propertyName = this._propertyNames[i];
    dest[propertyName] = source[propertyName];
  }

  // append reference to the original object
  dest._ = source;
},

And then bind properties related to the original object like this:

<core-list data="{{data}}" height="80">
  <template>
    <div class="item {{ {selected: selected} | tokenList }}"> {{_.name}} <br /> 
      <p><i>{{_.description}}</i></p> 
    </div>
  </template>
</core-list>

Using this approach now you can see changes to the original data.

@kevinpschaaf kevinpschaaf added p1 and removed p2 labels Aug 21, 2014
@kevinpschaaf
Copy link
Contributor

2-way binding will be resolved in core-list improvements currently on work-in-progress branch https://github.com/Polymer/core-list/tree/2way-improved. It will involve a minor change to the API (bind to model.foo rather than foo directly, which will be documented when released.

@poussa
Copy link

poussa commented Sep 8, 2014

@kevinpschaaf When do you think this will be fixed and released? I though the sync attribute would take care of keeping the model and list data in sync but just found out this issue so that seems not to be the case. I'll be waiting and needing the fix... Thanks.

@kevinpschaaf
Copy link
Contributor

Since the fix for this introduces a breaking change to the API, before we release it I am trying to get far enough through the implementations of other high-priority features that could also impact API (particularly variable-height, grouping, and grid layout) to at least understand what other API changes those might drive, so that we only break the API once. That said, I can probably merge the 2-way binding fixes separate from the other features once I get a good feeling for the final API's -- rough estimate for getting to that point is another a week, maybe a bit longer.

@poussa
Copy link

poussa commented Sep 9, 2014

Thanks for the update.

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

No branches or pull requests

5 participants