feat(tally): integrate tally poll #2715
Conversation
| @@ -7,7 +7,9 @@ export default class extends Controller { | |||
| if (this.hasSubmitTarget) { | |||
| this.toggleSubmit() | |||
| } | |||
| this.#updateSelectedCountText() | |||
| if (this.hasSelectedUsersCounterTarget) { | |||
There was a problem hiding this comment.
J'ai une erreur au moment de passer aprés la création des usagers, au moment de passer aux invitations quand je clic sur le CTA.
There was a problem hiding this comment.
Ah bah justement ce changement était pour éviter cette erreur 🤔, mais l'erreur ne vient pas de cette branche
There was a problem hiding this comment.
ce controller passe à la trappe dans #2574 mais j'ai l'impression que c'est pas lié à la fonctionnalité ici de toute façon
There was a problem hiding this comment.
Non effectivement 👍
There was a problem hiding this comment.
Merci @Michaelvilleneuve 👍 , je pense qu'il y aura quelques petits changements à faire suite aux derniers changements sur les csp et dans #2574
| @@ -7,7 +7,9 @@ export default class extends Controller { | |||
| if (this.hasSubmitTarget) { | |||
| this.toggleSubmit() | |||
| } | |||
| this.#updateSelectedCountText() | |||
| if (this.hasSelectedUsersCounterTarget) { | |||
There was a problem hiding this comment.
ce controller passe à la trappe dans #2574 mais j'ai l'impression que c'est pas lié à la fonctionnalité ici de toute façon
| @@ -54,7 +54,7 @@ | |||
| </div> | |||
| <div> | |||
| <% if @all_invitations_attempted %> | |||
| <%= link_to "Terminer", @user_list_upload.structure_users_path, class: "btn btn-primary d-block mx-auto", data: { turbo_frame: "_top" } %> | |||
| <%= link_to "Terminer", @user_list_upload.structure_users_path, class: "btn btn-primary d-block mx-auto", data: { turbo_frame: "_top", controller: "tally", "tally-form-id": "nWjZGj", "tally-delay-in-ms": 1000, action: "click->tally#showPopup" } %> | |||
There was a problem hiding this comment.
Il vaudrait pas mieux mettre l'id dans une variable d'env ? Ça permettrait de l'activer qu'en prod d'ailleurs
There was a problem hiding this comment.
Alors je l'ai pas mis dans une variable d'env car c'est vraiment lié à ce formulaire en particulier pas à Tally au global, et je me disais qu'on pourrait potentiellement en avoir 3-4 en même temps, rendant un peu pénible l'usage d'une variable d'env.
Mais dans les faits on en aura surement qu'un ou 2 donc why not
There was a problem hiding this comment.
Pour le coup je pense que ce n'est pas une bonne idée de le laisser en dur, ne serait-ce que pour ne pouvoir l'afficher dans tous les env. On veut pas l'avoir en staging et en demo non ?
| if (!document.querySelector("script#tally-script")) { | ||
| const script = document.createElement("script"); | ||
| script.src = "https://tally.so/widgets/embed.js"; | ||
| script.id = "tally-script" | ||
| script.onload = resolve | ||
| document.head.appendChild(script); | ||
| } else { |
There was a problem hiding this comment.
je sais pas si ça va passer avec les nouveaux csp, mais on peut pas le loader sur la page avant au pire ?
There was a problem hiding this comment.
Je trouve ça plutôt bien de justement le loader au besoin depuis là. Mais je pense qu'on doit pouvoir juste faire une exception non ?
There was a problem hiding this comment.
Si y a pas de souci niveau csp ça me va de le laisser là
| }, this.element.dataset.tallyDelayInMs || 0) | ||
| } | ||
|
|
||
| set #hasAlreadyAnswered(value) { |
There was a problem hiding this comment.
Il me semblait qu'on avait dit qu'on évitait les getter/setter dans le js. On peut peut-être juste renommer cette méthode (par ex setHasAlreadyAnswered)
There was a problem hiding this comment.
Ah bon ? Pourquoi ?
There was a problem hiding this comment.
On s'était dit ça pour le model User.js, j'ai retrouvé l'historique :
- Enlever les getter/setter du model
Applicantdans l'app js #1225 - feat(upload): Add batch actions for creation and invitation #1212 (comment)
- tech(refacto): remove useless getter and setter in User.jsx #1942
Après je n'ai pas d'avis tranché je disais surtout ça par homogénéisation, ça ne me dérange pas de laisser comme ça
Cette PR enlève les relicats de code Maze pour ajouter Tally à la place et l'activer lors d'un click sur "Terminer et revenir à l'accueil" ainsi que "Terminer" après envoi des invitations.
J'ai ajouté un système de délai afin d'éviter un affichage immédiat et afin d'éviter que la mise à jour turbo écrase le contenu ajouté par Tally.
Tally ne gère pas lui même la logique d'affichage ou non une fois que le formulaire a été vu, j'ai donc rajouté de la logique pour conditionner cela.
À noter : J'ai fait un petit fix sur submit_selected_ids_controller pour éviter une erreur JS
Fix #2699