[C-1799] Add slug support for collection deep-links#2680
Conversation
sabrina-kiam
left a comment
There was a problem hiding this comment.
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))) |
There was a problem hiding this comment.
why do we need to decode the slug here?
There was a problem hiding this comment.
Good question, so for urls that have special characters, they are typically encoded, so decoding here to get the right special characters back
| } = params ?? {} | ||
|
|
||
| const id = useMemo(() => { | ||
| if (collectionName) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
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.