fix(#865): `BackgroundVideo.astro` not playing when changing combat

AlejandroSuero commented 7 months ago

Descripción

Soluciono un bug que introduje en #849

Problema solucionado

Closes #865.

Cambios propuestos

Añadir autoplay en la etiqueta video.

Capturas de pantalla (si corresponde)

Chrome: (No problem)

https://github.com/midudev/la-velada-web-oficial/assets/71392160/08a765e7-e60f-4db1-83de-f53aa8bf4cc3

Safari: (Problem)

https://github.com/midudev/la-velada-web-oficial/assets/71392160/52d37a6d-6d78-4e57-855a-502c1484ff65

@midudev sabes por qué puede estar pasando esto?

Comprobación de cambios

[X] He revisado que no haya ninguna PR (pull request) ya abierta con un problema similar, siguiendo el apartado de buenas prácticas
[X] He revisado localmente los cambios para asegurarme de que no haya errores ni problemas.
[x] He probado estos cambios en múltiples dispositivos y navegadores para asegurarme de que la landing page se vea y funcione correctamente.
[X] He actualizado la documentación, si corresponde.

Impacto potencial

Contexto adicional

Enlaces útiles

Documentación del proyecto:
Código de referencia:

vercel[bot] commented 7 months ago

@AlejandroSuero is attempting to deploy a commit to the midudev pro Team on Vercel.

A member of the Team first needs to authorize it.

lucas-labs commented 7 months ago

@AlejandroSuero Safari creo que no reproduce automáticamente ningún video si está en modo low power (o ahooro de energía). Será por eso que no te funciona en Safari?

lucas-labs commented 7 months ago

@AlejandroSuero Safari creo que no reproduce automáticamente ningún video si está en modo low power (o ahooro de energía). Será por eso que no te funciona en Safari?

Aunque ahora que lo pienso, si fuese ese el problema, tampoco debería reproducirse la primera vez...

AlejandroSuero commented 7 months ago

@lucas-labs no ese no es el problema, además me asegurado ahora por si acaso y he conectado también el cargador, pero sigue igual.

Además si recargo la página se reproduce igualmente.

Tampoco cargan las fuentes en local, por algún motivo.

AlejandroSuero commented 7 months ago

He notado que se reduzco el script a esto:

<script>
   import { $ } from "@/lib/dom-selector"

   const $bgVideoDiv = $("#bg-video")

  if ($bgVideoDiv) $bgVideoDiv.querySelector("video")?.play()
</script>

Tampoco hay diferencia en Chrome, pero en Safari sigue con el mismo problema.

lucas-labs commented 7 months ago

😒 Safari is the new IE6.

Estuve googleando un poco y por ahora la única solución que estoy viendo repetida en distintas fuentes es el .play()... pero ya se está haciendo en tu script.

No estoy tan familiarizado con Astro, pero podría llegar a ser que el script solo se esté ejecutando la primera vez? Quizá habría que supeditar el script a algún evento (asegurarse de que el script se ejecute nuevamente al cambiar de página).

AlejandroSuero commented 7 months ago

Creo que puede ser un problema de cómo cachea los recursos Safari:

localhost:

Carga inicial

Segunda carga

lavelada.es:

Carga inicial

Segunda carga

AlejandroSuero commented 7 months ago

😒 Safari is the new IE6.

Estuve googleando un poco y por ahora la única solución que estoy viendo repetida en distintas fuentes es el .play()... pero ya se está haciendo en tu script.

No estoy tan familiarizado con Astro, pero podría llegar a ser que el script solo se esté ejecutando la primera vez? Quizá habría que supeditar el script a algún evento (asegurarse de que el script se ejecute nuevamente al cambiar de página).

Esta es mi primera vez trabajando con Astro también así que no me sé muy bien que eventos habría que tarjetear para conseguirlo.

lucas-labs commented 7 months ago

import { $ } from "@/lib/dom-selector"

- const $bgVideoDiv = $("#bg-video")

const playVideo = () => {
+   const $bgVideoDiv = $("#bg-video")

    if ($bgVideoDiv) {
        let video = $bgVideoDiv.querySelector("video")

        if (video) {
            video.play()
        }
    }
}

document.addEventListener("astro:page-load", playVideo)

@AlejandroSuero podrías probar el script así? (yo estoy en situación de windows así que no tengo Safari 😅, por desgracia o por suerte)

Creo que lo que puede estar pasando es que en tu script actual, estás tomando la referencia del #bg-video por fuera de la función del evento.

En el script de arriba lo vuelvo a tomar cada vez que se dispare el evento.

Seguramente la referencia al video que estamos haciéndo .play() es al video anterior.

AlejandroSuero commented 7 months ago

@lucas-labs he probado a hacer eso pero sigue el mismo resultado 😥.

PD: yo también tengo Windows pero mi portátil de trabajo es un Mac, aunque prefiero Linux antes que Mac, menos restricciones xD.

yurigo commented 7 months ago

Hay varios problemas con el reproductor de video.

En chrome todo bien.
En firefox sólo se reproduce el primer video y luego nada
En safari sólo se reproduce el primer video y luego nada (muy parecido al caso de firefox)
En safari ios el video se reproduce en pantalla completa

El caso más facil es el de ios: video tiene un atributo playsinline que lo soluciona.

En safari se puede solucionar en lugar de trabajar con el src de source, trabajar con el src de video... (no me preguntes 🙈)

En firefox hay un problema con las transition view. Si se quiere que en firefox funcione hay que descartar la view transition de combates a combate

AlejandroSuero commented 7 months ago

El último commit soluciona:

[X] Safari
- [X] iOS
[X] Chrome
[X] Brave
[ ] Firefox

En relación con Firefox:

Si se recarga la página estando en /combates/[id] reproduce el vídeo.

Carga inicial sin recargar:

Carga recargando /combates/[id]

Si sabéis algo que pueda estar ocasionando este problema, me gustaría saber por qué es el único que decide como no fetchear los recursos.

yurigo commented 7 months ago

Firefox no está llevando muy bien el tema de view-transitions... He hecho una pr a tu fork con los cambios que he ido mencionando. Puedes mirar los cambios en cada commit para ir solucionando los problemas.

En el caso de firefox he eliminado los atributos de transition:name en ambas páginas combates/index.astro y combates/[id].astro para deshabilitar la view-transicion entre esas páginas.

De este modo, firefox hace todo el ciclo de load de la página y va a buscar el video...

AlejandroSuero commented 7 months ago

@yurigo ahora ya funciona, gracias.

Voy a ver si meto un poco de lógica para detectar si es firefox y añadir o no las view transitions y si no funciona o se rompe pues se queda así.

AlejandroSuero commented 7 months ago

@yurigo @lucas-labs creo que ahora ya funciona en todos, al menos lo he probado en: Firefox, Safari, Safari iOS, Google Chrome, Brave.

Si veis algo que falla, decídmelo.

yurigo commented 7 months ago

@AlejandroSuero Estaba explorando otro camino para solucionar esto de las view transitions para firefox sin necesidad de consultar si es firefox o no.

He hecho lo siguiente:

He hecho un revert del commit que eliminaba las transition:name por lo que

combates/index.astro
```
height={combatData.titleSize[1]}
```
transition:name={title-image-${combat.id}} loading={index < 2 ? "eager" : "lazy"}
combates/[id].astro
```
height={imageHeight}
```
transition:name={title-image-${id}} loading={"eager"}

Y después, en BackgroundVideo.astro he añadido lo siguiente:

<div class="fixed left-0 top-0 -z-10 aspect-video h-[100vh] w-screen" id="bg-video">
    <video
+       id="video-player"
        muted
        loop
        class="aspect-video size-full overflow-hidden object-cover opacity-0 transition-opacity duration-500"
        style="mask-image: linear-gradient(to bottom, rgba(0,0,0,1) 70%, transparent);"
        oncanplay="this.classList.add('opacity-70')"
        playsinline
        src={url}
    >
        <source type="video/mp4" src={url} />
    </video>
</div>

+<script is:inline>
+   document.getElementById("video-player").load()
+</script>

byMagg commented 7 months ago

A mi me pasa en Chrome

https://github.com/midudev/la-velada-web-oficial/assets/44727429/70499a95-85d7-4119-ac9a-c1be442fb8c2

AlejandroSuero commented 7 months ago

@byMagg en teoría si clonas mi repo, haces git switch feature/fix-background-video-playing, instalas las dependecias y lo corres en local, debería estar arreglado.

Cuando se mergee ya estará en la principal.

midudev commented 7 months ago

El código de Firefox no puede funcionar tal y como está puesto ya que ese JavaScript se ejecutaría a nivel de servidor o build time y no en el cliente.

Igualmente lo demás tiene buena pinta así que lo voy a mergear y en todo caso quitaré ese código de Firefox.

Si al final el vídeo no funciona en Firefox, tampoco es grave, es una característica que estaría bien tener pero tampoco rompe la página.

AlejandroSuero commented 7 months ago

@midudev ya he quitado el código de Firefox en 96060e9 y he comprobado que funcionase.

https://github.com/midudev/la-velada-web-oficial/assets/71392160/da68d152-9eba-4dff-9f9f-ecfbfe614b6f

yurigo commented 7 months ago

En el último commit (el que elimina el código de detección de firefox) has eliminado de /combates/index.astro la línea de transition:name={`title-image-${combat.id}`}. Los exploradores que sí hacen view-transitions pierden esa feature.

midudev / la-velada-web-oficial