-
Notifications
You must be signed in to change notification settings - Fork 18
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 "start at:" option to share menu #943
Conversation
Just noticed an issue with the timestamp in the share menu. Will fix asap. |
Ole, if this lets me manually enter the time information, we're on the wrong track from my perspective: The timestamped sharing should be synchronized with the player so that the user can choose whether to add that timed information or not. Ideally, one could also change that information. Cf. https://tinyurl.com/279ecb8t under "Media". |
Oh ok, |
I agree with @oas777, that the timestamped sharing should be synchronized with the player. Here two screenshots how YouTube and SWITCHtube is offering this feature. I would prefer the solution of SWITCHtube or ETH-videoportal. YouTube:
SWITCHtube:
|
I also think we want a checkbox whether to include the timestamp. And that the time should be synchronized to the player. Well, when the share menu is opened, the current player time should be taken. Whether it needs to update continuously if the player if playing... not sure about that. But I also think it should be editable (as it is now). If the user wants to manually set a time. That's also useful for the share thing in the "my videos" section. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This already implements two things at once! It also closes #678.
As usual, some comments, but nothing special. The main problem is the UI for picking the timestamp.
Before this, the urls were build using the exact url of the page, meaning it would include any search query. This fixes this by removing existing time queries before appending a new one.
With this, the current state (whether it's loaded) and certain methods like getting and setting the current play time of the video can be accessed by using the player ref.
2c34ef0
to
32c7460
Compare
@oas777, @dagraf: I have updated this to be synchronized with the player. The current solution is closer to youtube as I don't think the time should be continuously updated when the video is still playing.
But if one or both of you still feel strongly that this should behave closer to SWITCHtube or the ETH portal, like David mentioned in his comment, we can do that of course. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the current behavior is what we want. Only taking the current video position when the share menu is opened.
A few more things:
- This time input needs to be disabled for livestreams and upcoming livestreams as it doesn't make sense there (right?)
- I would add more (vertical) space between the time input line and the resulting link input.
- I can input
3
then add a minus before it. The textbox resets to 0, but the link contains the -3.
I suppose we could "freeze" the time once the checkbox gets activated.
They do indeed. That's an oversight on my end.
We will need some form of indication that the time figures are editable, at the very least for accessibility considerations. |
Apologies, Ole, my (now deleted) answer did not fully take into account what you wrote yesterday and now you've already replied... I will continue in response to that latest comment of yours. |
Oh sorry, I didn't notice you deleted your answer |
My fault entirely. Let's stick with time synch as it is. Given you don't have the same interaction situation is was referring to in our current video portal, we can assume users will wait for the "right moment" in the video, then switch to "Share". And yes, let's see an alternative to the boxes please. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like how it looks and that you can use tab to switch to the next field. I think though that most people would use arrow keys to do that, so I fear you have to implement that as well :/
And a completely different thing I noticed: the embed route doesn't work anymore, due to the player context thingy. You have to add the context provider in routes/Embed.tsx
too. FYI: You can just open the link from the embed code in the browser.
f2a4a5d
to
5fc2854
Compare
5fc2854
to
64e9816
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it looks and works really well now! I found two bugs still unfortunately.
- The QR code doesn't contain the time parameter. Should be simple to fix.
- The embed route with time parameter doesn't seem to work. Paella shows it's playing the video, but I don't see any video. No idea what's going on there :/
EDIT: Uhm, it's weirder. Sometimes it works. If I wait for a long time before pressing play it works, if I press play immediately it doesn't. Wait... and the audio always plays, but what I meant is that the video is just grey in those cases. Wat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like all comments here (also by the others) were resolved. Thus merging. We can always re-adjust later.
FWIW, this is the syntax our video portal is using currently: Which is different from Tobira: YouTube has Vimeo so we know who is the odd one out. Probably have to accept time-stamped links from the old video portal will not work in the new one. Unless you tell me there's an easy way out. |
YouTube also has the We will have to see regarding your old video portal. With what we just discussed there are multiple such "old links need to work" issues. Might be worth it just adding a tiny service in front of Tobira that just rewrites old links to new links. But again, I think we can talk about this separately. |
If this is just about the time parameter, I think we can easiliy change the |
Thanks, Ole, if you could allow both that would be great. Slightly related: What was the reason for the "/v" in https://tobira.opencast.org/lectures/d-infk/2019/autumn/252-0856-00L/v/EJic0mSes6f? Because that would be https://video.ethz.ch/lectures/d-infk/2019/autumn/252-0856-00L/1603eb14-81aa-4286-b076-8d39b632dcae at our end. |
To avoid ambiguitities/name clashes. What if you want to create a page with a path the same as a video ID. Unlikely but this is a cleaner system. But did you also notice that your video portal uses Opencast IDs (the long ones) and Tobira uses its own IDs (shorter). As I already said: I am pretty sure we want some extra service in front of Tobira to rewrite old video portal links. We can't change Tobira such that all old links from your video portal still work. It also wouldn't make sense to put such backwards-compatibility things into a new project that is for the whole community. I also wouldn't change the |
Let me get this straight: You added this just in case someone was to create a page called "EJic0mSes6f"? |
PS: Agreed, I put this on the list of issues in the context of an ETH migration. |
Any 11 character alphanumeric string is a valid Tobira video ID (e.g. "vorlesungen", "jahresvideo" and "leadership2"). While most IDs will not clash with anything anyone ever wants to type, this is still a possibility. I don't want someone running into this rare issue 4 years from now, thinking "what a stupid software" (rightly so). But maybe more importantly: we might change the Tobira ID format in the future. And we might very well add additional things other than |
Closes #191
This adds a "Start at: {time}" timepicker to the direct link on the
Video details
page and both share options of the video page's share menu.Picking a time will append said time as a search query to the shared link.
The linked video will start at the chosen time.
Users can also manually add a time to a link in their browser by appending
?t=2h2m20s
(or any other combination of hours, minutes and seconds like?t=2m
or?t=1h20s
or?t=20s
etc). You can try it out with https://pr943.tobira.opencast.org/v/GjDqXFlukkH?t=20s (or any other video).There is unfortunately one visual issue with the time picker in chrome, where the rightmost number is slightly cut off. I don't know what causes this yet.