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

Retire JQuery de content-helps.js #6116

Merged
merged 9 commits into from
Jul 1, 2021

Conversation

artragis
Copy link
Member

@artragis artragis commented May 8, 2021

But de la PR

Ajouter les primitives ajax (/!\ fetch n'est pas supporté par IE) fix #6046
Retire jquery de content-helps.js en tant que démonstrateur

QA

  • make build-front
  • Créer un tuto et jouer avec les bouton d'ajouts et de retraits des demandes d'aide.

@coveralls
Copy link

coveralls commented May 8, 2021

Coverage Status

Coverage remained the same at 86.689% when pulling 6191302 on artragis:remove_some_jquery into 3b26ab5 on zestedesavoir:dev.

@firm1
Copy link
Contributor

firm1 commented May 16, 2021

Rapport de QA

QA KO

Je poste une vidéo pour montrer ce que j'observe, car ce n'est pas simple a expliquer.

cut.mp4

Quand je joue avec les boutons, en cliquant dessus (sur les boutons d'aide), le bouton change d'état et revient à son état d'avant genre 3-5 secondes après, ce qui laisse penser que je n'ai pas cliqué dessus. Sauf que si je refresh ma page, je vois bien que l'état a bien changé.

PS: je n'ai aucun message d'erreur dans ma console JS pendant le clic sur les boutons.

Mon navigateur : Brave (basé sur Chromium: 81.0.4044.138 ) et ça le fait aussi avec Firefox 88

@firm1
Copy link
Contributor

firm1 commented May 17, 2021

Je ne sais pas si le commit est censé corrigé mon bug, mais après re-QA, j'ai toujours le même comportement que dans la vidéo.

@artragis
Copy link
Member Author

même après build et refresh du cache? car là chez moi j'ai vraiment plus le bug.

le soucis venait du fait que la requête fetch n'était pas vue comme une requête ajax du coup il y avait un 302 qui était vu comme une erreur.

@firm1
Copy link
Contributor

firm1 commented May 17, 2021

Je confirme, c'était un problème de cache chez moi.

Du coup QA OK

@artragis artragis requested a review from Situphen May 17, 2021 14:02
Copy link
Member

@Situphen Situphen left a comment

Choose a reason for hiding this comment

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

Voici la revue demandée ! Je m'excuse pour l'attente. Le code me parait bien et ça fonctionne très bien donc c'est bon à ce niveau. Est-ce qu'il serait possible cependant de mettre un exemple dans la documentation pour l'usage de window.ajax ? Quelque chose comme :

const form = document.querySelector('.monFormulaire')
const data = new FormData(form)
ajax.post(form.getAttribute('action'), data,
  (result) => maFonctionEnCasDeSucces(result.une_valeur_retournee_par_le_serveur),
  () => maFonctionEnCasDEchec()
)

Afin de faciliter la migration du code JS du site pour ne plus utiliser JQuery,
nous avons créé un helper ajax qui vous permettra de manipuler rapidement les
requêtes vers les fonctionnalités du site qui prennent un formulaire ou du json en entrée
et retourne un json en réponse.
Copy link
Member

Choose a reason for hiding this comment

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

Pluriel car accord avec "les fonctionnalités du site"

Suggested change
et retourne un json en réponse.
et retournent un json en réponse.

requêtes vers les fonctionnalités du site qui prennent un formulaire ou du json en entrée
et retourne un json en réponse.

Cet objet est défini dans ``assets/js/commons/ZDSAjax.js``. Il vous permet d'appeler nativement
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Cet objet est défini dans ``assets/js/commons/ZDSAjax.js``. Il vous permet d'appeler nativement
Cet objet est défini dans ``assets/js/common/ZDSAjax.js``. Il vous permet d'appeler nativement

Cet objet est défini dans ``assets/js/commons/ZDSAjax.js``. Il vous permet d'appeler nativement
les méthodes get/post/put et sera vu par django comme un appel ajax.
Vous pourrez à chaque fois définir une méthode à appeler en cas de succès de la **communication**
(c'est-à-dire si la requête a peu être envoyée au serveur et obtenir un retour au format json, peu importe son code de retour)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(c'est-à-dire si la requête a peu être envoyée au serveur et obtenir un retour au format json, peu importe son code de retour)
(c'est-à-dire si la requête a pu être envoyée au serveur et obtenir un retour au format json, peu importe son code de retour)


$helpButton.parent().find('input[name="activated"]').val((!state).toString())
}
// helpButton.blur()
Copy link
Member

Choose a reason for hiding this comment

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

Commentaire à supprimer

Suggested change
// helpButton.blur()

@artragis
Copy link
Member Author

j'ai donc amélioré la doc, merci pour ton retour.

Copy link
Member

@Situphen Situphen left a comment

Choose a reason for hiding this comment

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

Encore deux petits points et normalement c'est bon !

doc/source/front-end/helpers-js.rst Outdated Show resolved Hide resolved
doc/source/front-end/helpers-js.rst Outdated Show resolved Hide resolved
artragis and others added 2 commits July 1, 2021 17:51
Co-authored-by: Situphen <Situphen@users.noreply.github.com>
Co-authored-by: Situphen <Situphen@users.noreply.github.com>
@Situphen Situphen enabled auto-merge (squash) July 1, 2021 18:06
@Situphen Situphen merged commit 20b81c1 into zestedesavoir:dev Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Drop de JQuery phase 2: Industrialisation - Gestion de Ajax
4 participants