#228: Rename property and increase stream history length issue-number #281
Conversation
telepcak
changed the title
|
Hi @telepcak , thanks for your time and contributions. I see you did both frontend and backend parts of the issue, that's nice. Here are some things I see from the pipeline and format:
Let me know if you need anything :) |
…stream-history-length
telepcak
changed the title
…ponent, openapi generated | [AntonioMrtz#228]
…y-length' of https://github.com/telepcak/SpotifyElectron into refactor/228-Rename-property-and-increase-stream-history-length
|
Hi, @AntonioMrtz , I hopefully fixed code according to proposed changes. Just leave a comment if sth still needs a change ! |
There was a problem hiding this comment.
Hi, thanks for your time. Code looks mostly good, but there's a few things we have to fix. We wanna get rid of playback word across the project so searching for this key word and refactor the naming convention should be done :) . Remember to star the project if you like it 😃
Electron/src/components/ShowAllItems/Items/ItemsAllSongsFromStreamHistory.tsx
Show resolved
Hide resolved
| return ( | ||
| <SongCard | ||
| // eslint-disable-next-line react/no-array-index-key | ||
| key={songItem.name} |
There was a problem hiding this comment.
I would use ${songItem.name}-${songItem.artist}
| userName, | ||
| songName, | ||
| ); | ||
| } catch (err) { | ||
| console.log( | ||
| `Unable to update User ${userName} playback history with Son ${songName}`, | ||
| `Unable to update User ${userName} stream history with Son ${songName}`, |
There was a problem hiding this comment.
Can you fix this typo? with Son
| streamHistory.slice(0, 5).map((songItem) => { | ||
| return ( | ||
| <SongCard | ||
| // eslint-disable-next-line react/no-array-index-key |
There was a problem hiding this comment.
Can we delete // eslint-disable-next-line react/no-array-index-key and use songItem.name-songItem.artist ?
| @@ -30,7 +30,7 @@ const useFetchGetUserPlaybackHistory = (userName: string | undefined) => { | |||
| fetchData(); | |||
| }, [userName]); | |||
|
|
|||
| return { playbackHistory, loading, error }; | |||
| return { streamHistory: streamHistory, loading, error }; | |||
There was a problem hiding this comment.
streamHistory: streamHistory is needed?
Description
Backend variable name refractored, fixed corresponding test, proposed change to show only 5 songs on frontend
Commit type
Refactor
Issue
#228
Proposed Changes
Variable name is simply refractored and frontend change to show only 5 songs in profile history is achieved through boolean variable and button to show all songs or only 5
Potential Impact
Screenshots
Additional Tasks
Assigned
@AntonioMrtz