laurencmendoza / spotify-metrics

A full-stack, single-page application that uses Spotify's Web API to allow users to see their top tracks and artists, as well as Spotify's audio analysis for their saved playlists
https://my-metrics-app-743645bb0090.herokuapp.com/
0 stars 0 forks source link

Grow #2

Open laurencmendoza opened 1 year ago

laurencmendoza commented 1 year ago

what is one area in your code that can be improved or what is already great, but could be enhanced? Why?

This is the return statement for my playlist details page, which has the playlist tracks component, which then has the audio analysis component for each track. Separating the three components allowed me to render each track's audio analysis on one page and allowed me to separate code for the different components of the page, but as of now, my loading animation only plays while the playlist name and image are loading at the top of the page, and there is a separate loading animation for the audio analysis for each track. Ideally, I'd like one loading animation for the whole page, while all of the information is getting pulled from the API, including the audio analysis. I tried finding a way to refactor it, but I was unsuccessful so far.

return (
        <div className="2xl:flex 2xl:justify-center">
            {playlist ? (
                <div className="my-10 sm:m-10 sm:flex justify-center">
                    {playlist.images[0] ? 
                    (<img className="h-[240px] mx-auto sm:mx-0" src={playlist.images[0].url} alt={playlist.name}/>) 
                    : (<img className="h-[240px] mx-auto sm:mx-0 bg-[white]" src="https://cdnweb.anghami.com/web/assets/img/placeholders/playlist-placeholder.png"/>)}
                    <div className="m-10">
                        <h1 className="text-center mb-10 text-[1.5rem] sm:text-[1.75rem] font-bold">{playlist.name}</h1>
                        <a href={playlist.external_urls.spotify} target="_blank">
                            <button className="block mx-auto text-[white] font-bold bg-spotify-green rounded-3xl h-[3rem] w-[210px] py-0 px-[24px] divide-none hover:scale-[1.02]">
                                LISTEN ON SPOTIFY
                            </button>
                        </a>
                    </div>
                </div>
            ) : (
                <div className="text-center">
                    <Loading/>
                </div>
            )}
            {playlistTracks &&
                <PlaylistTracks tracks={playlistTracks.items}/>}
        </div>
    )
}
krabecb commented 1 year ago

Great work on this return statement! Modularity is good here, but I wonder if the details page could handle the playlist name, image, and audio analysis as well? Perhaps when all the data finally comes back (I'm assuming there are multiple fetch requests), then the one loading animation will disappear! Just a thought, but would probably involve quite the refactor. Keep us posted!