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

Display time remaining until video goes live #2501

Merged
merged 11 commits into from Sep 5, 2022

Conversation

Aiz0
Copy link
Contributor

@Aiz0 Aiz0 commented Aug 20, 2022


Display time remaining until video goes live

Important note
We may remove your pull request if you do not use this provided PR template correctly.

Pull Request Type
Please select what type of pull request this is:

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue
closes #2391
alleviates the timezone issue on flatpak #1429

Description
Display how much time is left until the live stream or video premiere goes live above the exact premiere datetime.
This is static and will not update automatically as the premiere time gets closer.

The time left will be displayed in the format "Premieres in X U" where X is a number and U is a time unit such as hours, minutes, or days. I use Math.floor() on the number so it's always a whole number. From what I have seen this is what YouTube seems to do.

When to switch from hours to minutes to seconds remaining is based on how I understand YouTube does it.

Screenshots (if appropriate)

Before:
2022-08-20_T19:20:23

After:
2022-08-20_T19:48:09

with custom date
2022-08-20_T18:27:46
2022-08-20_T18:28:13
2022-08-20_T18:28:57
2022-08-20_T18:29:22
2022-08-20_T18:30:41

Testing (for code that is not small enough to be easily understandable)
Has this pull request been tested?
Please describe shortly how you tested it and whether there are any ramifications remaining.

I used upcoming livestreams from YouTube live channel and compared the times.
I also tested with a custom date instead of upcomingTimestamp to see if all types of time remaining where displayed correctly.

Desktop (please complete the following information):

  • OS: [e.g. iOS]
  • OS Version: [e.g. 22]
  • FreeTube version: [e.g. 0.8]

Additional context
Is it possible to differentiate between live streams and premieres?
If so, then it might be good to have separate strings for them. Such as "Live in" and "Premieres In"

Sometimes YouTube seems to display time remaining like this.
2022-08-20_T16:35:10
This seems to happen when there is less than 60 minutes left.
I don't know if they do this on all videos now, but that format should only be considered if we update the time automatically

Since I am new to FreeTube development I would appreciate help and feedback.

@Aiz0 Aiz0 marked this pull request as ready for review August 20, 2022 19:14
@ChunkyProgrammer
Copy link
Member

Thanks for opening a pr! Would you be able to lint it using npm run lint and applying the fixes?

@PrestonN PrestonN enabled auto-merge (squash) August 21, 2022 11:18
@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added the PR: waiting for review For PRs that are complete, tested, and ready for review label Aug 21, 2022
if (upcomingTimeLeft > 60) {
upcomingTimeLeft = upcomingTimeLeft / 60
timeUnit = 'Minute'
if (upcomingTimeLeft > 120) {
Copy link
Member

Choose a reason for hiding this comment

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

Haven't tested yey, but shouldn't this be 60 instead of 120? (Correct me if I'm wrong)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK youtube switches to showing time left in minutes at 120 minutes remaining. I tried to replicate how they choose to display it.

src/renderer/views/Watch/Watch.js Outdated Show resolved Hide resolved
src/renderer/views/Watch/Watch.vue Show resolved Hide resolved
@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Aug 23, 2022
Better temp variable scoping, flatten nested code, rename temp variables, use string intepolation

Co-authored-by: PikachuEXE <pikachuexe@gmail.com>
auto-merge was automatically disabled August 23, 2022 16:49

Head branch was pushed to by a user without write access

@PrestonN PrestonN enabled auto-merge (squash) August 23, 2022 16:50
tabs where used in some places in the suggested code
auto-merge was automatically disabled August 23, 2022 17:15

Head branch was pushed to by a user without write access

@PrestonN PrestonN enabled auto-merge (squash) August 23, 2022 17:15
Since upcomingTimeStamp will be null when the time has passed the scheduled timestamp
it doesn't make sense to use something that will rarely be displayed.
e.g. a user has to click on the video with less than a second remaing until it goes live for it to be displayed

it would also be displayed as "Premieres in Starting soon" which doesn't make sense
auto-merge was automatically disabled August 23, 2022 18:12

Head branch was pushed to by a user without write access

@PrestonN PrestonN enabled auto-merge (squash) August 23, 2022 18:12
@ChunkyProgrammer ChunkyProgrammer added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Aug 23, 2022
@Aiz0
Copy link
Contributor Author

Aiz0 commented Aug 23, 2022

Seeing as this is doesn't update automatically would this be preferred instead of showing "premiers in x seconds"?
2022-08-23_T20:54:19

@PikachuEXE
Copy link
Collaborator

PikachuEXE commented Aug 24, 2022

Seeing as this is doesn't update automatically would this be preferred instead of showing "premiers in x seconds"?

Yes

@PikachuEXE
Copy link
Collaborator

Let me know when final changes made will put review comment (should pass is no new issue found)

auto-merge was automatically disabled August 24, 2022 15:15

Head branch was pushed to by a user without write access

@PrestonN PrestonN enabled auto-merge (squash) August 24, 2022 15:16
@Aiz0
Copy link
Contributor Author

Aiz0 commented Aug 24, 2022

I've made the final changes now.

Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Code looks good, testing locally later

@PikachuEXE
Copy link
Collaborator

I can't find any video on https://www.youtube.com/channel/UC4R8DWoMoI7CAwX8_LjQHig
Any other channels can be used to test?

@absidue
Copy link
Member

absidue commented Aug 25, 2022

I can't find any video on https://www.youtube.com/channel/UC4R8DWoMoI7CAwX8_LjQHig Any other channels can be used to test?

You have to open the link on YouTube, it's a special page (so it doesn't work in FreeTube) that shows videos that are currently live and ones that have recently been live.

Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Tested with a few about to live videos in https://www.youtube.com/channel/UC4R8DWoMoI7CAwX8_LjQHig

Copy link
Member

@ChunkyProgrammer ChunkyProgrammer left a comment

Choose a reason for hiding this comment

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

LGTM

@PrestonN PrestonN merged commit c784841 into FreeTubeApp:development Sep 5, 2022
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Sep 5, 2022
@Aiz0 Aiz0 deleted the upcoming-display-time-left branch September 25, 2022 01:40
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.

[Feature Request]: Show time left until livestream/premiere starts
6 participants