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

Add new Paella skin #938

Merged
merged 7 commits into from
Sep 28, 2023
Merged

Add new Paella skin #938

merged 7 commits into from
Sep 28, 2023

Conversation

LukasKalbertodt
Copy link
Member

@LukasKalbertodt LukasKalbertodt commented Sep 18, 2023

Closes #918

Is still a draft for multiple reasons:

Open questions

Button alignment

I took the liberty and center aligned all button icons:

image

But the design itself wants to do it like this:

image

Opinions?

Semi-transparent background

I also made the toolbox background semi-transparent, which is also not part of the original design:

image

On hover it becomes fully opaque. What do you think about this?

@LukasKalbertodt LukasKalbertodt added the changelog:user User facing changes label Sep 18, 2023
@github-actions github-actions bot temporarily deployed to test-deployment-pr938 September 18, 2023 15:48 Destroyed
@github-actions github-actions bot temporarily deployed to test-deployment-pr938 September 18, 2023 16:06 Destroyed
@dagraf
Copy link
Collaborator

dagraf commented Sep 18, 2023

I took the liberty and center aligned all button icons:

I like the center aligned version.

I also made the toolbox background semi-transparent, which is also not part of the original design:
On hover it becomes fully opaque. What do you think about this?

I like the semi-transparent background and would even suggest to make it even more transparent. What I do not like so much is the hover over effect. I would not apply it here like this the hover over effect.

@oas777
Copy link
Collaborator

oas777 commented Sep 18, 2023

  • -1 on the alignment which has been chosen on purpose, cf. Design: Adapt the -/+ 10s buttons polimediaupv/paella-ethz#13 (comment): The numbers should be in line. Maybe Lisa can decide this for us?

  • +1 on transparency - we should push this to Paella though. David, I'm not sure I understand your comment: This becoming fully-opaque on hover-over helps users to see the control bar better when they want to use it, wouldn't it?

@dagraf
Copy link
Collaborator

dagraf commented Sep 19, 2023

Hover-over: Yes, this might help by heightening the contrast. Personally, I do not find it pretty when the bar becomes fully opaque.

@LukasKalbertodt
Copy link
Member Author

Re button alignment: I very slightly adjusted the positioning of the buttons and also of the text within the icons. It's still mostly centered, but the text is also less off the baseline. Let me know what you think.

I read the thread Olaf linked and I understand the "text baseline alignment" reasoning, but I think it has a big disadvantage: when you hover the button, the icon inside the blue area looks super misaligned.

image

I asked internally here and so far everyone prefers the center aligned version, including our designer after she saw the hover screenshot. But that's surely something we can discuss further and yes, maybe just let Lisa decide in the end!


Re transparency: I decreased the opaqueness of the toolbar background from 85% to 80% (idle) and 100% to 90% (on hover), as per suggestion. Let me know what you think!

@github-actions github-actions bot temporarily deployed to test-deployment-pr938 September 19, 2023 07:52 Destroyed
@LukasKalbertodt
Copy link
Member Author

I've tried using the configured primary color as "accent color" in Paella, but unfortunately that doesn't quite work yet. I described the problem here: polimediaupv/paella-skins#3
So I'm waiting for that to get resolved first.

@LukasKalbertodt
Copy link
Member Author

One more update: I added a slight transition animation between the "on hover" background transparency and the normal one. Maybe that makes the on hover effect a bit less jarring?

@github-actions github-actions bot temporarily deployed to test-deployment-pr938 September 19, 2023 08:26 Destroyed
@dagraf
Copy link
Collaborator

dagraf commented Sep 19, 2023

To @LukasKalbertodt adjustments:

  • Thanks for the screenshot with the misaligned icon in the hover button which does really look awkward. I would therefore vote for centering the buttons, again. :)
  • Transparency: +1 for switching between 80% and 90%. Your comment about the transition animation I do not understand.

Additional observation:

  • When taking the cursor away from the viewer it takes 4 seconds until the buttons disappear. I would make this much shorter, maybe even immediately (like this YouTube and Vimeo are doing it). Imagine having a video with subtitles and having always to wait 4 seconds when having skipped in the video until you are able to see the subtitles properly.

@LukasKalbertodt
Copy link
Member Author

LukasKalbertodt commented Sep 19, 2023

Your comment about the transition animation I do not understand.

Before, when your mouse cursor entered the playbar area, the background color switched immediately from 85% opaque to 100% opaque. An immediate change. Now it takes 80ms for this change to happen, where the background color changes gradually during that time. I hope this explains it?

When taking the cursor away from the viewer it takes 4 seconds until the buttons disappear. I would make this much shorter, maybe even immediately (like this YouTube and Vimeo are doing it).

Totally agree. It also immediately annoyed me. Will try to find a way to change this.

EDIT: Unfortunately, this cannot be changed right now. I created two upstream issues:

@oas777
Copy link
Collaborator

