Produits Entr'ouvert » Godo

(c'est dans le périmètre de financement initial, c'est actuellement contourné en ayant une passe de transformation des URL en liens côté serveur)

Ça avance bien.
Je pensais y être mais j'ai encore 2 soucis, qui sont peut-être liés (si ça dit à quelqu'un de regarder) :
Quand j'ajoute un italique ou un strong dans un lien, la markup est flat :

<a> debut du lien </a> <strong><a> suite du lien </a></strong>

à la place de

<a> debut du lien <strong> suite du lien </strong></a>

et le copier/coller des liens ne fonctionne pas.
(Et j'ai oublié les trads)

Aujourd'hui je sèche, je vais reprendre à tête reposé.

Sinon, encore un gros patch

Il intègre le dialog pour saisir/éditer les attributs d'un element. Pourra être utile pour les images, ajouter des class ou des options, etc.
Techniquement j'ai choisi de partir sur l'API dialog qui commence à être bien supporté et à corriger ses problèmes d'a11y.
J'ajoute un polyfill (une nouvelle dépendance npm, désolé) provisoirement le temps que le support soit ok côté firefox ESR.

Ajout une méthode pour créer / éditer / supprimer les liens. C'est vraiment pas simple du point de vue ergo. J'ai regardé par mal d'éditeur et ils fonctionnent tous différemment.
J'ai décidé pour le moment

• CRÉER un lien si la selection n'en possède pas
• ÉDITER un lien si la selection en possède 1, que la selection soit partiel ou plus large. Il est possible de supprimer le lien depuis le dialog d'edition.
• Supprimer liens si la selection en possède plusieurs.

La branch est à jour pour les testeurs.

J'ai tapé make build dans la branche ça m'a tapé

npm audit
# npm audit report

terser  <4.8.1
Severity: high
Terser insecure use of regular expressions before v4.8.1 and v5.14.2 leads to ReDoS - https://github.com/advisories/GHSA-4wf5-vphf-c2xc
fix available via `npm audit fix`
node_modules/terser

1 high severity vulnerability

To address all issues, run:
  npm audit fix
make: *** [Makefile:14 : build] Erreur 1

Après avoir fait ça, ça a buildé, j'ai ouvert un navigateur sur les fichiers dans dist/ ça a affiché "Uncaught ReferenceError: dom is not defined" dans la console.

_~~

Bref pas pu tester du tout mais j'étais parti pour un commentaire notant que les boites de dialogue c'était assez dépassé, qu'on pouvait peut-être imaginer un affichage inline/flottant (mais peut-être la CSS donne déjà ce type de rendu).

OK, premier patch.

Concernant le nesting des inlines (marks), prosemirror demande un ordre de nesting. Je propose `a > em > bold`.
Voilà pourquoi lorsque

Quand j'ajoute un italique ou un strong dans un lien, le markup est flat.

et le copier/coller des liens ne fonctionne pas.

Uncaught ReferenceError: dom is not defined

Même erreur. Un problème dans la définition de la fonction parseDOM des liens. Réglé.

j'étais parti pour un commentaire notant que les boites de dialogue c'était assez dépassé

Je ne m'occupe pas de l'affichage de l'element `dialog` dans ce ticket, il a l'affichage par défaut. Ce ticket est déjà costaud.
J'aimerais un focus sur l'ui de la création, l'édition et la suppresion de liens. En sachant que j'ai décidé de faire cela à travers 1 seul bouton.
Je suis ouvert à la discussion, si possible dans un nouveau ticket.

J'ai ajouté les liens dans l'éditeur basic, ils vont donc être disponible partout, et donc dans le champ wcs commentaire. Me dire si c'est une mauvaise idée.

J'ai poussé des choses sur la branche.

• Retirer le l'état global pour le dialogue. A mon sens c'est mieux de le recréer à chaque fois, c'est plus concis et moins sujet à oublier de restaurer des états qui traînent.
• Changer la classe TextField en une méthode. Le constructeur renvoie un DomElement : elle n'est pas vraiment utilisée. Je propose d'utiliser un gabarit aussi, je trouve ça plus facile à lire.
• Déstructurer les champs des options de la méthode text_field, pour correspondre à ce qui est fait sur open_dialog (et c'est plus concis aussi).
• Retirer un paramètre inutilisé 'required' sur l'appel à text_field.
• Bidouiller un peu le code qui vérifie s'il y a des hrefs différents pour le rendre plus concis et clair (à mon sens tout du moins). On pourrait simplifier encore en considérant qu'on ne peut avoir qu'une mark de type link sur un node mais je ne sais pas si c'est le cas.

• Bug d'autofocus : normalement le dialogue met le focus automatiquement soit sur le premier élément avec le tag autofocus, soit sur le premier input qu'il trouve quand il est ouvert. Hors, l'appel à showDialog est fait sur un mouseDown, le focus est mis à ce moment là correctement, puis le dialogue reçoit un mouseUp qui lui fait mettre le focus sur l'overlay du dialogue. En écoutant mouseUp sur les boutons Godo, on évite ce problème (et on colle plus au comportement des boutons en général, qui permettent "d'annuler" un clic en bougeant la souris en dehors du bouton en gardant le bouton enfoncé).
• Ajouter une marge entre les boutons : mieux ou pas, c'est subjectif.

• La popup c'est parfaitement utilisable.
• Éventuellement, lorsque je sélectionne un lien plus du texte en dehors, quand je valide, je m'attendrai à ce que toute la sélection passe en lien, pas uniquement que ça change l'adresse du lien existant. S'il faut en discuter ça peut faire un autre ticket, en l'état ça fonctionne très bien.
• Ça me paraîtrait bien de faire apparaître le bouton "lien" même sans sélectionner de texte, juste en cliquant sur un lien, pour pouvoir en changer l'adresse sans le sélectionner.
• Il y a un bug un peu emmerdant à régler : si on définit un lien qui englobe un texte en gras et un texte normal, puis qu'on sélectionne uniquement le texte en gras et qu'on lui assigne une autre adresse, on se retrouve avec deux liens différents, coupés au niveau du strong. C'est très marginal comme cas, donc pareil : ça pourra faire l'objet d'un autre ticket.

Retirer l'état global pour le dialogue. A mon sens c'est mieux de le recréer à chaque fois, c'est plus concis et moins sujet à oublier de restaurer des états qui traînent.

Il n'y a pas d'état global. L'état est scopé au module.
Dans ta proposition, tu recrées de 0 et injectes le dialog dans le DOM à chaque ouverture du dialog et tu lui assignes les events à chaque fois, avec les reflows et repaints que cela peut provoquer.
L'idée d'injecter qu'une fois est pour limiter les manipulations DOM.

Changer la classe TextField

L'idée d'utiliser une class est d'avoir un pattern souple, de pas avoir besoin de refactoriser par la suite, pouvoir ajouter des méthodes, utiliser l'héritage pour les besoins des nouveaux champs.

Je propose d'utiliser un gabarit aussi, je trouve ça plus facile à lire.

OK

Retirer un paramètre inutilisé 'required' sur l'appel à text_field.

Arf, oubli de ma part, required est prévu pour le field href.

Bidouiller un peu le code qui vérifie s'il y a des hrefs différents pour le rendre plus concis et clair

Je prends

On pourrait simplifier encore en considérant qu'on ne peut avoir qu'une mark de type link sur un node mais je ne sais pas si c'est le cas.

Non, prosemirror a un modèle de document plat https://prosemirror.net/docs/guide#doc.structure

Bug d'autofocus

Cool

Ajouter une marge entre les boutons : mieux ou pas, c'est subjectif.

Mieux je pense aussi. J'ai pas posé de marges parce qu'on va entrer en conflit avec gadjo et publik-base-theme, et j'ai pas encore arbitré sur la meilleure voie (surcharge côté portails ou pas). Le problème est plus coton côté publik-base-theme, qui demandera très certainement des surcouches de son côté.

Éventuellement, lorsque je sélectionne un lien plus du texte en dehors, quand je valide, je m'attendrai à ce que toute la sélection passe en lien, pas uniquement que ça change l'adresse du lien existant. S'il faut en discuter ça peut faire un autre ticket, en l'état ça fonctionne très bien.

J'ai opté pour cela, un usager parkinson s'il sélectionne trop ou pas assez doit pouvoir éditer le lien existant. J'ai pensé aussi à retourner le label du lien dans le dialog (et le rendre éditable ?) comme ça l'usager sait qu'il édite le lien trouver et pas plus ni moins. Mais oui, discutons-en dans un autre ticket. Cela laissera le temps aux CPF de tester et d'avoir plus d'avis.

Ça me paraîtrait bien de faire apparaître le bouton "lien" même sans sélectionner de texte, juste en cliquant sur un lien, pour pouvoir en changer l'adresse sans le sélectionner.

Pour le moment le bouton lien est inclus dans le menu "marks" qui n'apparait qu'à la sélection (Ça me parait bien pour éviter le bug que tu pointes ensuite). J'ai pensé aussi permettre l'édition au clic sur un lien.

Il y a un bug un peu emmerdant à régler : si on définit un lien qui englobe un texte en gras et un texte normal,

Dût au modèle flat de prosemirror. Il faudrait pour cela inspecter les noeuds voisins au premier et dernier noeud du range de selection, si tous les noeuds du range sont des liens. Il faudrait que je teste aussi l'utilisation de l'api de selection native pour récupérer le lien en entier et voir s'il est possible de le transformer en select prosemirror

Thomas, j'ai squashé tout ça, rebasé sur main et corrigé les erreurs du linter sur la branche. Je te laisse jeter un oeil, je peux squasher les deux commits et merger si tu veux.

• effacer du code obsolète
• réactiver le statut "disabled" sur les items de type marks

Si ce ticket passe en recette, puis prod, il faut regarder si besoin de surcharger les CSS côté publik-base-thème pour la modale.

(mais attendons le prochain cycle pour pousser, pour avoir le temps de regarder ce qui devrait être fait dans publik-base-theme et peut-être gadjo)

côté gadjo aussi des styles nécessaires ?

Sur Gadjo, des améliorations sont possibles, mais out of the box je trouve ça bien.

via la capture d'Agate dans #70718 :

• utile d'avoir un titre "paramètres du lien" (je trouve que non)
• si utile, il y a actuellement un seul paramètre, donc ça serait "paramètre".

Aussi, si on garde un titre,

+        open_dialog({
+          title: "Paramètres du lien",

il faudrait qu'il soit traduit.

+              label: "Url",

Pour le libellé du champ, je préférerais "Link" (vu qu'on pourrait y taper #plop, ou mailto:plop, pas des URL), et permettre une traduction.

+              placeholder: "http://url.net",

Ailleurs on essaie de défendre l'inadéquation des placeholder, je serais pour ne pas nous contredire et ne rien mettre ici.

tile d'avoir un titre "paramètres du lien" (je trouve que non)

Avoir un titre explicite est necessaire d'un point de vue a11y. "Paramètre" n'apporte l'information de ce que l'on va modifier.
Dans cette première version, je ne voulais pas compliquer les choses avec un titre a11y et un autre titre. Faire au plus simple.

il y a actuellement un seul paramètre, donc ça serait "paramètre".

OK pour passer au singulier

pour le libellé du champ, je préférerais "Link". et permettre une traduction.

Top - ok

défendre l'inadéquation des placeholder

Ok

Est-ce qu'on passe comme ça dans cette release ?

(perso je n'aurai pas l'occasion de tester)

mais
1/ "Paramètre du Lien" : pas de majuscule à Lien;

et à l'inverse,

2/ label: getLanguage('a').text, ça va écrire "lien" comme libellé, sans majuscule.

Testé en local et :

 $ npm run build

> Godo@0.0.1 build
> rollup -c

./src-js/godo.js → dist/js/godo.js, dist/js/godo.min.js...
(!) Unresolved dependencies
https://rollupjs.org/guide/en/#warning-treating-module-as-external-dependency
dialog-polyfill (imported by src-js/godo-dialog.mjs)
created dist/js/godo.js, dist/js/godo.min.js in 6s

et ainsi en toute logique :

Uncaught TypeError: Erreur lors de la résolution du spécificateur de module « dialog-polyfill ». Les spécificateurs de module relatifs doivent commencer par « ./ », « ../ » ou « / ».

Il manque quelque chose, dans le code, ou dans le README sur la manière de compiler ?

npm install

pour installer les nouvelles dépendances

Automatic expiration