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]: Column customization vs getAutoSizedColumnWidth #5441

Closed
2 tasks done
BenceLovas opened this issue Jun 10, 2024 · 3 comments
Closed
2 tasks done

[Datagrid]: Column customization vs getAutoSizedColumnWidth #5441

BenceLovas opened this issue Jun 10, 2024 · 3 comments
Assignees
Labels
role: dev Sev 2 Aspects of design is broken and impedes users in a significant way, but has workaround. type: bug 🐛 Something isn't working

Comments

@BenceLovas
Copy link

Package

Carbon for IBM Products

Description

TLDR: Setting the data of the table with a setState function, re-creates the columns definition array, which overrides the visibility of the columns set by the "column customization" feature.

So when we initially define the columns, we call the getAutoSizedColumnWidth function so we can calculate the required width for the column. We need the data as a dependency in the useMemo hook, without it the width would be calculated before the data arrives from the server, and that would lead to the column's text being truncated.

This data later on is updated by setData (in this example it's a delete, but it could be anything else), and because the data has been updated, the column definition array gets updated, which overrides the setting that has been already there, thus - in our case - the "Age" column disappears.

So, currently we feel like we have to decide between having well sized column widths, or we use the column customization feature with truncated columns. None of those are (I think understandably) appealing for our users.

Let me know if I should elaborate on this more, or happy to jump on a meeting if necessary. Thank you!

Component(s) impacted

Datagrid

Browser

Chrome

@carbon/ibm-products (previously @carbon/ibm-cloud-cognitive) version

2.34.0

Severity

Severity 2 = Aspects of design is broken, and impedes users in a significant way, but there is a way to complete their tasks. Affects major functionality, has a workaround.

Product/offering

IBM internal

CodeSandbox or Stackblitz example

https://codesandbox.io/p/devbox/modest-shockley-3lhyw7?file=%2Fsrc%2FExample%2FExample.jsx%3A74%2C5

Steps to reproduce the issue (if applicable)

  1. Open column customization
  2. Select "Age" column as well
  3. Delete one item with the row action button
  4. "Age" column disappears

Release date (if applicable)

No response

Code of Conduct

@github-project-automation github-project-automation bot moved this to Needs triage 🧐 in Carbon for IBM Products Jun 10, 2024
@matthewgallo matthewgallo added type: bug 🐛 Something isn't working Sev 2 Aspects of design is broken and impedes users in a significant way, but has workaround. role: dev and removed status: needs triage 🕵️‍♀️ labels Jun 10, 2024
@matthewgallo matthewgallo moved this from Needs triage 🧐 to Backlog 🌋 in Carbon for IBM Products Jun 10, 2024
@devadula-nandan devadula-nandan self-assigned this Jun 10, 2024
@devadula-nandan devadula-nandan moved this from Backlog 🌋 to In progress in Carbon for IBM Products Jun 12, 2024
@devadula-nandan devadula-nandan moved this from In progress to Backlog 🌋 in Carbon for IBM Products Jun 14, 2024
@devadula-nandan devadula-nandan removed their assignment Jun 14, 2024
@elycheea elycheea moved this from Backlog 🌋 to In progress in Carbon for IBM Products Sep 5, 2024
@elycheea elycheea changed the title [Datagrid]: Column cusomization vs getAutoSizedColumnWidth [Datagrid]: Column customization vs getAutoSizedColumnWidth Sep 5, 2024
@devadula-nandan
Copy link
Contributor

devadula-nandan commented Sep 9, 2024

Hi @BenceLovas,

As you said for columns, the useMemo hook depends on data, which is why columns is re-calculated whenever data changes and the datagridState is being re-calculated, re-applying the initialState.
so having initialState outside of the component, passing its reference in datagridState, and syncing it after the datagridState gets calculated, looks like a stable approach to me.

you can check it here

will try to update the docs to intimate this usage.

@BenceLovas
Copy link
Author

Hi @devadula-nandan ,

In my opinion a library should hide the implementation details from users, and especially not ask them to add those themselves.
I think it's a bug within the library which should be fixed by the team who develops it. What you suggest is a workaround, which you cannot even guarantee to always work in the future.

I appreciate the help, and your solution in fact works, but I hope there is a way to actually make it work without those tricks.

@ljcarot
Copy link
Member

ljcarot commented Sep 10, 2024

@BenceLovas We always first try to triage an issue with options for how to resolve the problem as @devadula-nandan has done for you. We then weigh the cost v benefit for every effort to consider and decide if it is something we can add without making a breaking change. It appears this would require a breaking change to the datagrid component, so we recommend proceeding with the solution recommended.

@ljcarot ljcarot closed this as completed Sep 10, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done 🚀 in Carbon for IBM Products Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
role: dev Sev 2 Aspects of design is broken and impedes users in a significant way, but has workaround. type: bug 🐛 Something isn't working
Projects
Archived in project
Development

No branches or pull requests

4 participants