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

gridstate sortcolumns field instead id property of column #1467

Closed
5 tasks done
zewa666 opened this issue Apr 15, 2024 · 5 comments · Fixed by #1469
Closed
5 tasks done

gridstate sortcolumns field instead id property of column #1467

zewa666 opened this issue Apr 15, 2024 · 5 comments · Fixed by #1469

Comments

@zewa666
Copy link
Contributor

zewa666 commented Apr 15, 2024

Describe the bug

hey there,

when using the OData Service in combo with gridstate events to persist the user selection to e.g localstorage I do get errors when restoring cols on next refresh.

the root cause was brought down to the columnId containing the field property instead of the id as seen here

I didnt dig deeper but guess there were reasons for doing so in order to make sorting work. do you know why or should I dig deeper?

Reproduction

  • activate odata service
  • add gridstate change listener
  • store to localstorage
  • apply to preset when refreshing

Which Framework are you using?

Angular

Environment Info

| Executable          | Version |
| ------------------- | ------- |
| (framework used)    | VERSION |
| Slickgrid-Universal | latest |
| TypeScript          | VERSION |
| Browser(s)          | VERSION |
| System OS           | VERSION |

Validations

@ghiscoding
Copy link
Owner

hmm I'm not really sure about this one, I actually haven't used the OData Service in a very long time, @jr01 might have more knowledge since I know he's a big user (and contributor to the OData Service). @jr01 are you using the OData with Grid State/Presets?

@zewa666
Copy link
Contributor Author

zewa666 commented Apr 15, 2024

looking at the graphql implementation, where its done correctly

, this perhaps was simply a mistake

@ghiscoding
Copy link
Owner

ghiscoding commented Apr 15, 2024

oh ok yeah that must be a typo error, I'd be ok to change it since yes it's supposed to be the column id as the name suggest (which is often, but not always, identical to the column name). By the way, I had to click on the link you provided and open it to see the differences, I'm not sure if you knew or not but you should select multiple lines then click on Copy Permalink to be able to see a few lines in here (note that this only works with code that is in the same repo, for example that wouldn't work pasting Angular-Slickgrid permalink in here)

image

and that gives me the following, the other good thing is that it's also assigned to a commit, so if the code changes in the future, the link will always point to the code that the permalink was created with :)

currentSorters.push({
columnId: column.sortCol.id + '',
direction: column.sortAsc ? SortDirection.ASC : SortDirection.DESC
});

@zewa666
Copy link
Contributor Author

zewa666 commented Apr 15, 2024

ok I'll create a PR for that. good opportunity to read up on the OData implementation

@jr01
Copy link
Collaborator

jr01 commented Apr 16, 2024

We currently only store the columns (not sorters and not filters) from the grid state in our backend, so we won't be affected by the fix :)

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