emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.47k stars 4.21k forks source link

each helper not updating changes #11880

Closed QuantumKing closed 9 years ago

QuantumKing commented 9 years ago

I have a component sortable-list which has the following template

<!-- templates/components/sortable-list.hbs -->
{{#each items as |item index|}}
  {{yield item index}}
{{/each}}

and code

// components/sortable-list.js
export default Ember.Component.extend(PreserveScroll, {
  initSortable: function() {
    var refreshed = false;
    var _this = this;

    this.$().sortable({
      axis: 'y',
      update: function(event, ui) {
        Ember.run.scheduleOnce('afterRender', this, function(){
          var id = ui.item.data('id').toString();
          var idx = Ember.$(this).children().index(ui.item);
          Ember.$(this).sortable('cancel');
          _this.updateItemOrder(id, idx);
        });
      },
      start: function() { // event, ui
        if (!refreshed) {
          _this.$().sortable('refreshPositions');
          refreshed = true;
        }
      }
    });
  }.on('didInsertElement'),

  destroySortable: function() {
    this.$().sortable('destroy');
  }.on('willDestroyElement'),

  updateItemOrder: function(key, idx) {
    var items = this.get('items');
    var item  = items.findBy('id', key);
    this.beginPropertyChanges();
    items.removeObject(item);
    items.insertAt(idx, item);
    this.endPropertyChanges();
  }
});

In one of my templates I have

<!-- templates/draft/index.hbs -->
{{#sortable-list items=model.items as |item index|}}
  {{item-summary item=item index=index}}
{{/sortable-list}}

The problem I'm having is when I sort the items and updateItemOrder gets called, with removeObject and insertAt updating the items array, the list is not being re-rendered. This used to work before I moved this code from my route's view to a component.

rwjblue commented 9 years ago

Which Embwr version? Can you put together a JSBin or ember-twiddle demonstrating this?

QuantumKing commented 9 years ago

http://ember-twiddle.com/0522b1f24fe849e4204e

QuantumKing commented 9 years ago

I think it might have something to do with sortable modifying the elements, by adding classes and styles.

pixelhandler commented 9 years ago

@rwjblue the ember-twiddle shows Ember v1.13.5

dmagunov commented 9 years ago

I have same issue with ember + jqueryui sortable https://github.com/embersherpa/ama/issues/10

dmagunov commented 9 years ago

Any thoughts?

connorcimowsky commented 9 years ago

I'm seeing this too on Ember 1.13.5. Here are the relevant bits of my component:

// components/page-list.js
didInsertElement: function() {
    var pageListComponent = this;
    this.$("ul.procedure").sortable({
        items: 'li:not(:first-child)',
        update: function(event, ui) {
            var sortOrder = Ember.$(this).sortable('toArray', { attribute: 'data-id' });
            var pageModels = [];

            sortOrder.forEach(function(id, index) {
                pageModels.push({
                    id: id,
                    display_index: index
                });
            });

            Ember.$(this).sortable('cancel');
            pageListComponent.sendAction('updateSortOrder', pageModels);
        }
    });
}
<!-- templates/components/page-list.js -->
<ul class="procedure">
    {{#each pages as |page|}}
        <li class="page-template" data-id="{{page.id}}">
            <a class="open-page" {{action 'editPage' page}} data-id="{{page.id}}">
                <span class="page-label">Page {{page.id}}, Display {{page.displayIndex}}</span>
            </a>
        </li>
    {{/each}}
</ul>
dmagunov commented 9 years ago

ping @rwjblue @stefanpenner @wagenet

stefanpenner commented 9 years ago

@dmagunov we are aware of this issue, but haven't gotten to it yet.

@krisselden has implemented (and may still maintain) some ember drag & drop code. Maybe he has some free time this week.

dmagunov commented 9 years ago

@stefanpenner any news?

jrjohnson commented 9 years ago

I'm not sure if this is the same issue. I have a helper inside of an each loop and the helper doesn't seem to fire when I update one of its properties. http://ember-twiddle.com/1fa91961e97658e83049

Thoughts?

stevehanson commented 9 years ago

I am also experiencing the issue. Is there maybe some way to manually trigger the each helper to refresh?

stevehanson commented 9 years ago

It's sloppy, but I got it to work for now by wrapping the sortable-list component in an if block and forcing the block to compute false and then true immediately in secession. Adding the same code to the else block causes there to be no blip when the sortable disappears.

{{#if showList}}
  {{sortable-list ...}}
{{else}}
  {{sortable-list ...}}
{{/if}}
actions: {
  listUpdated() {
    this.set('showList', false);
    Ember.run.next(this, function() {
      this.set('showList', true);
    });
  }
}

Here's the updated twiddle: http://ember-twiddle.com/50bbf5004db9d6612a84

dmagunov commented 9 years ago

@stevehanson thanks for example, but in Your case ember rerenders whole list, so focus, selection and so on is lost.

stevehanson commented 9 years ago

@dmagunov: Yeah, definitely not ideal. What is your scenario where you have focus or selection you would like to preserve? Since the list is only re-rendered after dropping a sortable element into a new position, it doesn't seem like you would have any focus or selection to worry about.

dmagunov commented 9 years ago

@stevehanson here my case http://discuss.emberjs.com/t/can-ember-render-only-changes-for-block-elements-helpers/7627

stevehanson commented 9 years ago

@dmagunov oh, i see. Thought you were having the same problem with jQuery UI sortable. Your case seems a little trickier :(

dmagunov commented 9 years ago

I see that the developers have forgotten about us)

aldhsu commented 9 years ago

+1 just ran into the issue that @jrjohnson had with that twiddle and we had very similar code. However our issue was once you click on a checkbox, it no longer responds to changes of the checked={{is-in array item}}

A few things I noticed:

  1. The helper was recomputing true but the checked attribute was not set in the DOM
  2. Whether it was in an each or not did not seem to matter.
aldhsu commented 9 years ago

@jrjohnson After some poking around, I fixed the problem in the twiddle by specifying preventDefault=false in the action. My mystery still remains I will try and build a twiddle to replicate.

rwjblue commented 9 years ago

I'm trying to disentangle this issue and it seems that there a couple different things going on in here.


@jrjohnson / @aldhsu - There are two issues in the twiddle from https://github.com/emberjs/ember.js/issues/11880#issuecomment-130064227:

  1. A helper does not automatically observer items in an array.
  2. An {{action automatically "prevents default" on events that it handles, this prevents the checkbox from showing the checked property changes.

Fixed example: http://ember-twiddle.com/35ec52938a0cb6205da3


The main issue here is related to using jQuery sortable as experienced by @QuantumKing / @dmagunov / @stevehanson.

I do see the issue, but I'm not 100% sure what is actually going on here yet. Does this work in 1.12 (a quick switch of the twiddle to 1.12.1 seems to show even more issues)?

dmagunov commented 9 years ago

@rwjblue it doesn't work properly either on 1.12 and latest 1.13 versions. But issues is differs. The main problem with glimmer engine is to tell him that "content (DOM) of each is changed, so refresh it" after sort finished and model updated.

aldhsu commented 9 years ago

@rwjblue thanks for the quick reply. Apologies, I didn't understand helpers are analogs for computed properties in DOM and would not be recomputed unless what they observe has changed.

I forced recomputes on helpers that observed arrays by passing in array.length to not have to add observers.

rwjblue commented 9 years ago

@aldhsu - Yep, definitely works that way too!

jrjohnson commented 9 years ago

@rwjblue I didn't know either of those things and now I do! Thanks!

shidel-dev commented 9 years ago

Experiencing this same issue with https://github.com/RubaXa/Sortable.

sohara commented 9 years ago

We are also experiencing the issue with jquery sortable on Ember 1.13.9.

I have a controller action to sort our model.items by a given item attribute (filename). Do so sorts the array properly, and the computed index property (it's position in the array) updates accordingly in the UI. Only the actual sequence of the items in the DOM has not changed.

{{#sortable-list updateSortOrder='sortItems'}}
  {{#each model.items as |item index|}}
    {{#sortable-item item=item index=index}}
      {{item.filename}}
      {{gallery-item item=item activeIds=activeIds galleryId=model.id remove="remove" toggleActive="toggleActive"}}
    {{/sortable-item}}
  {{/each}}
{{/sortable-list}}

It seems the sortable plugin does something that interferes with the each helper keeping track of whether items need to be rendered. Commenting out the sortable() function on didInsertElement allows to controller action's changing of the array sort order to be reflected in the DOM.

JaroVDH commented 9 years ago

I think I have a similar issue. Don't know if this needs its own issue as it's maybe not the same.

Here's a JSbin that shows the issue: http://emberjs.jsbin.com/sasizoside/1/edit?html,js,console,output

Observers/computed properties in a component that gets its value from an each aren't triggering on changes to that value. It only seems to happen for me when that value is an object.

Workaround I've been using is having a different, non-object property trigger the observers/computed properties instead.

Ember v1.13.8

dmagunov commented 9 years ago

ping @wycats , only You can help us with this issue

bryanaka commented 9 years ago

As a work around, I was able to get our sortable table working by calling sortable('cancel') prior to changing the position (our 1-indexed index). However, this only worked if you have key property set as @index with the each helper.

each project.sortedTasks key="@index" as |task index|

krisselden commented 9 years ago

@JaroVDH http://emberjs.jsbin.com/qitusawavi/1/edit?html,js,console,output

there is nothing causing it to call the helper again because it sees the same obj.

krisselden commented 9 years ago

This issue is just a dumping ground for complaints about each with not much that is actionable. Only provided examples jsbin/ember-twiddle are actually working as designed, misunderstanding about helpers and interior object mutations. The rest are misc issues with addon breakage. Please open an issue on those addons.

adamreisnz commented 7 years ago

This still seems to be an issue in Ember 2.13. Passing a MutableArray property into a helper, will not cause that helper to recompute, if the composition of the array changes (e.g. add item, remove item). This is a severe limitation, and the workarounds are not pleasant.