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

[WIP] Hide video player when video is in encoding #893

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

CartierPierre
Copy link
Contributor

Des utilisateurs se plaignent que le player video leur indique un message d'erreur quand la vidéo est en cours d'encodage. J'ai masqué le player lorsque la vidéo est en encodage.

@@ -184,7 +184,11 @@ <h4 class="accordion-header theme_title" id="theme_desc_title">
{% if form %}
{% include 'videos/video-form.html' %}
{% else %}
{% include 'videos/video-element.html' %}
{% if video.get_encoding_step == "" or video.encoding_in_progress %}
Copy link
Contributor

Choose a reason for hiding this comment

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

il y a la propriété encoded qui pourrait egalement servir : https://github.com/EsupPortail/Esup-Pod/blob/master/pod/video/models.py#L1000
Et, ne faudrait-il pas egalement ne pas charger les différents scripts utilisé par le lecteur ? (pour eviter les pb js)

Copy link
Contributor

Choose a reason for hiding this comment

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

pourquoi ne pas faire un simple display : none ? (je réfléchi tout haut !)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oui c'est possible aussi, je suis pas très bon coté JS, j'ai préféré intervenir directement dans le template. Est-ce pertinent de charger le player et de ne pas l'afficher pour les petites connexions ?

@ptitloup
Copy link
Contributor

Et il est possible de modifier la destination d'une PR sans la fermer et devoir en recréer une autre. Tu as un bouton d'edition à côté du titre. :-)

@Badatos
Copy link
Collaborator

Badatos commented Jun 19, 2023

Il me semble dommage de désactiver complètement le lecteur lorsque la vidéo n'est pas totalement encodée.
En effet, il arrive souvent que les premiers encodages en faible résolution soient rapides, et permettent déja de visualiser la vidéo avant que tous les encodages en plus haute résolution soient terminés.
Est-il possible de ne masquer le player que s'il n'y a absolument aucun des encodages de faits ?

Copy link
Collaborator

@LoicBonavent LoicBonavent left a comment

Choose a reason for hiding this comment

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

C'est vrai que c'est beaucoup mieux qu'avant où le message d'erreur Javascript pouvait être problématique.
Ma seule remarque concerne les erreurs Javascript qui en découlent dans la console. Peut-être qu'un display: none ferait l'affaire en fait.

@CartierPierre
Copy link
Contributor Author

Et il est possible de modifier la destination d'une PR sans la fermer et devoir en recréer une autre. Tu as un bouton d'edition à côté du titre. :-)

Oui mais j'avais rename la branche dans mon fork, du coup la PR marchait plus :/

@CartierPierre CartierPierre changed the title [DONE] Hide video player when video is in encoding [WIP] Hide video player when video is in encoding Jun 19, 2023
@CartierPierre
Copy link
Contributor Author

Ok je vais voir si on peut légèrement améliorer, je suis d'accord @Badatos
Si c'est plutôt coté JS qu'il faut intervenir, pour le coup je veux bien passer la main, je ne maitrise pas bcp. M'enfin vous avez l'idée générale du problème

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.

4 participants