acumulus8 / jammming

Spotify playlist maker. Works with Google and Spotify to authorize, search Spotify songs, make and save playlists to your account.
https://jammming-proj.netlify.app/
0 stars 0 forks source link

Summary #1

Open farishkash opened 6 years ago

farishkash commented 6 years ago

Overall met expectations.

The project is working properly.

Great job getting this up and running. I am sure you worked hard on this.

There were no errors for me to deconstruct which made this a really easy review.

I believe that you and I have interacted a few times to go over some of your javascript/react issues in the advisor chat.

My only concern which I don't believe is worthy of an issue is code formatting.

I don't know what your overall goals. If you are planning on showcasing your code for freelancing, subcontracting, or job hunting, I believe that presentation matters.

As a general rule, I follow the Airbnb style guide.

https://github.com/airbnb/javascript/tree/master/react

This is opinionated, so if it isn't part of your goals or you don't feel it isn't necessary that is ok too.

A couple examples from your app.js that I reformatted for you to reference.

Your state in the constructor.

 this.state = {searchResults: [],
                  playlistName: '',
                  playlistTracks: []
                }

As a general rule whenever you have an opening bracket, you want to drop the code onto the next line.

Reformatted below.

this.state = {
      searchResults: [],
      playlistName: "",
      playlistTracks: []
    };

Your render section.

render() {
    return (
      <div>
        <h1>Ja<span className="highlight">mmm</span>ing</h1>
        <div className="App">
          <SearchBar onSearch={this.search} />
          <div className="App-playlist">
            <SearchResults searchResults={this.state.searchResults} 
                          onAdd={this.addTrack}
                          onRemove={this.removeTrack} />
            <Playlist playlistName={this.state.playlistName} 
                      playlistTracks={this.state.playlistTracks} 
                      onRemove={this.removeTrack}
                      onNameChange={this.updatePlaylistName}
                      onSave={this.savePlaylist} />
          </div>
        </div>
      </div>
    );
  }
}

Each prop of a multi-prop jsx component drops to the next line . Reformatted below.

render() {
    return (
      <div>
        <h1>
          Ja<span className="highlight">mmm</span>ing
        </h1>
        <div className="App">
          <SearchBar onSearch={this.search} />
          <div className="App-playlist">
            <SearchResults
              searchResults={this.state.searchResults}
              onAdd={this.addTrack}
              onRemove={this.removeTrack}
            />
            <Playlist
              playlistName={this.state.playlistName}
              playlistTracks={this.state.playlistTracks}
              onRemove={this.removeTrack}
              onNameChange={this.updatePlaylistName}
              onSave={this.savePlaylist}
            />
          </div>
        </div>
      </div>
    );
  }
}

Good luck on your endeavors.

acumulus8 commented 6 years ago

Thanks for the suggestion! I agree with your opinion and will definitely check out that style guide.

Would like to get a job as a web developer/designer with some freelance work, so this will definitely be useful!