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

SingleSelect and MultiSelect Editors not working correctly when sorted desc #46

Closed
ghiscoding opened this issue Apr 10, 2018 · 7 comments
Labels

Comments

@ghiscoding
Copy link
Owner

I'm submitting a bug report

Current behavior:
If we sort the collection to be descending and we have a value higher than 10, it will select an incorrect value in the editor. For example, if the value is 94, it will incorrectly choose 4.

Expected/desired behavior:

  • What is the expected behavior?
    Choose the correct value in the editor

  • What is the motivation / use case for changing the behavior?

@ghiscoding
Copy link
Owner Author

ghiscoding commented Apr 10, 2018

@jmzagorski
I found the issue in the Editor because of the use of the find and indexOf here to pre-select the default value. If we sort the collection descending, and we have a cell value of 94, the find will return 2 found items 9 and 4 and since the last found value is 4, it will select that one.

You can already see this bug on the live Example 3 demo on the "% complete" column because that one has the sorted collection in descending order.

@jmzagorski
Copy link
Collaborator

yup, can't remember why I used indexOf. I think because the multiSelect returned an array and I just copied a lot of the logic over to singelSelect without testing it good enough? I'll have to look at it

@ghiscoding
Copy link
Owner Author

I actually found that problem while doing the new feature of Excel Copy Buffer, which I just merged. This copy buffer uses the serializeValue function to copy each cell values into it's buffer when the column definition is an editable field (Example 3). So anyway, I believe the MultipleSelectEditor also has this problem since it also uses the indexOf.

@ghiscoding
Copy link
Owner Author

I tried it in my Angular repo and made these changes

-      if (this.defaultValue.indexOf($e.value) !== -1) {
+      if (this.defaultValue === $e.value) {

That seems to be all good, also note that this.defaultValue is of type string because the previous line adds a cast with toString().

You can do the change today or I'll make the change sometime tonight.

@jmzagorski
Copy link
Collaborator

That will work for the singleSelectEditor, but not the multipleSelectEditor. The multipleSelectEditor.defaultValue is an array so you need to make that indexOf check to see if the select value exists in the array. I believe this was a copy and paste error on my part from the multipleSelectEditor to the singleSelectEditor.

Side note, i was put on a high priority 1 week project since the end of last week so I have not been able to get to anything I wanted to on slickgrid.

@ghiscoding
Copy link
Owner Author

There was 2 indexOf in the MultipleSelectEditor and I changed one of them with the same modification as the SingleSelect. I think you were possibly thinking about the 1st indexOf, which I didn't change. Anyhow, I made the same change in both Editors and tested with Example 3 with the collection sorted descending and they both work as expected (I'm actually not sure if it affected the MultipleSelect at all, anyhow I changed it too).

We can re-open if it comes back, but it seems to be ok with the quick tests I made, so I'll close the issue.

@ghiscoding
Copy link
Owner Author

ghiscoding commented Apr 12, 2018

@jmzagorski
I just released last version 1.12.0 before vacations, this fix is included in it :)

A lot of goodies before I leave.

@ghiscoding ghiscoding added the bug label Jul 26, 2018
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