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

[DONE] edit file js #824

Merged
merged 28 commits into from
May 11, 2023
Merged

Conversation

thinhantran
Copy link
Contributor

bonjour,
je trouves quelques fonctions dans file "main/js" qui n'utilise pas ailleurs. Je ne sais pas que je peux les enlever ?

@ptitloup ptitloup changed the title edit file js [WIP] edit file js May 3, 2023
@thinhantran
Copy link
Contributor Author

Je n'ai pas trouvé l'autre code en redouble et inutile dans les fichiers javascript. Vous pouvez faire une review ?

@thinhantran thinhantran changed the title [WIP] edit file js [DONE] edit file js May 5, 2023
@@ -70,22 +70,18 @@ document.addEventListener("click", (e) => {
form.style.display = "block";
});
show_form("");
document.getElementById("fileModal_id_file")?.remove();
document.getElementById("fileModal_id_file")?.remove(); // il y a pas id "fileModal_id_file"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pourrais-tu mettre les commentaires en anglais.

span.innerHTML = " " + parseInt(event.target.value).toHHMMSS();
});
});
// il n'y a pas id "id_time_start" et "id_time_end"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pourrais-tu mettre les commentaires en anglais.

const formClasses = ["form_new", "form_change", "form_modif", "form_delete"];
formClasses.forEach((formClass) => {
document.querySelectorAll(`form.${formClass}`).forEach((form) => {
form.style.display = "none";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pourquoi, ne pas remettre la condition suivante : if (form) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

j'ai pensé il a pas besoin. Mais c'est mieux s'il y a donc je vais modifier

#: pod/video/templates/videos/add_video.html:87
#: pod/video/templates/videos/video_edit.html:217
msgid "Help for form fields"
msgstr "Aide pour les champs de formulaire"
#: pod/video/templates/videos/category_modal.html:16
Copy link
Collaborator

Choose a reason for hiding this comment

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

Il manque un petit saut de ligne :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm après verifier sur mon ordinateur, il y a saut de ligne

@@ -1,3 +1,4 @@
// cette fonction (isFooterInView) n'est pas utilisée ailleurs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pourrais-tu mettre les commentaires en anglais.

@@ -1,3 +1,4 @@
// cette fonction n'est pas utilisée ailleurs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pourrais-tu mettre les commentaires en anglais.

@@ -1257,7 +1242,7 @@ var showalert = function (message, alerttype) {
formalertdiv?.remove();
}, 5000);
};

// cette fonction (show_messages) n'est pas utilisée ailleurs, il n'y a pas id "show_messages"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pourrais-tu mettre les commentaires en anglais.

@AymericJak
Copy link
Collaborator

Testé en local, fonctionnel.

Vérifie bien les tests unitaires :)

pod/chapter/static/js/chapters.js Outdated Show resolved Hide resolved
pod/chapter/static/js/chapters.js Outdated Show resolved Hide resolved
pod/chapter/static/js/chapters.js Show resolved Hide resolved
pod/chapter/static/js/chapters.js Outdated Show resolved Hide resolved
pod/enrichment/static/js/enrichment.js Show resolved Hide resolved
pod/enrichment/static/js/enrichment.js Outdated Show resolved Hide resolved
@mattbild
Copy link
Collaborator

Quand j'ajoute un chapitre puis que je le supprime, j'ai l'erreur vous n'êtes plus authentifié (alors que je le suis).
Le chapitre est bien supprimé en revanche

@thinhantran
Copy link
Contributor Author

je corrige et repousse

@mattbild
Copy link
Collaborator

ok, ça fonctionne maintenant.
Cela dit j'aurais groupé les tests sur l'action

    const data = await response.text();
    if (action === "new" || action === "modify") {
      if (data.indexOf(id_form) === -1) {
        showalert(
          gettext("You are no longer authenticated. Please log in again."),
          "alert-danger"
        );
        return;
      }
      show_form(data);
    } else if (action === "delete") {
      const parsed_data = JSON.parse(data);

@thinhantran
Copy link
Contributor Author

thinhantran commented May 11, 2023

il y a encore "elt.classList.add("info");"
dans
else if (action === "modify") {
show_form(data);
elt.classList.add("info");
Donc je ne sais pas c'est possible pour regouper comme ca

@mattbild
Copy link
Collaborator

mattbild commented May 11, 2023

il y a encore "elt.classList.add("info");"

ok bien vu

Copy link
Collaborator

@mattbild mattbild left a comment

Choose a reason for hiding this comment

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

après les différents changements, les modifs me paraissent correctes
.

@ptitloup ptitloup merged commit 62235bc into EsupPortail:develop May 11, 2023
4 checks passed
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.

None yet

5 participants