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.

Sign up for GitHub

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

Jump to bottom

[Refactor] Shift the audio playing logic for the Effects to the useAudioPlayer custom hook #15

Open
ArvindParekh opened this issue May 11, 2024 · 7 comments · May be fixed by #29
Open

[Refactor] Shift the audio playing logic for the Effects to the useAudioPlayer custom hook #15

ArvindParekh opened this issue May 11, 2024 · 7 comments · May be fixed by #29
Assignees
@qu1ll @sahild91
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@ArvindParekh
Copy link
Owner

Currently, the audio-playing logic for the Tracks is consolidated within the useAudioPlayer custom hook. However, the same logic for Effects is not centralized and is present individually within the Effect.jsx file. To maintain a consistent and centralized source of truth, it's advisable to include the audio playing logic for Effects within the useAudioPlayer custom hook.

The task involves:

  • Modifying the useAudioPlayer custom hook to incorporate the audio playing logic for Effects.
  • Adjusting the Effects.jsx and EffectCard.jsx components to accommodate this change, ensuring there's no loss of functionality.
@ArvindParekh ArvindParekh added enhancement New feature or request help wanted Extra attention is needed labels May 11, 2024
@ArvindParekh ArvindParekh self-assigned this May 11, 2024
@tigerabrodi
Copy link

@ArvindParekh

Do you mean EffectCard.jsx should also use the useAudioPlayer hook just like Tracks file does?

@ArvindParekh
Copy link
Owner Author

Yes, precisely. However, the useAudioPlayer hook currently doesn't support the kind of logic that's required for playing "Effect" audios, the logic is still present natively in EffectCard.jsx. It'd be better if there's one single custom hook that takes care of the entire audio logic for both Effects and Tracks.

@ArvindParekh ArvindParekh removed their assignment May 11, 2024
@sahild91
Copy link

@ArvindParekh ,
Is this issue still open for contribution?
If yes, maybe I can try it out and see if it can be made as a custom hook.

@ArvindParekh
Copy link
Owner Author

Hey @sahild91, yes the issue's still open, feel free to work on it if you'd like. We already have a custom hook in place, we'd just like to incorporate the logic of Effects into it.
(You can get the issue assigned to you, and here are the contributing guidelines if you'd like to take a look 👍🏻 )

@sahild91
Copy link

/assign

@qu1ll
Copy link

qu1ll commented May 31, 2024

Hey @sahild91, just wondering how far you got on this issue, I was thinking of taking a peak at it see what I can do.

@qu1ll
Copy link

qu1ll commented Jun 1, 2024

/assign

@qu1ll qu1ll linked a pull request Jun 1, 2024 that will close this issue
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@qu1ll qu1ll

@sahild91 sahild91

Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Modify audio playing logic for effects to work with custom hook qu1ll/Lofi-Music-App
4 participants
@qu1ll @tigerabrodi @ArvindParekh @sahild91