Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add albums #20

Merged
merged 23 commits into from Jul 14, 2016
Merged

Add albums #20

merged 23 commits into from Jul 14, 2016

Conversation

aleks-tpom6oh
Copy link
Contributor

Description

Add albums to the app.
Artist albums are shown in the activity that used to be called DetailActivity, now it is called ArtistActivity.
Biography and albums are shown in a view pager with tabs similar to the approach used in the original bandhook app.
I've migrated artist activity to use design support library from ObservableScrollView, that makes the code simpler.
And there is also an activity to show an album, that looks like the image in the issue.

screenshot-2016-07-08_13 23 32 299

screenshot-2016-07-08_13 23 39 351


fun transform(artists: List<LastFmArtist>): List<Artist> {
return artists.filter { artistHasQualityInfo(it) }.map { transform(it) }
return artists.take(10).filter { artistHasQualityInfo(it) }.map { transform(it) }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Artist without mbid that makes app unusable is in the end of the list.
This is a temporary solution of that problem, that is addressed more fundamentally here #16

@antoniolg
Copy link
Owner

Thanks @pvg-alex ! I'll give a go to all your work these days.

@aleks-tpom6oh
Copy link
Contributor Author

Thank you. This issue is addressed here too btw #17


lastFmService = restAdapter.create(LastFmService::class.java) ;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the little cleanup 😳

@antoniolg
Copy link
Owner

antoniolg commented Jul 10, 2016

For some reason I get an empty screen when running the code. I can't continue today, I'll try to debug it one of this days, but any ideas will be welcomed @pvg-alex . Code looks great, so it's just a matter of taking a look and see if UI looks good too.

EDIT: Ah, ok, I see the problem is #16. So let's merge that one first, and then please rebase this branch so that it works. I did myself but there are some conflicts that I prefer you to deal with.

Alexey Verein added 4 commits July 10, 2016 21:43
Conflicts:
	app/build.gradle
	app/src/main/java/com/antonioleiva/bandhookkotlin/data/mapper/ArtistMapper.kt
	app/src/main/java/com/antonioleiva/bandhookkotlin/repository/ArtistRepositoryImp.kt
@antoniolg
Copy link
Owner

Looks great @pvg-alex ! Would it be too difficult to make the card scroll up while the background image does a parallax? I'd also agree to merge it as it is now and do it on a separate PR. so that this PR doesn't keep growing.

@aleks-tpom6oh
Copy link
Contributor Author

Thanks @antoniolg
I don't think it will be a problem to add the parallax effect to the album scene, I was considering it but then decided to not do it because often album lists are not that big and album art seemed important enough to me to keep showing it all the time. But I think we can easily add the parallax there.
Though I would like to do it separately too.

@antoniolg
Copy link
Owner

Great, as you wish. Not a big deal anyway. Merging this. Thanks again!

@antoniolg antoniolg merged commit 13cc909 into antoniolg:master Jul 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants