Skip to content
This repository has been archived by the owner on Oct 4, 2023. It is now read-only.

[C-1799] Add slug support for collection deep-links #2680

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

Conversation

dylanjeffers
Copy link
Contributor

Description

Updates mobile deep links to support playlist slug format

Dragons

Need to confirm if we also need to decostruct the "slug" to handle + id and pass all three along to fetch request.

Copy link
Contributor

@sabrina-kiam sabrina-kiam left a comment

Choose a reason for hiding this comment

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

looks great!! thanks so much for doing this. i had a few questions we can discuss and then hopefully we can test everything soon

const handleFetchCollection = useCallback(() => {
dispatch(fetchCollection(id))
}, [dispatch, id])
dispatch(fetchCollection(id, slug && decodeURIComponent(slug)))
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to decode the slug here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question, so for urls that have special characters, they are typically encoded, so decoding here to get the right special characters back

Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha ty!

} = params ?? {}

const id = useMemo(() => {
if (collectionName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

curious if you think we should keep this around for that edge case you mentioned earlier where playlists in local storage might not have permalinks yet?

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 exactly what I was thinking as well, unless it's the case that the actions/sagas we are running take the permalink and extract handle id as a backup? Guessing that's not the case and I should send all three along in the fetch action?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah that is not the case, so we'd have to send all 3 along in fetch 😬 yea i think we'd want to pass along in fetch? also realizing this is the screen used for albums too probably, and albums use the original routing and expect the name to contain id and name so we still need this for sure

@AudiusProject AudiusProject deleted a comment from linear bot Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants