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

Update Paella and improve cursor #1041

Merged
merged 1 commit into from
Dec 21, 2023

Conversation

LukasKalbertodt
Copy link
Member

@LukasKalbertodt LukasKalbertodt commented Dec 20, 2023

Two main visible changes:

I think the first point is clearly an improvement. Regarding the second one I'm not sure: some live streams might allow seeking into the past, which wouldn't be possible anymore with this. I'm really not sure whether that's a concern for institutions out there. So yeah, feedback is welcome @dagraf @oas777 @ziegenberg

Main change from this: a `pointer` cursor when hovering over the
timeline or buttons.

Closes elan-ev#707
@LukasKalbertodt LukasKalbertodt added the changelog:user User facing changes label Dec 20, 2023
@github-actions github-actions bot temporarily deployed to test-deployment-pr1041 December 20, 2023 14:47 Destroyed
@oas777
Copy link
Collaborator

oas777 commented Dec 20, 2023

Regarding the second one I'm not sure: some live streams might allow seeking into the past, which wouldn't be possible anymore with this. I'm really not sure whether that's a concern for institutions out there. So yeah, feedback is welcome @dagraf @oas777 @ziegenberg

Not sure whether that's ex-post overthinking on your behalf or "underthinking" when the issue was opened. Anyway, I think it's an improvement for now. If someone wants this reversed/customized for the scenario you describe, they should come forward.

@ziegenberg
Copy link
Contributor

Paella has a configuration item for hiding/showing the timeline. We should surface that config in Tobira.

@LukasKalbertodt
Copy link
Member Author

LukasKalbertodt commented Dec 20, 2023

Paella has a configuration item for hiding/showing the timeline. We should surface that config in Tobira.

My philosophy for Tobira is to not make things configurable unless there is a good reason to do so. This is all in service of complexity management (which I deem very important in software dev) and consistency. If we can't find a consensus here on whether to hide or show it for live stream videos, sure let's add a config. But at least try to find out whether there is a choice that would work for everyone.

Do you have use cases where having the timeline for live events is useful?

@owi92
Copy link
Member

owi92 commented Dec 21, 2023

Just chiming in here as this is also waiting for David's feedback before we merge this:
I noticed two visual things (that should be fixed on paella's end I believe):
The progress bar is now cut-off on the right side (or rather it's not rounded anymore, like it was before this change):
Bildschirmfoto 2023-12-21 um 12 03 33

And I think when the progress bar/timeline is hidden for live events, it would make sense to also hide its background/shrink the control panel accordingly:
Bildschirmfoto 2023-12-21 um 12 05 13

@LukasKalbertodt
Copy link
Member Author

Oof, good catch, Paella never ceases to amaze me. I just removed the questionable commit, i.e. the "hide timeline for live" is gone now. I will create a separate issue for that, so that we can move along with the important part of this PR.

@github-actions github-actions bot temporarily deployed to test-deployment-pr1041 December 21, 2023 11:44 Destroyed
@LukasKalbertodt LukasKalbertodt changed the title Update Paella, improve cursor, and hide timeline for live videos Update Paella and improve cursor Dec 21, 2023
@owi92 owi92 merged commit 02f5982 into elan-ev:master Dec 21, 2023
2 checks passed
@LukasKalbertodt LukasKalbertodt deleted the update-paella-and-fix-cursor branch December 21, 2023 12:57
@oas777
Copy link
Collaborator

oas777 commented Dec 21, 2023

And I think when the progress bar/timeline is hidden for live events, it would make sense to also hide its background/shrink the control panel accordingly:

Honi soit qui mal y pense.

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
4 participants