Améliorations du Editeur de Types de Champ

[tchak]

Cette PR est une réécriture complète de l'éditeur en React.

La manière dont l'éditeur est implémenté à beaucoup changer au fil de l'exploration. La première idée c'était de simplement générer le form html avec du JS dynamique est de le sauver en totalité. Après avoir essayé cette approche je me suis rendu compte que bien que simple conceptuellement elle introduit beaucoup de "edge cases". J'ai donc créé une simple api JSON pour pouvoir enregistrer les changements champ par champ. J'ai pris la décision d'utiliser Vue à l'origine à fin d'essayer de réutiliser les helps de Rails pour construire le formulaire. Cette réutilisation n'avait plus de sens avec le nouveau mode des sauvegardes champ par champ.

En même temps à la fin du cycle de développement ma frustration avec Vue ne cessait de croitre. Mon verdict en tant que "professionnel du JS" c'est que Vue c'est un plat de spaghetti créé par des personnes qui n'aiment pas le JS :) La goute d'eau qui a fait débordé le vase c'était le plugin de drag&drop qui avait trop de problèmes.

Dès le début du projet, je pensais utiliser React et la seule raison qui m'en a empêché c'était la tentative de réutilisation des templates haml. L'intégration dans Rails de React est bien meilleure que Vue. L'écosystème de React est plus riche. Et qu'on l'aime ou pas, mais pour paraphraser "Nobody got fired for using React" :)

Je crois très sincèrement que les prochaines personnes arrivant sur le projet selon bien contentes d'avoir une techno pour laquelle il est très facile de trouver de la doc, des exemples et un écosystème de qualité.

Dérnir changement majeur dans cette PR c’est l’introduction de la queue des opérations. Ce qui va éviter les bugs avec le mouvement des champs.

[tchak]

• test new types_de_champ#move api
• fix css for the footer
• fix css for arrow buttons

[kemenaran]

@tchak merci pour la description de la PR, et du contexte qui a mené à ça ❤️

[tchak]

@kemenaran j'ai mis quelques todo que je veux finir, mais ça devrait être bon pour review

[kemenaran]

J'ai regardé ce que ça donnait.

Remarques générales

• Le glisser-déposer des champs fonctionne bien mieux qu'avant 🙌
• J'ai l'impression que font-awesome-svg ne package que les SVGs qu'on utilise, et c'est bien cool 🙌

Sur l'usabilité

J'ai bien vu qu'il y avait encore des bricoles d'usabilité que tu voulais arranger. Je liste quand même ce qui m'est venu en tête :

• Je pense que ça se tiendrait de regrouper le contrôle de drag-drop et les flèches haut/bas au même endroit (par exemple sur la gauche).
• En cliquant sur une flèche haut ou bas, j'ai du mal à comprendre que le champ a bougé : la transition est instantanée et peu visible. Je pense que ça mériterait un effet (animation ou highlight), et sans doute de décaler le viewport en même temps (pour que le champ déplacé reste au même endroit, et qu'on puisse cliquer plusieurs fois de suite sur la même flèche).
• Le bouton "Ajouter un champ" n'est pas clair : si on a scrollé en haut, on ne voit pas le champ rajouté en bas. On pourrait scroller en bas – ou mieux, ajouter le champ dans le viewport.

Globalement l'éditeur me semble fonctionner – mais en état actuel de la PR il n'est pas non plus utilisable.

(Plus généralement, j'ai l'impression qu'on a souvent du mal sur DS à arriver au point où une fonctionnalité non seulement existe, mais est réellement utilisable sans gros problèmes d'ergonomie… Ça me questionne un peu sur nos méthodes de développement – mais c'est un autre sujet.)

Sur la technique

Avertissement : je ne suis pas un grand expert de React. Je connais un p'tit peu, mais sans plus.

Le code m'a l'air correct, et ça a l'air de bien fonctionner (mis à part les problèmes d'usabilité mentionnés).

