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

Fix: Search not showing next page button when a next page exists #4126

Conversation

ChunkyProgrammer
Copy link
Contributor

@ChunkyProgrammer ChunkyProgrammer commented Sep 28, 2023

closes #1708

Similar to (part of): #3864

This PR checks for a continuation token to determine whether a next page button should be shown. A good example to test this with is when adding videos to a playlist.

Test case:

  • create a playlist
  • go to page to add videos to playlist
  • search "yo mama"
  • See next page button if using this PR, don't see it if not using this pr

Whether there's a next page or not is not included in the api as it would have to be a breaking change

@ChunkyProgrammer ChunkyProgrammer requested a review from a team as a code owner September 28, 2023 18:33
@ChunkyProgrammer ChunkyProgrammer requested review from syeopite and removed request for a team September 28, 2023 18:33
src/invidious/yt_backend/extractors.cr Outdated Show resolved Hide resolved
src/invidious/yt_backend/extractors.cr Outdated Show resolved Hide resolved
Comment on lines 721 to 728
if new_node = node["itemSectionRenderer"]?
raw_items << new_node["contents"].as_a
end
if node["continuationItemRenderer"]?
raw_items.push([node])
end
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if new_node = node["itemSectionRenderer"]?
raw_items << new_node["contents"].as_a
end
if node["continuationItemRenderer"]?
raw_items.push([node])
end
if new_node = node["itemSectionRenderer"]?
raw_items << new_node["contents"].as_a
elsif node["continuationItemRenderer"]?
raw_items.push([node])
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Iirc they're both in the same object which is why i didnt do an elsif. I can probably double check in a few days (not at my computer right now)

Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering, what's the use case for that?
If I understand correctly, it will extract the continuation for shelf renderers and alike?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, for the search page, the continuation seems to be in the same json object as the itemSectionRenderer

Copy link
Member

@SamantazFox SamantazFox Oct 20, 2023

Choose a reason for hiding this comment

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

@syeopite improving upon your suggestion with the use of <<
@ChunkyProgrammer after that I think we're all good ^^

Suggested change
if new_node = node["itemSectionRenderer"]?
raw_items << new_node["contents"].as_a
end
if node["continuationItemRenderer"]?
raw_items.push([node])
end
if new_node = node["itemSectionRenderer"]?
raw_items << new_node["contents"].as_a
elsif node["continuationItemRenderer"]?
raw_items << node
end

Co-Authored-By: syeopite <70992037+syeopite@users.noreply.github.com>
Co-Authored-By: Samantaz Fox <coding@samantaz.fr>
@ChunkyProgrammer
Copy link
Contributor Author

There seems to still be continuation tokens but no next page... (the button displays but clicking the button leads to a blank page)

@ChunkyProgrammer ChunkyProgrammer marked this pull request as draft October 25, 2023 22:41
@ChunkyProgrammer
Copy link
Contributor Author

There's some issues with this PR so I'll be closing it for the time being, I might open another PR in a few weeks that should hopefully work better

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.

[Bug] Next page button is missing
3 participants