-
Notifications
You must be signed in to change notification settings - Fork 0
[URH-19] useSound 신규 #37
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 노래 듣는 걸 참 좋아하는데요 그래서 이런 audio관련 훅이 더 반가운것 같습니다!
이미 많은 기능을 제공하고 있는 훅이지만.. 코드 리뷰를 하기 전에 빠르게 기능 추가를 제안드리고 싶어 먼저 코멘트를 남겨봅니다.
- 용도가 음악재생일 경우,
02:20 / 3:52
와 같은 형태로 활용할 수 있도록 현재재생시간, 전체재생시간을 제공 - 용도가 bgm재생일 경우, 음소거를 켜고 끄는 기능을 따로 제공
- 다른 용도 ETC...
이 useSound훅을 어느 정도의 범위를 염두에 두고 만드셨나요? 일단 이렇게 제가 생각한 범위에서 필요한 기능을 더 제안드리고 싶습니다. 특히 음소거의 경우 용도가 음악재생이든, BGM이든 추가됐을 때 유용하게 쓰일 수 있는 기능이라고 생각합니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다 👍
|
이부분은
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
멋져요 지예 님!
코딩컨벤션 내용 댓글 달았습니다. 확인 부탁드려요!
✨ feat: useSound 타입 추가 ✨ feat: useSound 테스트 코드 추가 ✨ feat: volume, playbackrate 유효성 검사 추가 ✨ feat: 다양한 format에 대해 테스트 코드 추가 ✨ feat: 오디오 파일 로드 에러 처리 ✨ feat: state들을 reducer로 관리하도록 처리 ✨ feat: setValidRange 함수 warning 문구 변경 ✨ feat: optional chaining으로 함수 호출 📝 docs: jsDoc 수정 ✨ feat: 음소거 기능 추가 ✨ feat: isMuted 리턴하도록 추가 🔨 refactor: 코드 정리 🐛 fix: currentTime은 setupAudioElement에서 제거 🩹 chore: 코딩컨벤션에 맞게 수정 ✨ feat: 테스트 코드 추가
e833c70
to
f608450
Compare
👾 Pull Request
1️⃣ Spec
2️⃣ 변경 사항
3️⃣ 예시 코드
4️⃣ 관련 문서 (선택 사항)