Par ailleurs l'éditeur devient un peu complexe, avec des edge-cases à gérer – et la séparation en composants, même si elle est verbeuse, a l'air de bien rendre compte de ça.

En revanche, en lisant le code, l'utilisation de React m'inquiète :

• On passe de ~400 LOC de Vue à ~1000 LOC de React.

  Et en lisant le code, j'ai souvent une impression de beaucoup de plomberie, ou de "beaucoup de bruit pour pas grand chose". Plein de déclarations, de dispatch, de dynamisme. C'est pas facile à suivre, je trouve.

  Ça fonctionne sans doute si on connait bien React – mais j'ai l'impression qu'il y a des libs certes moins avancées, mais plus directes à comprendre.

• L'API de React a l'air de changer régulièrement.

  Je m'inquiète de la maintenance, au sens "la lib a été mise à jour, et faut ré-écrire la moitié du composant" (contrairement au sens "on veut rajouter des features").

  Cela dit Vue 3 a l'air de changer d'API aussi. Mais je me demande si ça ne plaide pas pour une bibliothèque de rendu plus minimaliste.

• L'implémentation actuelle tire aussi Memo et les Reducers, et :

  • Niveau développement je m'y connais trop mal pour être à l'aise avec ça. Par exemple je ne comprends rien au fichier hooks.js.
  • Ça tire une plus grande surface d'API que juste React – donc des cas plus nombreux de "L'API change et il faut ré-écrire".
  • Vu le nombre d'actions qu'on fait, j'ai une impression naïve qu'on pourrait couper dans la plomberie. Que du "Data-Down, Actions-Up" à un seul niveau, sans dispatch dynamique, avec un composant top-level qui envoie des requêtes sur la queue, et basta.

Questions ouvertes

Quand on utilise Rails, on tire plein de magie – mais en échange on a un code ultra-concis.

Ici j'ai l'impression qu'utiliser une bibliothèque riche (React) fait presque plus de code que de tout écrire en vanilla-JS.

• Est-ce qu'on ne gagnerait pas en simplicité à virer l'utilisation des reducers et compagnie, et à se servir de React juste comme une bibliothèque de rendu ?
• Est-ce qu'on ne gagnerait pas en maintenabilité à utiliser une bibliothèque de rendu moins blindée de features, plus réduite en périmètre, mais qui évoluerait moins vite (genre preact, ou autre) ?

Vous en pensez quoi ?

[kemenaran]

Y'a beaucoup de ifs dans cette méthode. J'ai l'impression que c'est essentiellement lié à :

• la distinction champs publics / champs privés
• les champs répétables

Tu crois que ça serait pertinent de faire plutôt deux ou trois endpoints différents ? Du genre :

• move_type_de_champ
• move_type_de_champ_private
• move_type_de_champ_repetition

Ou p-ê en condensant public et private, et en ne gardant qu'une distinction avec les répétition ?

[tchak]

Oui, je pensais refactored cette méthode en même temps que d'ajouter les tests

[kemenaran]

J'imagine que c'est ce qu'on peut faire de mieux…

Ça reste la matrice de l'angoisse, et je pense toujours qu'on loupe un truc, mais je ne vois pas comment faire mieux. Ça me va.

[tchak]

up @kemenaran

[kemenaran]

• Le scroll à l'ajout d'un nouveau champ est cool ; au moins on voit où on est :)
• C'est toujours difficile de bouger les champs avec les flèches (vu que le champ bouge instantanément).
• Dans l'ensemble j'ai l'impression que les champs sont difficiles à distinguer les uns des autres. Mais ça fait sans doute partie d'une passe d'améliorations stylistiques et d'usabilités qu'on peut faire après.

En l'état je suis chaud pour merger cette PR – et continuer sur les améliorations d'usabilité après.

[kemenaran]

J'imagine que c'est ce qu'on peut faire de mieux…

Ça reste la matrice de l'angoisse, et je pense toujours qu'on loupe un truc, mais je ne vois pas comment faire mieux. Ça me va.