Ajouter la copie du permalien dans le menu dropdown des messages #6506
Conversation
assets/js/topic-message.js
Outdated
| e.preventDefault() | ||
| navigator.clipboard.writeText($(this).attr("data-message-url")); | ||
| const dropdown = e.target.closest(".dropdown") | ||
| dropdown.removeAttribute('open') |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 deajax-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 fichiermessage-permalink.jsavec seulement cette nouvelle fonction. - L'attribut HTML
data-ajax-inputde 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'attributopen:) - 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-frontetmake format-front.
|
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 |
Oui, je viens de remarquer. J'ai effectivement fait mon rebase dans le mauvais sens, je vais régler ça |
|
@philippemilink J'ai corrigé toutes tes remarques je pense, à l'exception du hide. J'ai essayé de faire ça avec |
|
Hello!
L'exemple donné par Philippe, où 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 ( // 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 |
There was a problem hiding this comment.
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 :) .
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
Permalien d'un message dans un MP
Faire ce test sur plusieurs navigateurs pour confirmer que ça fonctionne bien partout.