oas777 commented Sep 19, 2023

  • Centering buttons: I agree the blue background positioning looks poor on hover-over. But the other way round, non-aligned numbers look poor, too. Let's get Lisa's feedback, at least she knows who pays her invoice.
  • Transparency: A little bit more in the original state. Hover-over is fine.
  • I like the "slow" disappearance of the control bar, actually. But making it configurable ends this discussion, I think. Actually, I think it is configurable already because the productive player here disappears after 3 seconds approx. And another remarks: It does not reliably disappear her (Firefox/Windows).

@oas777
Copy link
Collaborator

oas777 commented Sep 19, 2023

PS: Overall, there are other issues we should discuss, cf.
grafik

Your viewport size is:
1280 × 531
DPR: 1.50
Screen Size: 1280 × 720

And the canvass should be all white.

@github-actions github-actions bot temporarily deployed to test-deployment-pr938 September 21, 2023 07:23 Destroyed
@oas777
Copy link
Collaborator

oas777 commented Sep 21, 2023

[Make Paella hide its UI after 2s already]
I thought we are making this configurable?

@LukasKalbertodt
Copy link
Member Author

[Make Paella hide its UI after 2s already]

I thought we are making this configurable?

Fernando replied in one of the issues I created and explained that it's already configurable. Not via config file, but via code. So Tobira can already configure it, which I did with the latest "commit". You can check in the test deployment whether 2s feel right for you. Keep in mind that in the future, the UI will be immediately hidden if oyu move the cursor outside of the player area.

@oas777
Copy link
Collaborator

oas777 commented Sep 21, 2023

Thanks. Two seconds work for me. In an ideal world, the hiding would be less abrupt/smoother.

@dagraf
Copy link
Collaborator

dagraf commented Sep 21, 2023

Two seconds work for me, too. But I'm looking forward to this future, where the UI will be immediately hidden.

@github-actions github-actions bot added the status:conflicts This PR has conflicts that need to be resolved label Sep 26, 2023
@github-actions

This comment was marked as resolved.

I don't really expect this code to end up in Tobira. If everyone agrees
that this is a good thing, this should go upstream.
@github-actions github-actions bot removed the status:conflicts This PR has conflicts that need to be resolved label Sep 28, 2023
@LukasKalbertodt
Copy link
Member Author

Okay, so as far as I can tell the only thing that's left is using Tobira's configured "primary color" for Paella, instead of the Paella default blue. Is my understanding correct or is there anything else that still has to change?

@github-actions github-actions bot temporarily deployed to test-deployment-pr938 September 28, 2023 12:17 Destroyed
@oas777
Copy link
Collaborator

oas777 commented Sep 28, 2023

Looks good so far, but features/symbols on the right side of the control bar are missing, cf.

https://polimediaupv.github.io/paella-ethz/?id=belmar_16_9_hls.

@LukasKalbertodt
Copy link
Member Author

features/symbols on the right side of the control bar are missing

From right to left:

  • Fullscreen button: we have this as well.
  • Subtitle button: We have this as well, but we don't have any videos with subtitles. That's why it's hidden. Maybe we should add a test video with subtitles...
  • Slides button: Still missing, tracked by Paella Player: Show video sections  #368
  • Layout button: I just added this (note that it's only visible for videos with two streams)
  • Settings button containing two sub-things:
    • Quality: we already have this, but it's hidden because Paella 7 removed the ability to change the quality when not using HLS. (I'm still very unhappy about this). So it's only shown for Livestreams in our test deployment.
    • Keyboard shortcuts: missing, but player keyboard shortcuts are a whole thing we need to tackle at some point. Not in this PR.

@github-actions github-actions bot temporarily deployed to test-deployment-pr938 September 28, 2023 14:19 Destroyed
@LukasKalbertodt
Copy link
Member Author

We decided that we can already merge this without the correct colors. We can always fix that later and this is already a big improvement obviously.

@LukasKalbertodt LukasKalbertodt marked this pull request as ready for review September 28, 2023 14:35
@oas777
Copy link
Collaborator

oas777 commented Sep 28, 2023

To add to the confusion, not only does the "version" we use in Tobira now differ from https://polimediaupv.github.io/paella-ethz/?id=belmar_16_9_hls, but that one differs from what we agreed with UPV (background should be white, e. g.). Let's revisit this at a later stage.

Copy link
Member

@owi92 owi92 left a comment

Choose a reason for hiding this comment

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

I agree that the background of the video container should be either black or white, or rather just not be visible at all by making the whole UI scale with the video, though that also comes with a few caveats. But yeah, let's discuss that in another issue. For now I'm happy that the new skin finally arrives in Tobira.

@owi92 owi92 merged commit daa3ed5 into elan-ev:master Sep 28, 2023
@LukasKalbertodt LukasKalbertodt deleted the paella-design branch September 28, 2023 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:user User facing changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apply updated ETH/Opencast paella skin
4 participants