Gwindow / WhatAndroid

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

[WIP] Hide the art container if images are disabled #43

Closed stuxo closed 9 years ago

stuxo commented 9 years ago

Extremely minor code change, but it does look quite nice (in my opinion). Most of my app usage is on mobile data and browsing the forums is a bit of a pain because of all the wasted space in a post if avatars are disabled.

With this change, if you then enable images, it makes the text view feel very small again. I might experiment with text wrapping around the avatar, or starting the text below the avatar or perhaps moving the avatar entirely.

Let me know what you guys think.

Twinklebear commented 9 years ago

Cool! Does this handle the case where images may be disabled/enabled while in the view? For example the user is viewing some comments with images disabled, goes into settings and enables images, do we properly re-show the view?

Having the text wrap around the avatar was the solution I was hoping to put together but didn't have time to come back to it. I think having the text below the avatar might leave too much empty space since next to the avatar will all be blank. If there's a better way to layout the comments I'm open to ideas though.

stuxo commented 9 years ago

Since it only checks if (imagesEnabled) when the activity loads, the updated setting will only be visible when loading another activity or reloading the current one. So in the example of the forums, if they were in a thread and changed the setting, they would have to leave the thread and enter it again to see the change.

As for the wrapping, its likely we would have to use something like this. I do hate adding dependancies, but it looks reasonably promising. I'll give it a go anyway, to see how it looks. I'll post results :)

Edit: forums

This is just a rough implementation. It doesn't seem to render correctly.

Twinklebear commented 9 years ago

Manually merged in the remaining changes from this PR to avoid dealing with more merge conflicts :stuck_out_tongue: . Thanks @stuxo