drum-it / jamming Goto Github PK

https://github.com/drum-IT/jamming/blob/master/src/Components/App/App.js#L24-L29

This definitely gets the job done, but it could be written a little more concisely by taking advantage of the function some().

let alreadyAdded = this.state.playlistTracks.some(playlistTrack => playListTrack.id === track.id);

This lets you abstract away the for loop and determine whether there is already some element in this.state.playlistTracks with the same id as the track being added.

https://github.com/drum-IT/jamming/blob/master/src/Components/SearchResults/SearchResults.js

The class syntax is the standard way to define a React component. But it's kind of a lot of ritual for a component that doesn't refer to this.state and only has a render() function.

In these situations, it's often preferable to write your component as a stateless functional component rather than a full blown component using class.

https://facebook.github.io/react/docs/components-and-props.html

Satisfactory 👍

First of all, good job getting this project to work! It's a smooth experience to clone your project, run the development server, and then get started searching for songs, adding them to playlists and finally adding those playlists to my actual Spotify account. This was no small feat, good job!

Make sure to look through the Spotify API documentation and think of other features that you can add to your web application. As long as you can break a user interface down to components and reason about the state that describes them, you should be able to use React to build your ideas!

https://developer.spotify.com/web-api/endpoint-reference/

https://github.com/drum-IT/jamming/blob/master/src/util/Spotify.js

Nicely done, you've clearly got a solid understanding of fetch's Promise-based API and have made good use of ES6's template strings throughout your module 👍

As a challenge to yourself, can you refactor this module more concisely using async/await? Do you have any opportunities to declare other variables, like the headers object or the API URL, outside of Spotify object so that you don't repeat code?

https://github.com/drum-IT/jamming/blob/master/src/Components/App/App.js#L23

When you write something like

let someArray = this.state.someProperty;
someArray.push();

You are directly mutating React's state. This is a bad practice, as React's state should only be changed by this.setState(). Consider using a non-mutating Array method instead, like slice, concat, map, or filter which return a brand new array instead.

You are then free to mutate that copied array and then pass the final version to this.setState().

let updatedPlaylistTracks = this.state.playlistTracks.concat(track);
this.setState({
  playlistTracks: updatedPlaylistTracks
});

https://github.com/drum-IT/jamming/blob/master/src/Components/Playlist/Playlist.js#L18

The isRemoval prop is a handy solution, but it's a little "extra".

Could your TrackList component just check for the presence of either this.props.onRemove or this.props.onAdd to decide how to render, instead?

https://github.com/drum-IT/jamming/blob/master/src/Components/App/App.js#L52-L56

When you save a playlist, the playlistTracks and playlistName should both be reset. searchResults should probably not be reset since the search could still be useful to the next playlist.

https://github.com/drum-IT/jamming/blob/master/src/Components/Playlist/Playlist.js#L17

Rather than using a defaultValue attribute, just use a value attribute. That way the value of the form will always reflect what is passed down via props.