-
Notifications
You must be signed in to change notification settings - Fork 19
The user can freely choose statuses an item heal by using the multiselect component #484
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
Conversation
Improve checkbox to get an indeterminate status Remove None and ALL front status Heal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello !
J'ai pu tester cette branche avec le MultiSelect et ça fonctionne bien, bien joué ! 😄
L'idée d'adapter le bloc de données sur la vue lecture avec les éléments sélectionnés est une bonne idée. Il faudra qu'on soit vigilant à ne pas le faire partout car si la liste comprend 20 éléments ça peut vite être problématique.
Je me permets quelques remarques pour améliorer le rendu et le fonctionnel :
- Sur le Select de base, on peut utiliser les flèches HAUT / BAS pour parcourir la liste, et ENTRÉE pour valider. Dans le cas du MultiSelect, on s'attendrait à pouvoir faire de même, avec ENTRÉE qui coche la Checkbox sans refermer l'input. On peut choisir d'ignorer cette partie mais je pense que pour des raisons d'accessibilité c'est important.
- Quand la liste déroulante est affichée sous l'input, on s'attend à ce qu'en cliquant sur l'input elle soit fermée à nouveau. Est-ce qu'on ajoute ce comportement ?
- Si on se fie au Figma, l'état d'un élément de la liste dont la checkbox est cochée affiche bien un fond plus clair, mais seulement si on est en hover avec la souris. Dans le cas où la souris n'est pas dessus, on ne devrait pas avoir le fond plus clair. Je mets une capture pour plus de clarté.
- D'après les discussions qu'on avait eu avec Walven, et les éléments retenus sur Discord ensemble, le comportement pour l'input devrait être le suivant :
- L'input s'adapte à la sélection en concaténant les valeurs de la façon suivante :
Valeur 1, Valeur 2, Valeur 3
- L'input affiche 3 lignes max. Donc si le texte concaténé dans l'input dépasse 3 lignes, on affiche un text-overflow: ellipsis "...".
- Ce qui permet au final d'afficher quelque chose de plus explicite que "8 sélectionnés".
- L'input s'adapte à la sélection en concaténant les valeurs de la façon suivante :
- L'icône de la Checkbox "-" n'est pas centrée, dans le cas où une partie des éléments de la liste est sélectionnée.
Je vous laisse réagir à ce retour, notamment @Walven, comme ça on évite les allers retours trop conséquents et on se met d'accord avant corrections / ajouts.
Bien joué encore ! 🎉
Just wanted to say I tested this and it works at properly assigning multiple statuses to the already existing array "statusList"! |
…d styling - Updated button click handlers to prevent event propagation. - Replaced MultiSelectInput with MultiSelectTextArea for improved user experience. - Adjusted MultiSelectContainer styles for better layout and responsiveness. - Implemented keyboard navigation and selection highlighting in RenderOptions. - Enhanced input resizing logic for better display of selected values. - Cleaned up unused code and improved overall structure for maintainability.
…tudio into feat-400-multiselect-heal-status
… option in multiple languages
J'ai énormément retravaillé le composant, il est dans l'état, très différent de sa première version. Sinon, j'ai pris en compte tes nombreux retours Aerun, normalement, tu devrais être plus content de cette version. Thanks for testing previously, but we need to test it again after my last review 😇 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello ! 😄
J'ai pris le temps de tester ta dernière proposition, et c'est vraiment chouette, bien joué !
J'ai quelques petits détails à soulever tout de même pour qu'on soit carrés.
-
Quand on sélectionne suffisamment d'éléments pour que l'input affiche 3 lignes, l'espace entre le bas de l'input et le texte semble trop petit vis à vis de l'espace du dessus.
-
Si je vais sur l'objet Antidote et que je sélectionne tous les statuts de la liste, tout est bon au niveau des checkboxes. Idem si je choisis "Tout sélectionner" à la souris. Par contre, si j'utilise le clavier (HAUT / BAS puis ENTRÉE) sur "Tout sélectionner", l'icône de Checkbox est sélectionnée mais l'icône de check n'est pas visible (cf. la capture d'écran).
-
Quand je sélectionne certains statuts, que je sors de la modale d'édition, et que je reclique sur le bloc "Soin" pour modifier la liste à nouveau, l'input semble afficher une troisième ligne vide. Je ne sais pas si c'est le même symptôme qu'on avait à un moment, mais il manque peut-être un refresh à un moment. Peut-être attendre une review de code avant d'attaquer ce problème ?
…tionality - Increased max-height of MultiSelectTextArea for better usability. - Introduced determineCheckboxState function for clearer checkbox state management. - Enhanced input resizing logic to accommodate new height adjustments. - Added label truncation for long selections in getSelectDefaultLabel function. - Updated Checkbox styles to handle indeterminate state visibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'ai pu rester, et on est tout bon pour moi, let's goooo ! 🥳
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quand on clique sur la scrollbar, le select se ferme alors que ça ne devrait pas être le cas.
Donc on ne peut pas utiliser la scrollbar.
…mprove MultiSelectTextArea props - Added mouse event handling to prevent default behavior in RenderOptions for better interaction. - Removed unnecessary `tabIndex` from MultiSelectTextArea to streamline props usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C'est bon pour moi GG :party
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ca marche nickel 👌
Juste un petit détail, est-ce qu'on mettrai pas 'tous les status' quand ils sont tous selectionnés au lieu de faire une liste ?
Si effectivement on peut faire ça. Ça éviterait de devoir ouvrir le Select pour vérifier qu'on a tous les statuts de sélectionnés. |
… and tooltip support - Added 'All statuses' translation to English and French language files. - Updated MultiSelect component to accept a tooltip for the 'select all' option. - Enhanced getSelectDefaultLabel function to return a custom label when all options are selected.
…tudio into feat-400-multiselect-heal-status
Thank you for your contribution to the Pokémon Studio repo.
Before submitting this PR into the develop branch, please make sure:
Description
Note before testing
Multiselect is a derived of Select, some functionnality of Select has be taken. Bug too certainly
Tests to perform
#400