imballinst / react-bs-datatable

Bootstrap datatable without jQuery. Features include: filter, sort, pagination, checkbox, and control customization.
https://imballinst.github.io/react-bs-datatable
MIT License
60 stars 20 forks source link

Remove unnecessary string assignment from Quantified value #122

Closed ahmetunal closed 2 years ago

ahmetunal commented 2 years ago

Bug: Sorting wasn't working when the row contains an object (was working in v2)

Example:

data=[
"name":{"firstname":"John", "lastname", "Doe"}, age: 42,
"name":{"firstname":"Agent", "lastname", "Smith"}, age: 49
]

we want to order by last name, we can send this with sortProps

    name: (value: any) => {
      return value.lastname;
    }

@imballinst This was working in v2, but I see you changed sortFn(quantifiedValue1) into sortFn(`${quantifiedValue1}`). Is there any specific reason for this change I'm missing?

imballinst commented 2 years ago

@ahmetunal Hmm, that's a great catch! I think it was my fault, I was a bit desperate in "silencing" the TypeScript errors so I cast it to a string. I later made this a string | number but forgot to revert the string assignment:

https://github.com/imballinst/react-bs-datatable/blob/5c7d3f9dab91d3db97b44402277d40a057f1e438/src/helpers/types.ts#L116

I didn't consider the scenario of a non-flat table column data. In that case, could you update the above line to this as well? I just tested it in my local and with your change, the build still passes, so it should be fine.

  Record<keyof TColumnType, (column: TColumnType) => TReturnType>
ahmetunal commented 2 years ago

@imballinst I just pushed the update

imballinst commented 2 years ago

It should be released as react-bs-datatable@3.0.1 now. @ahmetunal when you have the time, could you verify if this version works as this PR does? Thanks!

ahmetunal commented 2 years ago

@imballinst, I tested it, it works. Thanks. 👍