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

feat(Table): rows grouping #1003

Merged
merged 10 commits into from
Dec 9, 2024
Merged

feat(Table): rows grouping #1003

merged 10 commits into from
Dec 9, 2024

Conversation

savutsang
Copy link
Contributor

@savutsang savutsang commented Oct 11, 2024

Permettre d'afficher des rows sous des groups

  • We can now display multiple built-in columns (expand, checkbox, numbering) at the same time.
  • New prop expandChildsOnRowSelection to auto-expand childs rows on selection. It also follow the expandableRows settings (single or multiple).

Note: Les screenshots dans JIRA ont des background gris, mais c'est plutot blanche/transparent tel que dans Figma.

@savutsang savutsang requested a review from a team as a code owner October 11, 2024 13:59
@savutsang savutsang force-pushed the dev/DS-915 branch 3 times, most recently from 613185c to 12e6764 Compare October 11, 2024 14:25
Copy link

Storybook for this build: https://ds.equisoft.io/pr-1003/

Copy link

Webapp for this build: https://ds.equisoft.io/pr-1003/webapp/

@savutsang savutsang force-pushed the dev/DS-915 branch 2 times, most recently from 090a905 to 5c2120d Compare October 11, 2024 14:48
Copy link

@maboilard maboilard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Étrangement, les rows de section sont en état selected lorsque je clique sur le checkbox du column header. Les rows de section devraient cependant être indépendantes.
image

Copy link
Contributor

@LarryMatte LarryMatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quelques petits problèmes avec l'état des checkboxes.

  1. Si le parent est checked, ça sélectionne les enfants. Si tu un-check un enfant, le parent a les deux icons "-" & "✓" des states.
  2. Si tu un-check tous les enfants, le parent garde l'icon/état checked "✓"
  3. Si tu select tous les enfants, le parent n'a l'icon/état checked "✓"
DS-915_checkboxes-states.mov

Copy link
Contributor

@ogermain-kronos ogermain-kronos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je vais te laisser adresser les autres reviews mais LGTM!! 👍

@savutsang
Copy link
Contributor Author

savutsang commented Dec 6, 2024

Quelques petits problèmes avec l'état des checkboxes.

  1. Si le parent est checked, ça sélectionne les enfants. Si tu un-check un enfant, le parent a les deux icons "-" & "✓" des states.
  2. Si tu un-check tous les enfants, le parent garde l'icon/état checked "✓"
  3. Si tu select tous les enfants, le parent n'a l'icon/état checked "✓"

C'est corrige. Je l'ai refact, il y avait plein de bugs avec l'ancienne facon de faire.

LarryMatte
LarryMatte previously approved these changes Dec 6, 2024
Copy link
Contributor

@LarryMatte LarryMatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Attends l'approval de Max ou PY mais sinon la vie est belle pour moi.

Copy link

@maboilard maboilard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dans Figma, le children a une indentation sur la colonne de checkboxes et la première colonne de texte. Peut-être que c'est hors scope et qu'on peut arranger ça dans un autre ticket, mais je voulais quand même le mentionner.

image

@LarryMatte
Copy link
Contributor

Dans Figma, le children a une indentation sur la colonne de checkboxes et la première colonne de texte. Peut-être que c'est hors scope et qu'on peut arranger ça dans un autre ticket, mais je voulais quand même le mentionner.

image

Si c'est quelque chose qui peut être fixé rapidement, faisons là dans cette PR mais sinon, on le fera dans une autre PR étant donné que Oli en a besoin pour ce sur quoi il travaille présentement.

@ogermain-kronos
Copy link
Contributor

Notre maquette pour le tableau de doublons ne comporte pas cette indentation, donc la situation présente me va parfaitement.

@savutsang
Copy link
Contributor Author

savutsang commented Dec 9, 2024

Dans Figma, le children a une indentation sur la colonne de checkboxes et la première colonne de texte. Peut-être que c'est hors scope et qu'on peut arranger ça dans un autre ticket, mais je voulais quand même le mentionner.
image

Si c'est quelque chose qui peut être fixé rapidement, faisons là dans cette PR mais sinon, on le fera dans une autre PR étant donné que Oli en a besoin pour ce sur quoi il travaille présentement.

C'est un oublis de ma part mais c'est pas si simple de faire ce modificaton. Presentement le checkbox est dans une colonne a part, si on veut faire l'indentation, il faudrait que le checkbox et le texte soit dans la meme colonne. Bref un autre PR.

@ogermain-kronos
Copy link
Contributor

On a un souci de temps pour l'implémentation côté CRM. Je peux juste parler pour ma team, mais ça me va eeeeeen masse comme c'est là :)

Copy link

@maboilard maboilard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On pourra régler l'indentation dans un autre ticket. Beau boulot!

Copy link
Contributor

@pylafleur pylafleur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, on pourra itérer à partir de ça au besoin!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'ai migre au nouveau format storybook pour qu'on puisse jouer avec les controls, mais en realite ya juste le story Grouping qui a ete ajoute.

@savutsang savutsang merged commit ecbaaed into master Dec 9, 2024
22 checks passed
@savutsang savutsang deleted the dev/DS-915 branch December 9, 2024 21:57
LarryMatte pushed a commit that referenced this pull request Jan 9, 2025
* feat(Table): rows grouping

* fix: minor

* fix: remove temporary patch

* fix: remove unecessary fields

* fix: remove temporary patch for build

* fix: new version of grouping

* fix: comment test

* fix: proper checking

* fix: selection state

* fix: test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants