Ajouter la copie du permalien dans le menu dropdown des messages

[Migwel]

Fix #6155

Quelques remarques : je suis encore assez débutant point de vue front-end donc je suis plus qu'ouvert aux améliorations que vous pourriez me faire remarquer.
Par souci de simplicité et parce que je trouve que ça colle pas trop mal, j'ai réutilisé l'icône d'import pour cette copie de lien. Dites-moi si vous pensez que c'est bon ou si on devrait créer / importer une nouvelle icône.

Contrôle qualité

Permalien d'un message du forum

• Importer un jeu de données comportant des messages sur le forum
• Se rendre sur un topic comportant plusieurs messages
• Sélectionner un message de ce topic, cliquer sur le menu déroulant (en haut à droite)
• Cliquer sur "Copier le permalien". Ceci devrait fermer le menu déroulant
• Ouvrir un nouvel onglet et copier le lien dans la barre de navigation et s'y rendre
• Ceci devrait vous amener vers le message dont vous avez copié le permalien

Permalien d'un message dans un MP

• Importer un jeu de données comportant des messages privés
• Se rendre sur une conversation comportant plusieurs messages
• Sélectionner un message de cette conversation, cliquer sur le menu déroulant (en haut à droite)
• Cliquer sur "Copier le permalien". Ceci devrait fermer le menu déroulant
• Ouvrir un nouvel onglet et copier le lien dans la barre de navigation et s'y rendre
• Ceci devrait vous amener vers le message dont vous avez copié le permalien

Faire ce test sur plusieurs navigateurs pour confirmer que ça fonctionne bien partout.

[Migwel]

Ceci fonctionne mais je ne sais pas si c'est la façon la plus propre de fermer la dropdown ? Je n'ai pas trouvé d'autre exemple dans le code faisant ceci

[coveralls]

coverage: 88.413%. remained the same when pulling be5c21c on Migwel:issue6155 into 7297871 on zestedesavoir:dev.

[philippemilink]

Je n'ai pas encore testé, mais il y a déjà pas mal de choses qui me gênent dans le code :

• Pourquoi avoir déplacé la fonction JS pour marquer un message comme utile dans un autre fichier ? Cette fonction fait un appel Ajax, donc elle bien dans le fichier où elle est. Si tu veux déplacer toutes les fonctions liées à la gestion d'un message dans un fichier topic-message.js, il y en a plus à déplacer de ajax-actions.js. En effet, il n'y a pas de fichier JS qui se prête bien pour accueillir la fonction que tu veux rajouter. Moi je ferais un nouveau fichier message-permalink.js avec seulement cette nouvelle fonction.
• L'attribut HTML data-ajax-input de la balise <a> est mal nommé : on ne fait pas d'ajax ici. Je ferais d'une pierre deux coups avec <a data-copy-permalink="{{ request...}}..." ...>. Ensuite, dans le JS tu dois pouvoir faire $('.topic-message').on('click', "[data-copy-permalink='*']", (voire même seulement "[data-copy-permalink]" mais je ne suis plus sûr...)
• Pourquoi ne pas afficher le bouton dans les MPs ? Ça peut toujours être utile de l'avoir aussi dans les MP, pour mentionner dans la conversation un ancien message de la conversation (surtout qu'on peut toujours le faire manuellement en prenant le lien de la date du message).
• Concernant ta question pour dropdown.removeAttribute('open'), je n'ai pas vraiment de réponse précise, mais j'ai remarqué qu'on utilise .hide() ici, ce qui me semble plus élégant que de supprimer l'attribut open :)
• Le linter JavaScript n'est pas content : regarde dans les fichiers modifiés par la PR sur l'interface GitHub (que ça ne fasse pas échouer la CI est un bug connu : Les erreurs du linter front ne font pas échouer la CI #6277). En local, tu peux exécuter make lint-front et make format-front.

[Arnaud-D]

C'est bizarre ce que tu as fais-là. Je ne sais pas comment tu as fais ton rebase, mais tu as les commits récents de dev en plein milieu des tiens, ce qui est gênant à terme pour la fusion. ^^

[Migwel]

C'est bizarre ce que tu as fais-là. Je ne sais pas comment tu as fais ton rebase, mais tu as les commits récents de dev en plein milieu des tiens, ce qui est gênant à terme pour la fusion. ^^

Oui, je viens de remarquer. J'ai effectivement fait mon rebase dans le mauvais sens, je vais régler ça

[Migwel]

@philippemilink J'ai corrigé toutes tes remarques je pense, à l'exception du hide. J'ai essayé de faire ça avec hide() mais le problème est que ça cache la dropdown à tout jamais 😅 Pour le autocomplete où on l'utilise ce n'est pas un problème mais ici, ça l'est évidemment

[AmauryCarrade]

Hello!

hide() ne marchera pas ici car ce menu est un élément <details>. La fonction hide() de jQuery applique essentiellement un display: none dessus, or l'affichage du contenu d'un élément <details> ne fonctionne pas ainsi, et est, en effet, pilotée par l'attribut open (ou le clic de l'utilisateurice).

L'exemple donné par Philippe, où hide() est utilisé, fonctionne car c'est un cas où le menu (d’autocomplétion) est un élément classique qu’il faut masquer en CSS, et non un <details>.

Changer l'attribut HTML est une méthode qui fonctionne, et ce n'est pas une mauvaise pratique. On peut également utiliser l'interface JavaScript (HTMLDetailsElement), en modifiant l'attribut JS open.

// Ne fonctionne que si `e.target.closest` retourne bien un objet
// JavaScript natif ou assimilable à un HTMLDetailsElement ; j’ai
// un doute sur le fait que ça marche avec un objet jQuery.
const dropdown = e.target.closest(".dropdown")
dropdown.open = false

[AmauryCarrade]

On est sur une quête de limitation de jQuery — voir #5786, #6046, et plein de PRs. Il serait bien que les nouvelles PR n'en intègrent pas.

Ce code est assez simple à écrire en VanillaJS, avec la même verbosité. N'hésite pas à demander, si besoin d'un coup de main là dessus :) .

[Arnaud-D]

@Migwel tu as prévu de continuer cette PR ? C'est pour une question de suivi, si tu ne comptes pas t'y remettre, on la marquera comme "En attente de reprise".