fix(client): get sound value from redux to play sound

[nayabatir1]

Checklist:

• [x] I have read freeCodeCamp's contribution guidelines.
• [x] My pull request has a descriptive title (not a vague title like Update index.md)
• [x] My pull request targets the main branch of freeCodeCamp.
• [x] I have tested these changes either locally on my machine, or GitPod.

Closes #46372 Closes #48106

This 2nd method of solving issue. Here I've removed localstorage dependency from playTone. Sound value is grabbed directly from redux and audio is played accordingly.

[gitpod-io[bot]]

[ghost]

:eyes: Review this PR in a CodeSee Review Map

View the CodeSee Map of this change

Legend

[ilenia-magoni]

Please take a moment to give a name to the PR that follows the guidelines: https://contribute.freecodecamp.org/#/how-to-open-a-pull-request?id=prepare-a-good-pr-title

[nayabatir1]

Please take a moment to give a name to the PR that follows the guidelines: https://contribute.freecodecamp.org/#/how-to-open-a-pull-request?id=prepare-a-good-pr-title

I've updated pr title

[nayabatir1]

I did some research and found that audio can be played without any dependency. I think we should definitely use this method

const audio = new Audio(toneUrls[state]);

const storedVolume = (store.get('soundVolume') as number) ?? 50;
const calculateDecibel = storedVolume / 100;

audio.volume = calculateDecibel;

await audio.play();

[raisedadead]

You do not need to work on the other PR (the other solution). Just wait on some conclusive comments from others.

This is likely the version we want. You are welcome to hang on to the other if you prefer - but if there is anything the maintainers like, that is less noise.

[nayabatir1]

You do not need to work on the other PR (the other solution). Just wait on some conclusive comments from others.

This is likely the version we want. You are welcome to hang on to the other if you prefer - but if there is anything the maintainers like, that is less noise.

Sorry if it bothers you. I found a better solution so I thought why not implement it.

[ojeytonwilliams]

https://github.com/freeCodeCamp/freeCodeCamp/pull/48106#pullrequestreview-1146486113 applies here, too. I hadn't realised there were two PRs solving the same issue.

I found a better solution so I thought why not implement it.

The reason is that small, focused PRs are easier to understand. This makes it easier for reviewers to see if there are any issues that need to be addressed before merging. Also, if we later find that there are problems, it's easier to debug them.

[raisedadead]

Sorry if it bothers you.

That's not the problem.

The issue is you have two PRs, making it hard for multiple people to review your work. You may be working on a couple of issues; the maintainers deal with a dozen PRs. It is easy for us to lose context.

We get that you have two different approaches, and the simplest thing to do in such cases would be to keep the discussion on the most favored approach (this PR) and not work on the other one until you are asked to.

It would have been even better to get a confirmation on the main issue before you started – but we are past that.

Let's focus on working on the review of this PR.

[naomi-lgbt]

I did some research and found that audio can be played without any dependency. I think we should definitely use this method

The purpose of using Tone specifically was the ability to easily generate dynamic audio in the editor - while it would be possible to do so with the raw Audio context, IMHO it's not worth the overhead.

[nayabatir1]

@ojeytonwilliams can I create checkout of this branch for removing tone package. Checkout of main branch will result in merge conflict as both branch modify same function playTone

[ojeytonwilliams]

Sorry, @nayabatir1, I hadn't realised why we were using Tone specifically. @naomi-lgbt understands this issue better than I do, so I will defer to her.

For this PR, please could you keep the changes to those needed to fix the original issue, https://github.com/freeCodeCamp/freeCodeCamp/issues/46372? There's no need to re-arrange the imports or change to statically importing to address that.

[nayabatir1]

@naomi-lgbt I looked into all the mentioned files. All functions must be converted to React components in order to access redux store values. For simplicity, I think we should consider this pr

[naomi-lgbt]

We can certainly consider that approach, though we'd need to not remove Tone in the process.

[nayabatir1]

We can certainly consider that approach, though we'd need to not remove Tone in the process.

@naomi-lgbt I've updated pr.