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 / prevent recommendations as feed #617

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ChristiaanScheermeijer
Copy link
Collaborator

Description

This small fix prevents the recommendations playlist ending up in the feed param (?r=:feedId). This can cause problems when navigating to a different screen that does consume the feed for fetching the playlist.

Reproduction:

Copy link

Visit the preview URL for this PR (updated for commit 3cc10b7):

https://ottwebapp--pr617-fix-prevent-recommen-nf0votiw.web.app

(expires Sat, 19 Oct 2024 14:57:03 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c198f8a3a199ba8747819f7f1e45cf602b777529

Copy link
Collaborator

@langemike langemike left a comment

Choose a reason for hiding this comment

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

Well spotted! Tested and works ✅

@AntonLantukh
Copy link
Collaborator

AntonLantukh commented Sep 23, 2024

@ChristiaanScheermeijer I think r param is also used for the client analytics purposes to pass the source playlist (where the click came from). In this case the source will be the previous playlist and not the one with recommendations.

@ChristiaanScheermeijer
Copy link
Collaborator Author

@AntonLantukh do you suggest that we should use the recommendations feed by default in the MediaEvent page instead of the r= feed?

@ChristiaanScheermeijer
Copy link
Collaborator Author

@AntonLantukh ?

@AntonLantukh
Copy link
Collaborator

@ChristiaanScheermeijer sorry for the delay, lots of things happening.
I think we can use recommendations list on the MediaEvent page the way we already do for MediaMovie, while using const feedId = useQueryParam('r'); for previous / next item playback + to pass it to the player.

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.

4 participants