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

Bug in sorting related to initialSortBy #436

Closed
1 task done
giraz82 opened this issue Oct 12, 2018 · 8 comments
Closed
1 task done

Bug in sorting related to initialSortBy #436

giraz82 opened this issue Oct 12, 2018 · 8 comments
Assignees
Labels

Comments

@giraz82
Copy link

giraz82 commented Oct 12, 2018

Issue Type

  • Bug

Specs

What version are you using?
2.15.0

What browser?
Chrome 69.0.3497.100, Safari 12.0

Expected Behavior

Sort changes when clicking on column headers

Actual Behavior

Sort does not change

Steps to Reproduce the Problem

https://jsfiddle.net/gira82/cq4etkp6/1/

Basically I need to have an array of ids related to selected rows.
So I have a selectionChanged method that reduce the rows selected to an array of ids.

In the fiddle I attached there are three things that make the sort working:

  1. Do not render selectedIds array in the template (comment out line 2 of html)
  2. Do not update selectedIds in the vue model (comment out line 99 of js)
  3. Remove the option "initialSortBy: {field: 'name', type: 'asc'}" from the options.

I think there is something not working properly in the sorting code.

Thank you

@xaksis
Copy link
Owner

xaksis commented Oct 12, 2018

you need to take care of the 0 length case when sort is changed:
This will make your jsfiddle work.

selectionChanged(params) {
    	/* console.log(params) */;
      if (!params.selectedRows.length) {
      	this.selectedIds = [];
        return;
      } 
      const selectedIds = params.selectedRows.reduce((acc, row) => {
        acc.push(row.id)
        return acc
      }, [])
      console.log(selectedIds);
      this.selectedIds = selectedIds
    }

closing.

@xaksis xaksis closed this as completed Oct 12, 2018
@giraz82
Copy link
Author

giraz82 commented Oct 12, 2018

Sorry, but the .reduce correctly returns an empty array when params.selectedRows is empty.
In my humble opinion, the point is not this, and the sort is still not working in the updated fiddle: https://jsfiddle.net/gira82/cq4etkp6/18/

I carefully debug the problem before I opened the issue, and I cannot see any problem in this simple code.
It's curious that removing initialSortBy all starts to work.
I can make more tests if needed.

@xaksis
Copy link
Owner

xaksis commented Oct 12, 2018

apologies for the initial haste, I'll re-open this. However if i comment out this.selectedIds assignment... everything seems to be working fine. I'll debug it more.

@xaksis xaksis reopened this Oct 12, 2018
@xaksis
Copy link
Owner

xaksis commented Oct 12, 2018

looks like if I don't print out {{ selectedIds }} , everything is working as well. strange.

@giraz82
Copy link
Author

giraz82 commented Oct 12, 2018

Yes, as I wrote at the beginning, three things will make the script work:

  1. Do not render (or use in any way) selectedIds array in the template
  2. Do not update selectedIds array at the end of the method
  3. Remove the option "initialSortBy: {field: 'name', type: 'asc'}" from the options.

1 and 2 sound inexplicable to me (after about 2 years of Vue developing)

If you look at console, it seems that the method is triggered three times every time you change sorting.
Hope this will help.

@xaksis xaksis self-assigned this Oct 13, 2018
@xaksis xaksis added the bug label Oct 13, 2018
@xaksis xaksis closed this as completed in 06b0fe7 Oct 13, 2018
@xaksis
Copy link
Owner

xaksis commented Oct 13, 2018

so just published 2.15.2 and seems to have passed all my tests... I'm going to keep this open for a bit so we can test with your jsfiddle

@xaksis xaksis reopened this Oct 13, 2018
@xaksis
Copy link
Owner

xaksis commented Oct 13, 2018

ok tested with your fiddle... seems to be working well now. Thanks for reporting this!

@xaksis xaksis closed this as completed Oct 13, 2018
@giraz82
Copy link
Author

giraz82 commented Oct 13, 2018

Thank you to you for so fast fix! 👍

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

No branches or pull requests

2 participants