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: Add media failed animation #89

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

Conversation

erdnaxe
Copy link
Contributor

@erdnaxe erdnaxe commented May 6, 2021

Proposal for #87.

Some notes:

  • I propose to put the animation inside a SVG element to make it easier to tweak in the future. This also prevent the need for more HTML elements.
  • This PR is not working as the loading animation is put in background (behind the video stream). To put an image on top of a video, it seems that we need to add another element as <video> does not seem to support ::before CSS modifier.

Is it problematic if we add an element next to <video> to place this animation?

@jech jech force-pushed the master branch 2 times, most recently from ace55c3 to 32663db Compare May 9, 2021 15:49
@jech jech force-pushed the master branch 3 times, most recently from 6eeac02 to de78f3c Compare May 18, 2021 11:51
@jech
Copy link
Owner

jech commented Jun 9, 2021

I'm not very keen on animations, they tend to use a suprisingly large amount of CPU time when they're many of them. But I'm quite open to such a change if we verify that the CPU time is not too large.

@jech jech force-pushed the master branch 2 times, most recently from 4f84b48 to 0d2ca28 Compare July 15, 2021 23:40
@jech
Copy link
Owner

jech commented Aug 28, 2021

@erdnaxe, just to let you know that I am interested in this patch, and I'm keenly waiting for you to find the right approach.

@erdnaxe
Copy link
Contributor Author

erdnaxe commented Aug 28, 2021

I am not convinced by the solution I proposed here. Maybe we could avoid showing something on the whole video and rather use div.top-video-controls to show an element when the media is failing.

What do you think of displaying an element in div.top-video-controls when the media is failing? Do you want something more visible?

Proposition of layout, both media are failing, but on the left I am hovering with my mouse:

image

(I made this screenshot using the Web inspector and adding a SVG element in the top controls, I can patch the JS if it turns out to be what we want.)

@jech
Copy link
Owner

jech commented Aug 28, 2021

I fear it's not visible enough. Think about the usage scenario: you are giving a lecture, and suddenly your connection has an issue. You want to notice it straight away.

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.

2 participants