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

Load next videos in the queue #57

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Load next videos in the queue #57

wants to merge 3 commits into from

Conversation

xypnox
Copy link
Member

@xypnox xypnox commented Oct 22, 2020

This PR will add the following feature:

Load the next set of videos in queue.

By queue I mean when the video is being played, the queue for next videos to be played.

This can happen with several interactions:

  • When the last video or close to the last video is played in the queue.
  • When the user scrolls to the bottom of the queue
    • infinite scroll
    • loading button for next videos

The first problem is a bit easier, we fetch the next set of videos when the last video starts to play or ends.

The second problem has two solutions:

  1. Infinite Scroll
    Pros: Smoother UX, Feels modern.
    Cons: Addictive, "Unethical" (We don't have anything to gain by watchtime increase but users waste more time searching/waiting for a good video to show up), a little harder to implement and manage.

  2. Loading Button
    Pros: Ethical, saves time, easier to implement and debug.
    Cons: Feels older.

Apart from the UX there are a few other difficulties as well:

The Video component and Queue in Store don't have a FeedType.

This is required because while loading the next set of videos we want the FeedType from which to load the video.

The Lists and Queue in the reducers serve the same purpose.

The current approach is to load the data from API into the lists and then fill the Queue whenever the user starts a Video.
Due to this there is duplicate data present in queue. As the state of the queue visible in the component is different from that stored in the redux store, the state of the redux store is useless. The Queue is just another step in between the Store's lists and the local state queue in the component.

This affects the problem in hand because when we need to fetch new videos to add to the list, we have to:

  1. Fetch them from API which get added to the list.
  2. Add the videos from the list to the queue.
  3. Update the local state queue with the queue from the store.

Instead of this, we can store a current playing list type variable and simply update the local queue of the component with the list of that type. Fetching new videos to add to the queue will be simplified: The new videos get added to the list, the local queue is updated with the list.

In the spirit of simplifying, I have added the type of the current list in the queue. With some modification we can remove the posts variable and then it would resemble my suggestion above. Afaik, there are no specific usecases that require us to have a separate queue in the redux store. As this requires some thought and discussion, I have described the change here before implementing it.

Please review the idea, I will start on the feature as soon as we land on a consensus.

This PR will fix #50

@xypnox
Copy link
Member Author

xypnox commented Oct 22, 2020

Also the deploy is failing due to being WIP (some imports haven't been used)

@ghostwriternr ghostwriternr removed their request for review May 30, 2021 16:32
@ghostwriternr
Copy link
Member

@xypnox let's go through the implementation for this PR in the coming week. On a quick sidenote, do check why the build is failing. Looks like some node version issue, I don't have access to the build settings for our Netlify team.

@ghostwriternr ghostwriternr marked this pull request as draft May 30, 2021 16:39
@xypnox
Copy link
Member Author

xypnox commented May 30, 2021

I have updated the Netlify env to add node v12. It should build properly now, it seems node-sass requires >= v12 and netlify defaults to v10.

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.

Load more videos when the end of the queue is reached
2 participants