chinitacode / jamming Goto Github PK

(Important) Don't modify state directly, splice changes the array in place. React specifically mentions this in their docs. Instead, create a new array containing the new state you would like and then pass it to setState.

(Meets Expectations) Overall, nice work.

Remember to use more descriptive commit messages that summarize the changes that were made. Also, remember not to modify state directly in React components, instead create new objects with the desired state and use setState to update state. Again, env variables are common in most projects, it would help to become familiar with them.

Otherwise, your code is clean and easy to follow, nice work!

However, this also brings a little trouble if users remove a song from the playlist and want to add it again, they have to research it because the song is no longer in the search results list. To solve this problem, a new functionality should be added to the .removeTrack() in the App component to add the removed song back to the search results list. But this also has a potential problem: the song can only be added back to the present search results list which may not be of the same category as its original search results list. Adding this functionality can cause more troubles than not to, and for this reason we choose not to.

Perhaps having a track removed from the searchResults might be a net-negative user experience? If a user must be so fully committed to having a song in a playlist that they can't easily remove it, or there are side effects to removing it, then there's definitely an issue.

A user might remove a track from a playlist just to change the playlist's track order, but not being able to see a track at all after removing it once is definitely a caveat.

(Important) This is a great place for an environment variable. There is a project called dotenv that makes this convenient!

1. Currently, Jamming forbids users to trigger search action by simply pressing Enter key in the keyboard but by clicking the search button only, thus bringing inconvenient user experience;
2. Each track information is text-based, making the web app looks plain and boring. Adding album image provided by Spotify API can help improve users’ general visual experience;
3. Jamming allows users to add songs from search results and prevent them from adding the same song repeatedly to the playlist, but the search results list stays unchanged. It can be improved by adding the functionality to remove each song from search results right after it is added to the playlist to improve user experience.

You'll never go wrong when designing and proposing features if you can always justify them with an improved user experience. 👍

(Important) It's good to see you making multiple commits, however, when writing commit messages try to summarize the changes that were made in around 50 characters. Keep it short, descriptive, and concise. For good examples of commits, look at the commit history of popular projects like React, Node, or your favorite open source project.

1. To allow users to press Enter to trigger search;
2. To display album picture along with track information in both search results and in the playlist;
3. To remove the song from search results once it is added to the playlist by users, preventing them from repeating the adding action.

Three-for-one, how fun! 👍

Satisfactory

You've written a proper proposal, and have done the work of coming up with potential caveats to your proposed solution... But you may have arrived at the wrong conclusion of which solution is the simplest and most feasible, or whether the tradeoff of implementing a certain solution ultimately benefits the user.

When in doubt, avoid the solution that requires duplicating code or adding event handlers unnecessarily. If a solution can be implemented simply within HTML and CSS, that's probably your best solution.

Pressing Enter to trigger search

A new customized method .enter() should be added to the SearchBar component before render. It should take event as its only parameter and inside its block, write a conditional to check whether the key property of event is equal to “Enter” using the implicit conversion operator ‘==’. Inside the conditional’s block call this.search() method, then bind .enter() with this value in the constructor and below in render, pass a prop name onKeyPress with this.enter as its value to the tag.

Did you research any other ways to accomplish this feature?

This solutions works but seems to be duplicating logic by throwing in more event listeners and handlers that all basically just call the search method. If you ever wanted to make changes to what happens before a search, you'd now likely have to update your logic in multiple locations.

Could you write a simpler solution? Perhaps an HTML-based one that doesn't require duplicate JavaScript methods?

If you use a form element, an input element of type="submit", and use an onSubmit listener, you could greatly simplify the amount of JavaScript you ship to the browser.

Displaying Album Picture

To display the album picture we need to obtain the url of the image from Spotify’s API first, therefore we need to return it in .search() method in Spotify.js by adding the code imageUrl: track.album.images[0].url, enabling us to pass the image url along with other track information to the SearchResults component and to the Track component by SearchResults. Thus now we only need to add an <img /> tag with src property whose value being this.props.track.imageUrl to the render method of the Track component. It should be placed in a new <div> tag with a className of “Track-image” between <div className="Track-information"> and the <a> tag.
Then the left to do is to style the image inside the Track.css file to make it display right in font of the <+> or <-> icon on both the search results list and the playlist.

Fair enough. Nice job researching the Spotify API to determine a simple way to implement your feature 👍

Remove tracks from search results right after it is added to the playlist

The removal functionality should be added to the .addTrack() method inside App component in App.js file and be placed inside the conditional block’s curly brackets right after updating the state of the playlist as one adds a selected track from the search results.

The functionality should as well take track as the single argument. A new variable, searchResultsList which stores the current state of search results should be created, and in the next step its value should be set to the new array excluding the track element added to the playlist. To do this, we can use .filter() method or .splice() method on searchResultsList and here we use .filter() method as example. The .filter() method should take a callback as its parameter, which takes selectedResult as its single argument and checks whether the id property of itself and track equals to each other and finally extracts the rest of the searchResultsList whose id of tracks are different from the added track. In the last step, set the state of searchResults to the new searchResultsList.

Fair enough 👍