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

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

Merged

Conversation

AlejandroSuero
Copy link
Contributor

@AlejandroSuero AlejandroSuero commented Apr 5, 2024

Descripción

Soluciono un bug que introduje en #849

Problema solucionado

Closes #865.

Cambios propuestos

  1. Añadir autoplay en la etiqueta video.

Capturas de pantalla (si corresponde)

Chrome: (No problem)

fix-video-background.mp4

Safari: (Problem)

video-problem-in-safari.mp4

@midudev sabes por qué puede estar pasando esto?

Comprobación de cambios

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

Impacto potencial

Contexto adicional

Enlaces útiles

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

@AlejandroSuero AlejandroSuero changed the title fix(BackgroundVideo): not playing when changing combat fix(#865): not playing when changing combat Apr 5, 2024
Copy link

vercel bot commented Apr 5, 2024

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

A member of the Team first needs to authorize it.

@AlejandroSuero AlejandroSuero changed the title fix(#865): not playing when changing combat [WIP] fix(#865): not playing when changing combat Apr 5, 2024
@lucas-labs
Copy link

@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
Copy link

@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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link

lucas-labs commented Apr 5, 2024

😒 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
Copy link
Contributor Author

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

  • localhost:
Screenshot 2024-04-05 at 22 11 57

Carga inicial

Screenshot 2024-04-05 at 22 11 38

Segunda carga

  • lavelada.es:
Screenshot 2024-04-05 at 22 12 22

Carga inicial

Screenshot 2024-04-05 at 22 12 38

Segunda carga

@AlejandroSuero
Copy link
Contributor Author

😒 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
Copy link

lucas-labs commented Apr 5, 2024

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
Copy link
Contributor Author

@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
Copy link
Contributor

yurigo commented Apr 5, 2024

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
Copy link
Contributor Author

AlejandroSuero commented Apr 5, 2024

El último commit soluciona:

  • Safari
    • iOS
  • Chrome
  • 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 inicial de /combates/[id]

Carga recargando /combates/[id]
Carga recagando /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
Copy link
Contributor

yurigo commented Apr 5, 2024

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...

* add src on video tag for safari support

* refactor url

* remove transitions from combats to combat to fix video loading for firefox

* remove script in backgroundvideo
@AlejandroSuero
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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
Copy link
Contributor

yurigo commented Apr 6, 2024

@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>

src/lib/browser.ts Outdated Show resolved Hide resolved
@AlejandroSuero AlejandroSuero changed the title [WIP] fix(#865): not playing when changing combat fix(#865): BackgroundVideo.astro not playing when changing combat Apr 6, 2024
@byMagg
Copy link

byMagg commented Apr 11, 2024

A mi me pasa en Chrome

Combates.-.La.Velada.del.Ano.IV.-.Google.Chrome.2024-04-11.20-10-14.mp4

@AlejandroSuero
Copy link
Contributor Author

@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
Copy link
Owner

midudev commented Apr 12, 2024

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
Copy link
Contributor Author

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

check-firefox-96060e9.mp4

@yurigo
Copy link
Contributor

yurigo commented Apr 12, 2024

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 midudev merged commit 702544f into midudev:main May 17, 2024
1 of 3 checks passed
@AlejandroSuero AlejandroSuero deleted the feature/fix-background-video-playing branch May 18, 2024 07:58
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.

Video background en sección combate solo se reproduce la primera vez
6 participants