search

Gwindow / WhatAndroid

The What.CD Android App
http://gwindow.github.com/WhatAndroid/
BSD 2-Clause "Simplified" License
100 stars 20 forks source link

Material Design Overhaul / Day Night Theme [Ready For Review] #47

Closed stuxo closed 8 years ago

Twinklebear commented 8 years ago

The screenshots you posted on the forums of the light/dark themes look really nice! Let me know when this is closer to being ready and I'll do some testing as well.

Edit: I just saw you changed the title, I'll check it out :)

stuxo commented 8 years ago

I'm quite happy with it now @Twinklebear, I dropped the [WIP] from the pull request. Its feature complete, ideally the only changes I make will be based off your's and other's feedback.

Edit: If you have any suggestions as to light theme colour schemes (or even if you like the teal) please let me know, I'm a bit unsure of it.

Twinklebear commented 8 years ago

It looks like the Tags & Similar Artists headers for the torrent group description wasn't re-styled, same with various headers for a request (the artists & top contributors are updated but it seems like tags/acceptable media etc. aren't)

It also seemed to start me in the light material theme even though the setting wasn't selected. The login progress dialog also seems to flash as white when using the dark theme.

Besides those smaller things this looks really good, I think the teal is pretty nice.

stuxo commented 8 years ago

Updated @Twinklebear, please check out the changes on the Request page and the profile page (recent snatches and uploads with images enables/disabled)

Twinklebear commented 8 years ago

It looks like I still get defaulted to the light theme although the setting isn't checked, where the artist page tags/similar artists headers updated as well?

It also seems like the card views shown for uploads/snatches may not be getting updated when changing to the dark theme (they did switch previously iirc). I've been testing on Android 6

stuxo commented 8 years ago

I've been testing on android 6 too, I can't reproduce the issue where you start in light theme. It could be that the entire system is set to light theme because its day time? This should be overwritten for the app though.

You're right about the recent's in dark theme, I'll fix that and the artists headers tonight.

Edit: Updated PR with the above changes

Twinklebear commented 8 years ago

Let me know when the album description headers & artist headers are updated. I did also notice that when going to an artist page there's no loading feedback shown now, I'm not sure if these changes removed it (or maybe it was gone previously), but some loading feedback for users would be good as well. Otherwise it looks like the app is hanging/broken and showing a blank page.

stuxo commented 8 years ago

Let me know when the album description headers & artist headers are updated. This has been added I did also notice that when going to an artist page there's no loading feedback shown now Can you give me steps to reproduce this? I see a loading indicator when I do an artist search

Twinklebear commented 8 years ago

Ah so if you're directly to an artist page from the search we've already loaded them, it's a bit of an interesting thing in how the site handles searching for artists. If you search and match an artist name e.g. "the sword" the search response you get is actually the artist page, so no loading is done in the artist fragment, we just forward that data we got from the search response.

Try going to an artist from an album or similar artist list, (albums seem to be missing a loader too actually).

stuxo commented 8 years ago

Oh ok, I can see it now. I'll have a quick look tonight and see if I did anything weird to break it.

Is that the last outstanding issue before we can close this PR?

Twinklebear commented 8 years ago

I think so, I might've missed a commit when I was noticing that the album description headers & artist description page headers weren't updated.

stuxo commented 8 years ago

I have found the problem. It is because the app used to use a spinner in the AppBar for paged fragment activities. Since the migration to Toolbar, this hasn't been supported. That is why Artist and torrent group don't have indicators.

I won't have time to work on this for a while and I don't see a quick fix. I guess we can either park this PR, merge with know issue or you could have a look at it?

Twinklebear commented 8 years ago

Interesting, I thought there was also a big spinner shown in the fragment while it was loading. Yea I can take a look at this and see what happened to the loading spinner.

Twinklebear commented 8 years ago

Hey @stuxo , I checked the git blame on the ArtistFragment and it looks like we may have only ever shown progress in the menu bar, or a while back stopped showing the spinner in the fragment. So I don't think these changes removed any of that, and if you're ready to merge it I think everything looks good!

Adding back in some loading feedback to fragments missing it will have to be added as a todo.

stuxo commented 8 years ago

I just pushed a fix for another missing background on dark theme. I feel like there may be other obscure things found once we roll it out. I'm happy to do a minor bug fix PR if a few things come back related to dark theme.

With that, I'm happy to merge.

Twinklebear commented 8 years ago

Cool, sounds good!