Repository navigation
Conversation
Links that point at a specific podcast episode all navigated to the podcast item page, because /item/:id has no way to address an episode. The three places the issue calls out -- the currently-playing bar title, the episode cards on the Home shelves, and the playlist item rows -- are now built as /item/:id?episode=<episodeId>, and the item page opens the show notes for the episode in the query. The query is dropped again when the modal closes so the same link can be reopened without a page reload, and the global modal is closed if the page is left while it is open.
Contributor
|
Thanks for opening a PR, but no frontend changes are being reviewed or merged due to the ongoing React rewrite. |
Author
|
Understood — thanks for the heads-up about the React rewrite. Closing this out so it doesn't sit in the review queue; happy to revisit once frontend changes are being accepted again. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brief summary
Episode-specific links all land on the podcast item page instead of the episode.
/item/:idhas no way to address an episode, so every link that knows an episode id still had to throw it away and link to/item/<podcastId>.This makes those links carry the episode in the query (
/item/<podcastId>?episode=<episodeId>) and teaches the item page to open the show notes for the episode named in the query.Which issue is fixed?
Fixes #5523
The three places the issue lists:
MediaPlayerContainer)LazyBookCard, viaBookShelfCategorized'sepisodeshelves)ItemTableRow)Clicking the play button already started the correct episode, which is why the bug reads as "it plays the episode but the link goes somewhere else".
In-depth Description
Item page (
client/pages/item/_id/index.vue)showEpisodeFromQuerylooks the episode up in the already-fetchedmedia.episodesand drives the existingViewEpisodemodal through the store commits the episodes table already uses (setSelectedLibraryItem,globals/setSelectedEpisode,globals/setShowViewPodcastEpisodeModal). It runs onmountedand on a watcher, so/item/li_x?episode=ep_yworks on a cold load and when the query changes while the page is open. An unknown id gets a toast instead of a silently dead page, and a non-podcast item ignores the query.Two follow-on details that the deep link makes necessary:
?episode=…, re-clicking the same link changes nothing, and clicking around the same podcast's episodes becomes dead.ViewEpisodeis a global modal, sobeforeDestroycloses it if this page opened it and the user navigated away with it still up, instead of leaving it over the next page.Link sites
Each one builds the same
/item/:id[?episode=…]shape from the episode id it already has in hand — no new plumbing, no server or API change. Non-episode items keep the plain/item/:id.How have you tested this?
client/cypress/tests/components/cards/LazyBookCard.cy.jsgains awhen recentEpisode is presentblock next to the existingwhen collapsedSeries is presentblock, asserting the card routes to/item/1?episode=ep-123.To be straight about the state of this: I have not run the component suite or the app locally. Neither the client toolchain nor a full install fit the disk budget on the machine this was written on, so the Cypress test above is written but unrun, and the reproduction steps are reasoned from the code paths rather than executed. If you would rather take it without the test, say so and I'll drop it.
What I did check: every touched
<script>block parses,en-us.jsonis valid JSON, and the newToastEpisodeNotFoundkey is in the alphabetical position thei18n-integrationworkflow expects.Reasoning for the three surfaces, for review convenience — the item page's
asyncDataalready fetches?expanded=1, somedia.episodesis present on mount and the lookup needs no extra request;LazyEpisodeRow.clickedEpisodeandLazyEpisodesTable.viewEpisodeare the two existing callers of this modal and the new code reuses their exact three store commits; andViewEpisodeis rendered once inlayouts/default.vue, so it is already mounted on the item page.Suggested manual pass: open an episode from each of the three surfaces and check the modal shows that episode; confirm the play button still starts it; confirm an unknown
?episode=id toasts; confirm Home shelf cards for non-episodes and ordinary book/playlist rows still go to the plain item page; and confirm closing the modal then re-clicking the same link reopens it.Screenshots
N/A — routing plus the existing episode modal, no visual change.