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

Fix datagrid configurable columns management #10339

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

djhi
Copy link
Collaborator

@djhi djhi commented Nov 8, 2024

Supersedes #10157 because I couldn't push on the PR branch

Problem

When you change the columns order of a ConfigurableDatagrid, or just if you remove a column and add another one, this is not managed and the column management is broken : aka the displayed columns in the configure list does not control the corresponding columns.

Here is a quick illustration of the issue : https://www.loom.com/share/c5a6afaac36a4111bedf08ece818da17 (in french, sorry)

That's because currently, only a change in the number of columns trigger and update. And even in this case, you may have unexpected results (like the column enabled to be displayed are not the same as before).

Remaining issues

  • We currently store the indexes of the columns. If the columns change dynamically through code, hiding/showing columns will not target the correct ones.
  • We still display columns that have no source (such as when adding a button) as Unlabeled column #XXX in both the SelectColumnsButton and DatagridEditor

Solution

This changes bring :

  • better detection : now, if you change the source of a column, add or remove a column (or both), it triggers the update
  • keeps your previous settings : it keeps the columns you displayed before
  • keeps your order : it keeps the columns displayed in the order you have set
  • enable non omited new columns : if you added new columns, and not omited them, they will be displayed at the end

How To Test

  • There are stories and tests

Additional Checks

  • The PR targets master for a bugfix, or next for a feature
  • The PR includes unit tests (if not possible, describe why) : not new feature. tests still pass
  • The PR includes one or several stories (if not possible, describe why) : not a new feature. stories are still relevant
  • The documentation is up to date : no change needed to the documentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work In Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants