MOBILE-213: Implementation of CreatedForYou screen #535
Conversation
Added heading composable and renamed a few other data classes
User can retry fetching playlist if the playlist was not fetched initially
…mishra/listenbrainz-android into feat-createdForYou-hemang_mishra
There was a problem hiding this comment.
Hi @hemang-mishra, fantastic work! some things need to reworked but overall amazing work!!
| * @param onReorderClick Lambda to be executed when the reorder button is clicked. | ||
| */ | ||
| @Composable | ||
| fun PlaylistTrackComposable( |
There was a problem hiding this comment.
Please use ListenCardSmallDefault instead of creating new cards, it is very hard to maintain duplicates + consistency is sacrificed. What every extra options you many want to show, show it in trailingContent composable lambda.
Also, please do not add the play icon, rather play the song when card click, i.e., when onClick is invoked.
| } | ||
|
|
||
|
|
||
| fun formatSeconds(seconds: Int): String { |
There was a problem hiding this comment.
Put functions like these in utils whenever possible :)
| val context = LocalContext.current | ||
| val scope = rememberCoroutineScope() | ||
| val socialUiState by socialViewModel.uiState.collectAsState() | ||
| CreatedForYouScreen(uiState = uiState, |
There was a problem hiding this comment.
Good work with this abstraction, just add a preview and it would be chef's kiss.
| else uiState.createdForTabUIState.createdForYouPlaylists[0].playlist | ||
| ) | ||
| } | ||
| var dropdownItemIndex by remember { |
There was a problem hiding this comment.
No requirement for this variable when we use ListenCardSmallDefault.
...ain/java/org/listenbrainz/android/ui/screens/profile/createdforyou/PlaylistSelectionCards.kt
Show resolved
Hide resolved
|
@07jasjeet I have made the required changes. |
There was a problem hiding this comment.
Hi @hemang-mishra, you are doing high quality work and I really appreciate it. There are changes regarding some performance issues that I've pointed out. Also, the empty and retry states look half baked. With a bit of center alignment, the page will look polished!
| onSaveClick: (CreatedForYouPlaylist) -> Unit, | ||
| onPlaylistSelect: (CreatedForYouPlaylist) -> Unit | ||
| ) { | ||
| Row( |
| } | ||
| ) { | ||
| Text( | ||
| text = removeExcessiveSpaces(removeHtmlTags(description)).trim(), |
There was a problem hiding this comment.
Wrap this in remember with description as key
| ) { | ||
| playlists.forEachIndexed { index, playlist-> | ||
| PlaylistTitleCard( | ||
| title = modifyTitle(playlist), |
There was a problem hiding this comment.
remember with playlist as key
| playlists.forEachIndexed { index, playlist-> | ||
| PlaylistTitleCard( | ||
| title = modifyTitle(playlist), | ||
| fractionLeft = getFractionLeft( |
There was a problem hiding this comment.
use remember here with necessary keys
There was a problem hiding this comment.
Hi @hemang-mishra, did some changes on top of your work to polish some areas. Changes LGTM, good work!
|
Thanks for the contribution @hemang-mishra! |
In this PR, I have added the created for you screen in the profile tab.
Ticket
Changes Made and features implemented:
PlaylistServiceand repository calledPlaylistDataRepositoryfor actions related to fetching and saving playlists. API calls related to Playlist screen will also be made in these files.removeHtmlTagsfunction to Util package as it was being used inCreateForYouScreenas well.identifierof the playlist will be shared.Screenshots:
ScreenRecordingCreatedForUScreen.mp4