Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sorted list not displayed correctly in riot v3.0.5 #2205

Closed
cristi-salcescu opened this issue Jan 6, 2017 · 14 comments
Closed

Sorted list not displayed correctly in riot v3.0.5 #2205

cristi-salcescu opened this issue Jan 6, 2017 · 14 comments
Labels

Comments

@cristi-salcescu
Copy link

@cristi-salcescu cristi-salcescu commented Jan 6, 2017

  1. Describe your issue:

I have problems displaying a sorted list in riotjs v3.0.5
I created a sample where every 3s a list of objects is modified, then sorted and displayed with riot.

  1. Can you reproduce the issue?

https://jsfiddle.net/cristi_salcescu/4gpmovqa/

  1. On which browser/OS does the issue appear?

All browsers

  1. Which version of Riot does it affect?

3.0.5

  1. How would you tag this issue?
  • Question
  • Bug
  • Discussion
  • Feature request
  • Tip
  • Enhancement
  • Performance
@rsbondi rsbondi added the bug label Jan 6, 2017
@rsbondi
Copy link
Contributor

@rsbondi rsbondi commented Jan 6, 2017

works with no-reorder

@rsbondi
Copy link
Contributor

@rsbondi rsbondi commented Jan 6, 2017

It seems to be not dealing with the changing values correctly

    	that.items[2] = { fname: generateString() + "UPDATE" };
    	that.items[4] = { fname: generateString() + "UPDATE" };

remove this and the new values sort correctly

@cristi-salcescu
Copy link
Author

@cristi-salcescu cristi-salcescu commented Jan 6, 2017

Using no-reorder is not an option as I had problems with it.
Thanks for the second hint. It made me realize that doing the update like this works :

that.items[2].fname = generateString() + "UPDATE";
that.items[4].fname = generateString() + "UPDATE";
that.items.push({ fname: "new" + generateString() });
that.items.push({ fname: "new" + generateString()}); 

I still have sorting problems when deleting and adding items :

that.items.splice(2, 1);
that.items.splice(4, 1);
that.items.push({ fname: "new" + generateString() });
that.items.push({ fname: "new" + generateString()});  
@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Jan 9, 2017

@cristi-salcescu it's a bug thanks for reporting it. I made a simpler example starting from your original code https://jsfiddle.net/b8zm0bg7/. We will fix it soon

@cristi-salcescu
Copy link
Author

@cristi-salcescu cristi-salcescu commented Jan 10, 2017

There are still problems in 3.0.6 when testing it with more items (50) and using setInterval not setTimeout.
https://jsfiddle.net/cristi_salcescu/7sw8k4qu/

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Jan 10, 2017

@cristi-salcescu the problem seems to be in underscore, it modifies your original array I have tried with the normal js sort function and everything works fine https://jsfiddle.net/7sw8k4qu/7/

GianlucaGuarini added a commit that referenced this issue Jan 10, 2017
@cristi-salcescu
Copy link
Author

@cristi-salcescu cristi-salcescu commented Jan 11, 2017

In https://jsfiddle.net/7sw8k4qu/7/ the sort function is using "name" property which is undefined. It should sort by "fname".
I removed underscore from the sample and increase the number of initial items to 30. It doesn't work correctly.
https://jsfiddle.net/cristi_salcescu/7sw8k4qu/9/

@alexssung
Copy link

@alexssung alexssung commented Jan 12, 2017

I'm also having issues on v3.0.7 with adding to and updating a list, then sorting it. I used both underscore sortBy() and plain javascript sort() functions--both had the same issue.

After many hours, I found out about "no-reorder" and ended up adding it to the loop tag and it worked.

No idea what no-reorder does or why it worked.

@1984weed
Copy link

@1984weed 1984weed commented Jan 18, 2017

I also have this similar issue.
http://plnkr.co/edit/L4CwQKp5AQ4pEpTjVo65?p=preview

In it's case , new create object is always sorted last element.
Of course it works with no-reorder. But it's not clear.

It might be better no-reorder is default property.

@CosmoMyzrailGorynych
Copy link

@CosmoMyzrailGorynych CosmoMyzrailGorynych commented Jan 19, 2017

Splicing arrays really goes mad in 3.0.7. It pulls extra objects or even duplicates them (in DOM).

@gabrielmoreira
Copy link

@gabrielmoreira gabrielmoreira commented Jan 20, 2017

I'm trying to figure out a bug, and I think it might be related.

https://output.jsbin.com/jujobe#/todos/active

Whenever I mark the input check on this todo-list, the correctly marked item disappears from the list, but something I do not know is ticking other input checks in my list, even without rendering the checked attribute on dom.

Removing no-reorder the problem is sorting, but adding no-reorder the problem is this incorrectly checked inputs.

@bradleypeabody
Copy link

@bradleypeabody bradleypeabody commented Jan 29, 2017

Ran into this same issue. Here is another complete example of the problem in case it helps:
http://jsfiddle.net/bkw5ommu/26/

It works with no-reorder, but without if you try typing in something that filters (try putting "mo" in the box) and then clearing that field out you get all the records back but in the wrong sequence. It's not a matter of arrays being out of sequence in the view's data, because it works by just adding 'no-reorder'. Something in the DOM rendering is causing this.

Was able to reproduce the problem in 3.0.7 and 3.0.6 but not in 3.0.5.

@sourcegr
Copy link
Contributor

@sourcegr sourcegr commented Feb 1, 2017

@bradleypeabody

change your 77 line to:

self.tripListFiltered.push(JSON.parse(JSON.stringify(trip)))

@CosmoMyzrailGorynych
Copy link

@CosmoMyzrailGorynych CosmoMyzrailGorynych commented Feb 3, 2017

@papas-source it is a sadisic performance killer O_O

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.