elan-ev / tobira

Video portal for Opencast
https://elan-ev.github.io/tobira/
Apache License 2.0
22 stars 17 forks source link

Fix various UI things in our paella integration #1208

Closed owi92 closed 2 months ago

owi92 commented 2 months ago

This mainly does two things:

Also:

Things mentioned in #1196 but missing here:

Oh and I also noticed the thing @dagraf mentions in https://github.com/elan-ev/tobira/issues/1196#issuecomment-2218695823: Open submenus are not fixed but move with the screen when scrolling up or down. This also needs to be fixed in paella.

Closes #1196

Marked as draft bc I still need to figure out how to hide the settings menu when there are no menu items 🙈

LukasKalbertodt commented 2 months ago

Basically all changes look good to me and are a clear improvement! Code looks good as well. I have only two remarks:

the control elements (which are also buttons) inside the videos (i.e. maximize video, switch sides etc.) are now less opaque

I know this was discussed in the issue, but it's worth noting that this is also an accessibility concern. Making them less opaque obviously reduces contrast, and makes it hard to see for some people. Tobira does not yet have a high contrast mode, otherwise we could at least improve the situation in that mode. So yeah, maybe Olaf and David can reconsider whether the buttons in the current form (without this PR) are really too distracting.

Marked as draft bc I still need to figure out how to hide the settings menu when there are no menu items 🙈

I think I would prefer if the quality selection thing could always be there, even if there is only one quality. Then at least users can see the quality they are currently watching. I am not sure if it's possible to convince Paella of that though...

oas777 commented 2 months ago

Testing this on a large screen, for once. A certain improvement can be seen, thanks.

However, adding the "Seek video to the previous/next slide" icons was not a request from my end, it was a fear I expressed because it leaves us with eight (!) icons within the video canvass. Which makes me give up on the idea of making them more opaque/"invisible", also in conjunction with the hover-over effect, the different black and grey backgrounds, Firefox adding its thing with a different grey, and the accessibility concerns Lukas mentions. Is there a chance therefore of hiding icons individually the way you added the icons for slide navigation?

I think I would prefer if the quality selection thing could always be there, even if there is only one quality. Then at least users can > see the quality they are currently watching. I am not sure if it's possible to convince Paella of that though...

+1. Which kind of kills the settings menu, does it? The additional "CC" probably shouldn't be hidden there.

owi92 commented 2 months ago

Alright, I can certainly remove the changes to the button opacity again, and also the settings menu if we pull out captions and quality selection (which isn't what Lukas was saying with his comment - at least I don't think so - but I see how it could be read like that and you seem to be agreeing with that point, however interpreted).

Is there a chance therefore of hiding icons individually the way you added the icons for slide navigation?

Hm sorry, I'm not sure I understand. In which case should they be hidden and when are they supposed to be shown? In any case, I don't think paella supports that.

An alternative to the slide navigation buttons at the top is to have them at the sides of the video container (which is still inside the container, so that doesn't necessarily improve the overall situation:

Bildschirmfoto 2024-07-10 um 15 23 00

Or should we just leave out that feature altogether?

owi92 commented 2 months ago

also, re this comment (sorry for not adressing this yet): https://github.com/elan-ev/tobira/issues/1196#issuecomment-2206112365: would you prefer it like this (with blod playtime and colors of the quality selector inverted, if we get rid of the settings menu again)?:

Bildschirmfoto 2024-07-10 um 15 50 59
oas777 commented 2 months ago

Alright, I can certainly remove the changes to the button opacity again, and also the settings menu if we pull out captions and quality selection (which isn't what Lukas was saying with his comment - at least I don't think so - but I see how it could be read like that and you seem to be agreeing with that point, however interpreted).

I see what you mean, maybe I misread Lukas' comment.

Is there a chance therefore of hiding icons individually the way you added the icons for slide navigation?

Hm sorry, I'm not sure I understand. In which case should they be hidden and when are they supposed to be shown? In any case, I don't think paella supports that.

Version 6 used to allow this. And with adding/removing the slide navigation icons, I thought this might be possible for other icons as well.

An alternative to the slide navigation buttons at the top is to have them at the sides of the video container (which is still inside the container, so that doesn't necessarily improve the overall situation: Bildschirmfoto 2024-07-10 um 15 23 00

Or should we just leave out that feature altogether?

Let's discuss this in our meeting.

owi92 commented 2 months ago

I have opened https://github.com/polimediaupv/paella-core/issues/368 to address the issue of open submenus not being fixed to their trigger button, and https://github.com/polimediaupv/paella-skins/issues/6 to figure out what to do with the vertical lines in the progress bar. Edit: also https://github.com/polimediaupv/paella-slide-plugins/issues/7 in regards to the blurry slidemap progress bar marks.

LukasKalbertodt commented 2 months ago

For reference, I just checked: it is possible, by changing the Tobira configuration (specifically player.paella_plugin_config) to disable some buttons. For example:

paella_plugin_config = """{
    "es.upv.paella.frameControlButtonPlugin": { "enabled": false }
}"""

This disables the button that lets you see all slide previews. Sadly, the buttons that you, Olaf, wanted to remove most of all -- e.g. the "enlarge" button hovering over the video -- don't seem to be controlled by plugins? Or at least I don't see which ones are responsible for that so I can't disable it right now. But if we are really interested we could dig deeper or ask the Paella devs.