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

Compilations feature #4032

Open
wants to merge 58 commits into
base: master
Choose a base branch
from
Open

Conversation

broquemonsieur
Copy link

@broquemonsieur broquemonsieur commented Aug 6, 2023

Addresses #3857

To try this out, pull the fork/branch

  1. Sign In/Register
  2. Click on Settings, enable "Autoplay" and "Save preferences"
  3. Go to the feeds menu
  4. Select "Compilations", click "Create Compilation" and title it
  5. "Add videos", type a search term and add 0, 1, or many videos
  6. Go back to the feeds menu
  7. Select "Compilations" and click on the most recently created compilation followed by "Edit"
  8. For each compilation_video, adjust the starting timestamp and the ending timestamp, either or none (using legal or illegal arguments)
  9. Click "Save"
  10. Rearrange the videos using the arrows on the left
  11. "Play"

iv-alpha
iv-beta
iv-charlie
iv-delta
iv-echo
iv-foxtrot
iv-golf
iv-hotel
iv-india
iv-juliet
iv-kilo

@unixfox
Copy link
Member

unixfox commented Aug 6, 2023

Make sure to run crystal tool format.


Mmh this requires database changes, I guess this won't be able to get merged anytime soon.

Explained here: #2254 (comment)


Note: this includes typo fixes and "utils" minor changes.

@unixfox unixfox added the blocked require something else first label Aug 6, 2023
docker-compose.yml Outdated Show resolved Hide resolved
Copy link
Member

@unixfox unixfox left a comment

Choose a reason for hiding this comment

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

Can you revert the changes where you remove the newline at the end of a file? It's unnecessary thank you.

I see that on many files.

@unixfox
Copy link
Member

unixfox commented Aug 6, 2023

Additional questions:

  • Does this work without JavaScript?
  • Can you add screenshots on the first comment of your PR?
  • Does this work on old browsers like at least internet explorer 11?

@broquemonsieur

This comment was marked as resolved.

@iv-org iv-org deleted a comment from broquemonsieur Aug 6, 2023
@broquemonsieur
Copy link
Author

broquemonsieur commented Oct 21, 2023

Additional questions:

* Does this work without JavaScript?

* Can you add screenshots on the first comment of your PR?

* Does this work on old browsers like at least internet explorer 11?

Regarding JavaScript, I'm unsure - but will address this right after IE 11
Regarding Internet Explorer 11, I'm unsure - I'm currently running a Windows container via VMWare Fusion on my macOS. It seems though that Docker doesn't support this scenario. Are addressing these questions necessary for the PR?

@broquemonsieur
Copy link
Author

I finished testing and determined JavaScript-less and IE 11 functionality are out-of-scope for this PR

Explanation: My feature is contingent on the ability to start a video after timestamp 0:00. This is done by setting the t URL Query Parameter as seen currently in production when one switches invidious instances. Rerunning this very workflow on a production instance in a JavaScript-less environment or IE 11 starts the video at 0:00 which means this issue should've already been solved prior to my PR if invidious were truly accessible.

(All good and no disrespect meant! I believe the fact that invidious already allow basic functionality on JavaScript-less environments or IE 11 is more than apt for the accessibility cause - glad the maintainers care!)

@ChunkyProgrammer
Copy link
Contributor

I'm curious, is there a reason why compilations are separate from playlists? I feel like this feature might be easier to maintain if it updated playlists to allow specifying start and end times for playlist videos instead of creating new tables and views for compilations.

@unixfox
Copy link
Member

unixfox commented Jan 18, 2024

True and maybe this way it would have probably necessitate no schema change in the database (no migrations to do probably).

@broquemonsieur
Copy link
Author

broquemonsieur commented Jan 22, 2024

@ChunkyProgrammer Why don't you visit 000136.xyz (@unixfox add this to public invidious list - it's been a month-thanks!) and see for yourself? If you are sold on the idea, fork my fork and simplify it yourself :)
I have taken this concept as far as I intended, for my personal use as and you can see, I'm having a blast
Screenshot 2024-01-21 at 6 18 42 PM

@broquemonsieur
Copy link
Author

True and maybe this way it would have probably necessitate no schema change in the database (no migrations to do probably).

It would also allow you to consider #4359

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked require something else first
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants