Skip to content

Added "last" and a reversed ?playlistPosition in playlist URL#3974

Merged
Chocobozzz merged 6 commits into
Chocobozzz:developfrom
Poslovitch:feature/3897-playlistposition-last
Apr 26, 2021
Merged

Added "last" and a reversed ?playlistPosition in playlist URL#3974
Chocobozzz merged 6 commits into
Chocobozzz:developfrom
Poslovitch:feature/3897-playlistposition-last

Conversation

@Poslovitch

Copy link
Copy Markdown
Contributor

Description

I implemented the ?playlistPosition=last alongside a "new" feature: the ability to provide a negative playlistPosition, which acts like a "reversed" array index. ?playlistPosition=-1 selects the first video from the bottom of the playlist, ?playlistPosition=-2 the second, etc.

This was done as to avoid modifying the prototype of the updatePlaylistIndex (position: number) method.

Related issues

Resolves #3897.

Has this been tested?

  • ð��� yes, light tests as follows are enough
  1. Create a playlist and add a dozen videos to it.
  2. Check that ?playlistPosition=last works.
  3. Check that ?playlistPosition=-1, ?playlistPosition=-3, etc. works.
  4. Check that ?playlistPosition=1, ?playlistPosition=3, etc. works.
  5. Check that ?playlistPosition=-100 is defaulted to 1.
  6. Check that ?playlistPosition=saladedefruit is defaulted to 1.

@Poslovitch Poslovitch changed the title Added "last" and a reversed playlistPosition Added "last" and a reversed ?playlistPosition in playlist URL Apr 17, 2021
Comment thread client/src/app/+videos/+video-watch/video-watch.component.ts Outdated
Comment thread client/src/app/+videos/+video-watch/video-watch.component.ts Outdated

updatePlaylistIndex (position: number) {
if (this.playlistElements.length === 0 || !position) return
if (position < 0) position = (this.playlistPagination.totalItems + 1) + position // Handle the "reverse" index

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't think you need to calculate the position here since you already built it in the video watch component

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, the position is not properly calculated in the video watch component, because this.playlist is undefined at that time. In the commit I just pushed, I got rid of it. In the video watch component, this.playlistPosition can be negative, while updatePlaylistIndex() does the job of translating the actually negative position into an index it can use.

Comment thread client/src/app/+videos/+video-watch/video-watch.component.ts Outdated
@Chocobozzz

Copy link
Copy Markdown
Owner

Thanks @Poslovitch

@Chocobozzz Chocobozzz merged commit e771e82 into Chocobozzz:develop Apr 26, 2021
@Poslovitch Poslovitch deleted the feature/3897-playlistposition-last branch April 26, 2021 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add "last video" option to playlist url

3 participants