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

[DataGrid] onSort causes first sort in DataGrid to not work #3197

Closed
1 of 3 tasks
cgerein opened this issue Jul 11, 2023 · 6 comments
Closed
1 of 3 tasks

[DataGrid] onSort causes first sort in DataGrid to not work #3197

cgerein opened this issue Jul 11, 2023 · 6 comments
Labels
component: Datagrid needs: investigation A technical issue needs to be researched Sev 3 Visible or noticeable to users but does not impede functionality. Has a workaround. type: bug 🐛 Something isn't working version: 1 Carbon 10 / v1 version: 2 Carbon 11 / v2

Comments

@cgerein
Copy link

cgerein commented Jul 11, 2023

What package(s) are you using?

  • Carbon for IBM Products (or Carbon for IBM Cloud & Cognitive)
  • Legacy/CDAI
  • Legacy/Security

Detailed description

Passing a custom sort function to useDataGrid() with onSort causes the first sort you try to do to not change the actual sort state on the column, but the onSort function will still be called. This will cause onSort to call with the id and sort order you would expect, but the DataGrid will still show the previous order. Any clicks/changes to column order after the first click works how you would expect it to.

"@carbon/ibm-products": "2.1.2"

Steps to reproduce the issue

  1. Pass an onSort function to the DataGrid state
useDataGrid({ ...state, onSort: () => {} }, ...plugins, useSortableColumns)
  1. Go to your DataGrid and try to sort a column
  2. Depending on your function, it will have been called, but the column state will be the same
  3. Clicking the column to sort again will call the onSort function again with the same parameters, and will properly set the columns sort state

Here's an example of the issue
https://codesandbox.io/p/sandbox/tender-northcutt-5lw2vc?file=%2Fsrc%2FExample%2FExample.jsx%3A9%2C24

Additional information

  • Screenshots or code
  • Notes
@cgerein cgerein added the type: bug 🐛 Something isn't working label Jul 11, 2023
@elycheea elycheea added version: 2 Carbon 11 / v2 version: 1 Carbon 10 / v1 UX: issue Any type of experience feedback that once addressed improves or enhances the experience UX: usability Issues that impact task completion (prevents, slows down, introduces errors) Sev 3 Visible or noticeable to users but does not impede functionality. Has a workaround. labels Jul 20, 2023
@elycheea elycheea moved this to Todo 📨 in Carbon for IBM Products Jul 31, 2023
@elycheea elycheea moved this from Todo 📨 to In progress in Carbon for IBM Products Aug 3, 2023
@davidmenendez
Copy link
Contributor

davidmenendez commented Aug 29, 2023

hey @cgerein i tried to recreate this, but in your example and locally i found that adding onSort actually prevents the sort from firing at all. the problem is that when you pass onSort you're signifying that you will be handling sorting the column yourself. so when you're callback is just (a, b) => console.log(a, b) nothing happens because no sorting has actually taken place.

can you verify that what i'm seeing is correct and can you clarify what your expected behavior is?

@elycheea
Copy link
Contributor

Maybe be worth pointing the documentation to react-table here in name of separating business and component logic.

@elycheea elycheea added status: waiting for author 💬 status: open question 💬 and removed UX: issue Any type of experience feedback that once addressed improves or enhances the experience UX: usability Issues that impact task completion (prevents, slows down, introduces errors) Sev 3 Visible or noticeable to users but does not impede functionality. Has a workaround. status: waiting for author 💬 labels Aug 31, 2023
@cgerein
Copy link
Author

cgerein commented Aug 31, 2023

Hey, thanks for taking a look. I'm still seeing the issue with it in that codesandbox in the description. Essentially what I was seeing there and locally was that the first click you did to sort a column would be called with ASC, but the column state wouldn't change. By that I meant that the column header wouldn't update the sort state.

Once you clicked the header a second time it shows the ascending icon properly, but is also still passing ASC to the onSort function. After that second click the column was sorting, or calling the onSort with the correct parameters.

My expected behaviour would be that clicking twice on a column header doesn't pass ASC to the onSort twice.

@davidmenendez
Copy link
Contributor

ok i see what you're saying. let me verify that this is an issue on our end and not with the underlying react table library.

@elycheea elycheea added needs: investigation A technical issue needs to be researched and removed status: open question 💬 labels Sep 6, 2023
@elycheea elycheea moved this from In progress to Todo 📨 in Carbon for IBM Products Sep 6, 2023
@davidmenendez
Copy link
Contributor

hey @cgerein just to give a quick update- i've been investigating this, but i haven't been able to find a conclusive reason for why this is happening. i think this may be put on hold for a bit because there are some more pressing issues, but we're still looking into this.

@sstrubberg sstrubberg added the Sev 3 Visible or noticeable to users but does not impede functionality. Has a workaround. label May 14, 2024
@matthewgallo matthewgallo moved this from Backlog 🌋 to Later 🏔️ in Carbon for IBM Products Jun 5, 2024
@ljcarot ljcarot moved this from Later 🏔️ to Icebox 🧊 in Carbon for IBM Products Oct 30, 2024
@ljcarot
Copy link
Member

ljcarot commented Nov 25, 2024

We've been investigating TanStack Table, a third-party, open-source offering, which provides extensive data table capabilities surpassing what our Carbon Datagrid offers. It provides much more flexibility and customization. TanStack Table is headless which means it can easily be added alongside Datagrid component in your product or application. The benefits of more flexibility for product teams and less maintenance for Carbon makes it a win win. Lastly, it is available in multiple frameworks including React and Web Component so it provides an option to non-React product teams.

For these reasons, we have decided to transition from building our own custom table component to using an example-based approach with TanStack Table. Datagrid will still exist in our library for existing teams but we are announcing the deprecation* of the Datagrid component in v2.54.0 release so teams can begin to work through the transition. Details about how to use both Datagrid and TanStack together can be found here.

*Deprecation means that no new features will be added however sev 1 and sev 2 bugs will be supported.

@ljcarot ljcarot closed this as completed Nov 25, 2024
@github-project-automation github-project-automation bot moved this from Icebox 🧊 to Done 🚀 in Carbon for IBM Products Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Datagrid needs: investigation A technical issue needs to be researched Sev 3 Visible or noticeable to users but does not impede functionality. Has a workaround. type: bug 🐛 Something isn't working version: 1 Carbon 10 / v1 version: 2 Carbon 11 / v2
Projects
Archived in project
Development

No branches or pull requests

5 participants