Implemented music player #35
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Hi, @heysagnik! I have to migrate this code to |
|
Ok 👌 |
|
|
||
| function App(): JSX.Element { | ||
| const [view, setView] = React.useState(false) |
There was a problem hiding this comment.
Let's make it more clear.
If we are using view, then its type should be an enum value instead of a boolean because we want to make the view more declarative.
Here, we are just using the state if we want to show music player or not and hide or show it. We can create a custom hook named useVisibilityToggle for it and use it here
const useVisibilityToggle = (initialState: boolean): {
isVisible: boolean;
show: () => void;
hide: () => void;
toggleVisibility: () => void
} => {
const [isVisible, setIsVisible] = React.useState(initialState);
const show = useCallback(() => {
setIsVisible(true);
}, []);
const hide = useCallback(() => {
setIsVisible(true);
}, []);
const toggleVisibility = useCallback(() => {
setIsVisible(visible => !visible);
}, []);
return { isVisible, show, hide, toggleVisibility }
}
| @@ -58,4 +64,17 @@ const items: ItemType[] = [{ | |||
| "link": "https://telegram.me/heysagnik" //Telegram Pofile | |||
| }] | |||
|
|
|||
| export default items | |||
| const songs: SongType[] = [ | |||
There was a problem hiding this comment.
This contract looks good for the package. @heysagnik do we need to give a default song player? I don't think that is required. If we do that, we will have to find an extension to bundle mp3 files with webpack. It's better to leave that head ache for the consumers
|
|
||
| export default function MusicPlayer(props: any) { | ||
| const [isPlaying, setIsPlaying] = useState(true) | ||
| const [isPlayAnime, setIsPlayAnime] = useState(false) |
There was a problem hiding this comment.
looks like a redundant state. Why can't we govern the same behaviour with isPlaying state?
| const [isPlayAnime, setIsPlayAnime] = useState(false) | ||
| const totalSongs = songs.length | ||
| let songIdx = 0 | ||
| const [song, setSong] = useState({info: songs[songIdx], audioSrc: new Audio(songs[songIdx].audio)}) |
There was a problem hiding this comment.
React state should have minimal information. All the information which are kept in this state is derived information and can be retrieved easily with currentSongIndex. So, keep state for currentSongIndex only and derive this information on rerenders.
|
@roopeshsn thank you for taking out time to contribute to this project 🎉 Please resolve merge conflicts and try to move this code in the |
|
Hi, @taksuparth! Thanks for the feedback! Will make the necessary changes as said by you! |
|
Hi @roopeshsn! Any update on this? |
Implemented a minimal music player.