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

[gh14] jump links #54

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[gh14] jump links #54

wants to merge 6 commits into from

Conversation

ahaywood
Copy link
Owner

Fixes #14

Feature description

  • URLs work for deep linking. Uses the structure ?time=XX
  • Links within the episode page jump to the correct spot within the audio
  • Update the route, so when a user clicks on the jump link, the URL updates

@vercel
Copy link

vercel bot commented Aug 17, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/ahhacreative/compressedfm/249eUyTyyapfxXN3rxB5qkYS52Pq
✅ Preview: https://compressedfm-git-feature-gh14-jump-links-ahhacreative.vercel.app

[Deployment for 2b95eee failed]

@ahaywood
Copy link
Owner Author

There are a couple of bugs:

  • When you first load the episode the audio gets stuck in a loop.
  • Play / Pause gets out of sync.

@@ -35,6 +36,12 @@ const IndividualEpisodePage = ({
}) => {
// state
const [skipTo, setSkipTo] = useState(null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Going to list out what I have changed.

This should be 0 not null


useEffect(() => {
const UrlParams = router.query;
skipToTimestamp(UrlParams.time);
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to make sure that time is always a number. Number(())

I see you did it below

@@ -13,6 +13,10 @@ export const useAudioPlayer = (audioRef, progressBarRef) => {
progressBarRef.current.max = seconds;
};

useEffect(() => {
onLoadedMetadata();
}, [audioRef?.current?.loadedmetadata, audioRef?.current?.readyState]);
Copy link
Contributor

@destinio destinio Aug 17, 2021

Choose a reason for hiding this comment

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

audioRef appears to be a required prop so it will always be there no need for the first ?

audioRed.current?.load.......

The ? only needs to be on properties that we have no clue if they are there or not.

I actually just learned this from James' optional chaining video

…n query param and jump to time without auto play. Duration isn't displaying initially
@vercel
Copy link

vercel bot commented Aug 31, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
compressedfm ❌ Failed (Inspect) Aug 31, 2022 at 5:11PM (UTC)

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.

Finish setting up Jump Links on the Individual Episode Page
3 participants