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

✨ feat: add youtube tracking remover #247

Merged
merged 10 commits into from
Nov 2, 2023
Merged

Conversation

Aeris1One
Copy link
Collaborator

@Aeris1One Aeris1One commented Oct 31, 2023

Ce plugin remplace les messages envoyés et contenant un lien YouTube avec des paramètres

image

Fonctionne sur activation de l'option de config serveur enable_youtube_tracking_remover

Le bouton "?" affiche un petit texte explicatif localisé :
image
image

@ascpial
Copy link
Contributor

ascpial commented Oct 31, 2023

Ça serait bien avec un opt-out qui permet de choisir entre l'option "edit my messages (you won't have control on the message after)", "warn me" et "do nothing"
par défaut c'est sur warn me, et ça envoie un message quand tu envoie un lien qui est supprimé rapidement après, avec un bouton avec une roue dentée pour changer le paramètre.

@Aeris1One Aeris1One force-pushed the youtube_tracking_remover branch from 7ba2d1f to 87fc9f9 Compare October 31, 2023 20:50
@Aeris1One
Copy link
Collaborator Author

Je ne pense pas que ça doive être une décision par utilisateur, mais par serveur (parce qu'un utilisateur peut ne pas être géné par le tracking alors que d'autres personnes sur le serveur si).
Je n'ai pas implémenté d'option "avertir uniquement" parce que je ne voyais pas vraiment comment faire, on ne peux pas faire d'ephemeral sans interaction en premier lieu de l'utilisateur, et ça ne force pas l'utilisateur à retirer les liens de tracking, on en reviens donc au point d'avant, où les personnes habituellement génées par le tracking font déjà attention aux paramètres dans les messages qu'elles envoient, et les personnes qui n'en ont rien à faire ignoreront l'avertissement. Il me semble donc pas mal inutile.

Une option pourrait être d'envoyer un second message "Voici les liens YouTube sans tracking" sans modifier l'original.

@ascpial
Copy link
Contributor

ascpial commented Oct 31, 2023

Je ne parlait pas d'un message éphémère dans le sens des interactions Discord mais tout simplement d'un message qui est supprimé au bout disons d'une minute.
Envoyé un message d'alerte comme tu l'as indiqué, c'est à dire avec le lien sans tracking pourrait être une autre manière de le faire. De plus, il doit être possible de retirer l'embed du message original pour inciter à moins cliquer et à laisser l'embed sur le message d'alerte.

En tous cas, je penses qu'il faut laisser à l'utilisateur la possibilité de ne pas voir ses messages envoyés via un webhook et ne plus avoir de contrôle dessus.

@Aeris1One
Copy link
Collaborator Author

image
J'ai ajouté une fonctionnalité permettant de supprimer son message durant les 5 minutes après son envoi. Je ne vais pas implémenter la modification (c'est faisable avec un bouton qui ouvre un modal) parce qu'il suffit à l'utilisateur de supprimer puis renvoyer son message.

La durée de 5 minutes est un bon milieu entre ne pas pouvoir supprimer du tout et avoir un bouton suppression inutile qui reste à l'infini une fois que la personne a décidé de ne pas supprimer son message. Le bouton "?" disparait aussi après 5 minutes car il devient plus ou moins inutile ensuite, il est surtout utile pour la personne qui a vu son message se faire remplacer ou les autres qui étaient connectées à ce moment-là.

Si tu souhaites implémenter une autre fonctionnalité, comme les messages d'avertissement, n'hésite pas à ouvrir une autre PR. Celle-ci pourra être merge quand le code aura été validé.

@Aeris1One Aeris1One self-assigned this Oct 31, 2023
@Aeris1One Aeris1One requested a review from a team October 31, 2023 21:33
@ZRunner
Copy link
Contributor

ZRunner commented Nov 2, 2023

L'émoji ❓ rouge sur fond vert rend horrible imo, et les boutons verts signifient généralement une action de confirmation. Je verrai plus un bouton gris voire bleu pour ce genre d'indication, avec un émoji plus neutre en couleur.

Pour la fonctionnalité d'édition de message, je suis d'accord avec ascpial : autant sur mobile on peut facilement copier-coller le message du webhook pour le modifier, autant sur desktop le copier-coller ne reprend pas le markdown donc c'est relou. Une option de juste avertir sans forcément supprimer (et éventuellement avec un bouton pour faire le remplacement par le bot au cas par cas) me semble nécessaire.

Au niveau du traitement des paramètres d'URL, je vois que tu as séparé les cas d'utilisation de youtube.com et youtu.be, il y a une raison ? J'ai l'impression que c'est grosso-modo le même traitement mais je n'ai pas encore regardé dans les détails.

@Aeris1One
Copy link
Collaborator Author

Les liens en youtube.com utilisent la syntaxe youtube.com/watch?v=[videoid] alors que les liens youtu.be utilisent youtu.be/[videoid] directement. Dans le premier cas l'identifiant de la vidéo est un paramètre v alors que dans le second il est directement inclus dans le path. C'est le même traitement pour rajouter les paramètres time et t à l'URL ensuite cela dit.

Pour le bouton je peux le passer en gris, mais j'ai l'impression que ça fait encore plus bizarre :
image

@ZRunner
Copy link
Contributor

ZRunner commented Nov 2, 2023

Pour le bouton je peux le passer en gris, mais j'ai l'impression que ça fait encore plus bizarre
Et le ❔ (en gris donc) sur bouton bleu, ça donnerait quoi ?

@Aeris1One Aeris1One force-pushed the youtube_tracking_remover branch from 477bf68 to 0c1dceb Compare November 2, 2023 16:52
@Aeris1One Aeris1One force-pushed the youtube_tracking_remover branch from 0c1dceb to eb94c87 Compare November 2, 2023 16:53
Copy link
Contributor

@ZRunner ZRunner left a comment

Choose a reason for hiding this comment

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

LGTM

@Aeris1One Aeris1One merged commit 0d965c5 into beta Nov 2, 2023
3 checks passed
@Aeris1One Aeris1One deleted the youtube_tracking_remover branch November 2, 2023 16:56
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.

3 participants