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

ETQ administrateur, je peux deplacer un champ via un select #9861

Merged
merged 8 commits into from
Jan 15, 2024

Conversation

mfo
Copy link
Contributor

@mfo mfo commented Dec 15, 2023

closes #9611

TODO

  • deplacer un champ ds les champs root (sans overlap sur les repetitions) 🎉
  • deplacer un champ ds une repetition (sans overlap sur les champs root) 🎉
  • ajouter la position des champs dans l'ui
  • pour deplacer les champs via un select, celui ci est dynamique (maj quand on ajoute, supprime, renomme, deplace un champ)
  • on vire le drag & drop 🗑️

en image (attention, il reste le handle du drag & drop, je l'ai viré, cf: #9611)

au début on a un select vide

Screenshot 2024-01-12 at 9 51 53 AM

au select:focus, on le seed via le app/javascript/controllers/select_champ_position_template_controller.ts

Screenshot 2024-01-12 at 9 51 58 AM

au select:focus out, si on a pas changé la cible, on reste sur le select seedé

Screenshot 2024-01-12 at 9 52 04 AM

@mfo mfo marked this pull request as draft December 15, 2023 09:13
@mfo mfo force-pushed the US/move-type-de-champ-with-select branch 3 times, most recently from f0cd49f to d15b469 Compare December 18, 2023 08:33
…ction to move and morph fields between two coordinates
@mfo mfo force-pushed the US/move-type-de-champ-with-select branch 3 times, most recently from 738e16a to 50e6ee7 Compare January 3, 2024 10:32
@mfo mfo force-pushed the US/move-type-de-champ-with-select branch 3 times, most recently from 19fc6b5 to 73a2292 Compare January 11, 2024 11:07
@mfo mfo changed the title wip: select pour positionner un champs ETQ administrateur, je peux deplacer un champ via un select Jan 11, 2024
@mfo mfo force-pushed the US/move-type-de-champ-with-select branch from 73a2292 to 85c85af Compare January 11, 2024 14:56
@mfo mfo force-pushed the US/move-type-de-champ-with-select branch from 85c85af to b3cd139 Compare January 11, 2024 15:01
@mfo mfo marked this pull request as ready for review January 11, 2024 15:05
@mfo mfo force-pushed the US/move-type-de-champ-with-select branch from b3cd139 to e7eee7b Compare January 11, 2024 15:12
@LeSim
Copy link
Member

LeSim commented Jan 11, 2024

Alors, déjà bravo, je pense qu'un nombre infini d'admin graveront ton nom en lettre d'or pour cette fonctionnalité.

Juste des petites remarques UX en première lecture :

je m'attendais a ce que cette fonctionnalité de déplacement se situe avec les autres options : dans le cadre du champs, a droite des flèches
est ce que tu pourrais retirer l'option de déplacement vers le champ lui même ?
au niveau du wording, peut être "déplacer ce champ à la place de "

@LeSim
Copy link
Member

LeSim commented Jan 11, 2024

Du coup, si je pousse niveau perf en utilisant la fonctionnalité sur la démarche du fond vert (69279) 263 champs, j'ai ça :

load initial de la page : on semble avoir un n+1 sur un watermak, mais on peut voir plus tard

Screenshot 2024-01-11 at 16-49-40 demarches-simplifiees fr

déplacement d un premier champ tout en bas du formulaire :

Screenshot 2024-01-11 at 16-50-06 demarches-simplifiees fr

Ca vaudrait peut être le coup de regarder ?

@colinux
Copy link
Member

colinux commented Jan 11, 2024

Enorme bigup sur la feature !

  • je m'attendais a ce que cette fonctionnalité de déplacement se situe avec les autres options : dans le cadre du champs, a droite des flèches

même remarque. Une idée évoquée avec @lisa-durand sur cette barre c'était de déplacer la barre d'icônes/actions en bas de la zone du champ (au lieu d'être collée au-dessus du champ, elle devient collée par en dessous).

@mfo
Copy link
Contributor Author

mfo commented Jan 11, 2024

@LeSim tx (dsl j'avais edité ta réponse pour répondre 🤦 ),

Mes réponses :

  • je m'attendais a ce que cette fonctionnalité de déplacement se situe avec les autres options : dans le cadre du champs, a droite des flèches

https://www.figma.com/file/6iUaraN3yfDoJIJoSSNZPn?node-id=0:1#595105090 – niveau UX, semblerait que se soit mieux (sens d'olivier & lisa). Je préfère laisser à d'autres le fait de challenger le truc. L'UI proposé est la : #9611

  • est ce que tu pourrais retirer l'option de déplacement vers le champ lui même ?

Pas certains que se soit une bonne idée, je m'explique : c'est ce qui permet de se retrouver a la bonne place dans la liste des champs [ex, t'es au champs 50 sur 100, tu as ainsi 50 options au dessus, 50 options en dessous, car le focus des options est sur le champ actuel] ? Après je peux essayer de le mettre en disabled/le renommer...

  • au niveau du wording, peut être "déplacer ce champ à la place de "

yeap, d'ailleurs je pense que le wording suivant est plus parlant : "déplacer ce champ après" (a la place de me fait penser a un swap)

Du coup, si je pousse niveau perf en utilisant la fonctionnalité sur la démarche du fond vert (69279) 263 champs, j'ai ça :

load initial de la page : on semble avoir un n+1 sur un watermak, mais on peut voir plus tard

Screenshot 2024-01-11 at 16-49-40 demarches-simplifiees fr

déplacement d un premier champ tout en bas du formulaire :

Screenshot 2024-01-11 at 16-50-06 demarches-simplifiees fr

Ca vaudrait peut être le coup de regarder ?

Je pense que c'est le renumber – donc p-e que l'ordering par float serait une réponse ? (merci @tchak)

@mfo
Copy link
Contributor Author

mfo commented Jan 11, 2024

Enorme bigup sur la feature !

  • je m'attendais a ce que cette fonctionnalité de déplacement se situe avec les autres options : dans le cadre du champs, a droite des flèches

même remarque. Une idée évoquée avec @lisa-durand sur cette barre c'était de déplacer la barre d'icônes/actions en bas de la zone du champ (au lieu d'être collée au-dessus du champ, elle devient collée par en dessous).

sur cette barre, je vous laisse vous decider avec @lisa-durand

@colinux
Copy link
Member

colinux commented Jan 11, 2024

dessous).

Oui l'emplacement de cette barre fera l'objet d'une autre PR si on la bouge. Mais autant d'avoir tout le texte "Déplacer…" dans un select comme palceholder ça me choquait moins que ce soit entre les 2 champs (car c'est comparable à 2 boutons, même si l'un est un select), autant là d'avoir le libellé à part (à gauche) du select et hors dsfr je trouve pas ça clair (ou alors le screenshot est pas à jour ?)

@mfo
Copy link
Contributor Author

mfo commented Jan 11, 2024

dessous).

Oui l'emplacement de cette barre fera l'objet d'une autre PR si on la bouge. Mais autant d'avoir tout le texte "Déplacer…" dans un select comme palceholder ça me choquait moins que ce soit entre les 2 champs (car c'est comparable à 2 boutons, même si l'un est un select), autant là d'avoir le libellé à part (à gauche) du select et hors dsfr je trouve pas ça clair (ou alors le screenshot est pas à jour ?)

dans les ecrans de #9611, il y a les deux options. je suis parti sur l'option avec un label pour des questions d'a11y/standard, en gros, un control est précédé d'un libelle. v2 ?

@mfo mfo force-pushed the US/move-type-de-champ-with-select branch from f9766f9 to 1404a1e Compare January 12, 2024 09:04
@colinux
Copy link
Member

colinux commented Jan 12, 2024

dessous).

Oui l'emplacement de cette barre fera l'objet d'une autre PR si on la bouge. Mais autant d'avoir tout le texte "Déplacer…" dans un select comme palceholder ça me choquait moins que ce soit entre les 2 champs (car c'est comparable à 2 boutons, même si l'un est un select), autant là d'avoir le libellé à part (à gauche) du select et hors dsfr je trouve pas ça clair (ou alors le screenshot est pas à jour ?)

dans les ecrans de #9611, il y a les deux options. je suis parti sur l'option avec un label pour des questions d'a11y/standard, en gros, un control est précédé d'un libelle. v2 ?

oui ça me va :) merci !

@mfo mfo force-pushed the US/move-type-de-champ-with-select branch from 1404a1e to dd9f944 Compare January 12, 2024 09:36
@mfo mfo force-pushed the US/move-type-de-champ-with-select branch from dd9f944 to 68e9d35 Compare January 12, 2024 10:00
@@ -0,0 +1,3 @@
= form_with(url: move_and_morph_admin_procedure_type_de_champ_path(@coordinate.revision.procedure, @coordinate.type_de_champ.stable_id), class: 'fr-ml-3w flex', method: :patch, data: { turbo: true }) do |f|
= label_tag :target_stable_id, "Déplacer ce champ à la place de ", for: describedby_id, class: 'flex align-center flex-no-shrink fr-mr-3w'
Copy link
Contributor

Choose a reason for hiding this comment

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

Est-ce qu'on garde ce wording ou on met plutot "Déplacer ce champ après" car la on dirait qu'on le remplace et ça amène un peu de confusion je trouve.

Copy link
Member

Choose a reason for hiding this comment

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

ah oui ça m'avait échappé dans le refacto, je suis d'accord

Copy link
Contributor Author

@mfo mfo Jan 15, 2024

Choose a reason for hiding this comment

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

deplacé apres ne marche pas dans le sens bas vers le haut. c'est bien deplacer ce champ à la place de (a l'inverse du feedback que j'ai donné a lesim). si vous trouvez un autre wording ca serait top. mais de haut en bas, le 1er champ arrive apres le dernier champ. a l'inverse le dernier champ vient remplacer le 1er champ

@mfo mfo added this pull request to the merge queue Jan 15, 2024
Merged via the queue into main with commit 0328446 Jan 15, 2024
15 checks passed
@mfo mfo deleted the US/move-type-de-champ-with-select branch January 15, 2024 13:19
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.

[admin] ETQ Admin je peux déplacer les champs dans le formulaire n'importe ou dans la page
4 participants