Feat: BP Revamp in LB-Android Part 1 (Completed Overview Screens) #382
Conversation
Earlier the search icon in the top bar used to be a global search in the app for users , now in the BP page , the search icon will be used to search local songs
Also added previews for albums and artists on main overview screen
pranavkonidena
changed the title
|
@pranavkonidena the app crashes as soon as I load this branch |
app/src/main/java/org/listenbrainz/android/ui/screens/brainzplayer/BrainzPlayerScreen.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/listenbrainz/android/model/AppNavigationItem.kt
Outdated
Show resolved
Hide resolved
| @@ -4,7 +4,7 @@ import androidx.annotation.DrawableRes | |||
| import org.listenbrainz.android.R | |||
|
|
|||
| sealed class AppNavigationItem(val route: String, @DrawableRes val iconUnselected: Int, @DrawableRes val iconSelected: Int, val title: String) { | |||
| object BrainzPlayer : AppNavigationItem("brainzplayer", R.drawable.player_unselected, R.drawable.player_selected, "Player") | |||
| object BrainzPlayer : AppNavigationItem("brainzplayer", R.drawable.player_unselected, R.drawable.player_selected, "Local Player") | |||
There was a problem hiding this comment.
@pranavkonidena Rename this to Player
|
@pranavkonidena Looking at the design here, I am not sure if artist should always be the primary text. For albums, album name should be primary and artist name secondary. Same for song |
|
@akshaaatt I have made the change for albums @However, for recently played, the design mandates artist as the primary title. I will confirm this from Aerozol once and update as needed |
|
@akshaaatt whenever u have time, could u pls go thru the PR again? I have made the reqd changes. |
| title = album.title, | ||
| artist = album.artist, | ||
| title = album.artist, | ||
| artist = album.title, |
There was a problem hiding this comment.
This looks wrong to me now. Passing the album title as the artist field name. We should rename the parameters to something more generic
There was a problem hiding this comment.
@pranavkonidena I am merging this PR rn since we are doing a cleanup. Please fix this in the next PR.
There was a problem hiding this comment.
Alright @akshaaatt ! Thanks will make sure these are fixed in the next PR
This PR aims to revamp the existing BP with the new figma and features as given on the GSoC Ideas Page. The updated figma file can be found here