nextcloud / news-android

📱🗞️ Android client for the Nextcloud news/feed reader app
https://play.google.com/store/apps/details?id=de.luhmer.owncloudnewsreader
GNU Affero General Public License v3.0
697 stars 258 forks source link

Better handling of the star/fav button #652

Closed stefan-niedermann closed 6 years ago

stefan-niedermann commented 6 years ago
  1. The star/favorite button should be on the left side and use full height of the row (not as now only the height of the headline)
  2. Use yellow for true value and filled grey for false value, do not use outlined version
  3. Exception from 2: Keep the outlined white version and the white version in the toolbar exactly as it is now, do not use colors there or change anything ;-)

Reasons: Files and notes and the webinterface always have the star at the left side and check youre swipe direction: from left to right marks as favorite, therefore the star should be left ;-)

David-Development commented 6 years ago

Thanks for the input! 👍

@jancborchardt what are your thoughts on 1. ? I'm not sure if this will look good when the control is on the left side.. In the files app it's some kind of overlay of the file icon - and you have to open a menu to mark it as favorite. In the news app we need to have this control right there (visible).

AndyScherzinger commented 6 years ago

In the files app we decided to not have it on the left side because of material design / layouting issue since the "standard" grid alignment of a list would be broken by that, thus we decided to stay with the overlay

David-Development commented 6 years ago

@AndyScherzinger Great, thanks for the input! 👍I'm working on updating the design of the news app to be more consistent with the material design. I guess removing it completely would be a little extreme for my use-case though.. However I'm planning on adding thumbnail support for articles. Maybe we can combine that somehow..

AndyScherzinger commented 6 years ago

@David-Development sounds great! In case you need a hand with UI/Layouting stuff, just ping me and I am happy to help!

David-Development commented 6 years ago

@AndyScherzinger Thank you 👍

I pushed my latest changes (https://github.com/nextcloud/news-android/pull/658). However I'm not really satisfied with the design I've came up with yet. Maybe you guys can throw in some ideas?

Here's what it looks like: (click for full-size image)

I feel like we're giving a lot of space away here.. My design ideas were taken from the google news app.

Maybe it would be nice to include this "timeline" design?

Maybe we also need to redesign the toolbar? The google apps don't have different colors for the toolbar / content anymore..

AndyScherzinger commented 6 years ago

Can you re-upload the last two screenshots? they seem to be broken :/

David-Development commented 6 years ago

@AndyScherzinger The screenshots were just linked to the play-store page of the google news app (https://play.google.com/store/apps/details?id=com.google.android.apps.magazines). In particular the screenshots with the "Zeitachse" and "Lokalnachrichten".

David-Development commented 6 years ago

Alright.. second try.. however this design includes some text from the rss-item itself. Originally there was an option in the settings to enable the "extra-text" so I'm not sure if it's a good idea to show some of the content in the default view since some users might want to read the headlines without content..?

For clarification: The news app has 5 different view modes / layouts for the list:

I don't have any statistic about it but I'm wondering how many users use a different layout than the default one. I think most users just stick to the default settings.. Therefore it would make sense to improve the "default" layout and (maybe) get rid of some other layouts? (or just leave them as they are..)

AndyScherzinger commented 6 years ago

Looks nice @David-Development :) Some minor issues I found looking at the code and screenshots:

As for the discussion about provided layouts I guess most users will stay with the default layout so yes that should be the optimized version as for the other, remaining layouts I think at least some of them should be removed in case they just offer a different design, e.g.

Hope this helps :)

AndyScherzinger commented 6 years ago

@David-Development kind of like this: light_extended

AndyScherzinger commented 6 years ago

it would still need some more alignment optimizations e.g. star icon / 3-dor menu aren't aligned but for that the item layouts need to have the left/right margins instead on the list element itself, that way also the slider would be on the far right of the screen where it should be :)

David-Development commented 6 years ago

@AndyScherzinger thank you for your extensive feedback! I agree.. I moved the margin to the list itself (to make the divider line smaller) - looks like you got rid of it completely! But I noticed some issues with swipe / scrolling (slider looks misplaced). I'll fix it ;)

Btw: I updated your post above to include "todos" that we can check as done :)

David-Development commented 6 years ago

@AndyScherzinger not quite sure if I understand the following comment correctly, maybe you can clarify it a little? :)

fav icon should be aligned top to always be on the same line at the top line of the headline, same would apply for the play button which should be aligned top with the summary text

star icon / 3-dot menu aren't aligned

True, but how can we fix that? I mean we need to have at least 16dp spacing to the right side.

AndyScherzinger commented 6 years ago

Yeah, I lost the devider but not on purpose. Just didn't have the time to restore it, great that you could preserve it!

As for Fav icon and top alignment. Right now the Fav icon is aligned centered vertical thus the position differs based on the number of lines the headline consumes. If it is aligned too it'll always be aligned with the first line of the headline and will also always have the same distance to the divider line.

As for the play button, not sure where to put it, do you have a screenshot of how it looks like with the latest changes?

As for the Fav icon spacing on the right, I'd say we would be fine with not having 16dp exactly.

AndyScherzinger commented 6 years ago

One idea for the play icon: make it an overlay on top of the left aligned image, like we do with video thumbnails in the files client. That way you don't need extra space and also the layout would look exactly the same compared to non podcast items :)

David-Development commented 6 years ago

I lost the devider but not on purpose. Just didn't have the time to restore it, great that you could preserve it!

Oh okay! 😬 I think it looks better without though! I removed it now too! (well at least uncomment the line where I enabled it).. The google "sms/messages" app doesn't use divider lines anymore either..

aligned centered vertical thus the position differs based on the number of lines the headline consumes..

if I get this right, the thumbnail image is aligned under the headline. The headline has (by default 2 lines). The user can however adjust the lines he wants to see in the settings. So the "textview" for the headline is always 2 lines in height but it'll center the text in the middle of the two lines if it only needs one line. Does that make sense? Since I'm setting the number of lines in the code at runtime, the view will have this height, no matter if it needs it or not. Also I'm not sure about how the view recycling will work if we use this dynamically? (sometimes one line, sometimes two lines) - maybe my concern is a little outdated here but back in the days with the listview I was running in some issues with this..

There is one problem.. I'm adding some rounded corners to the images by using the Universal Image Loader Library.. However the images are not getting croped (they're kind of streched or "gestaucht")..

Also I'm wondering if the default theme should be light? (right now the default is dark mode)..

Current state of work:

AndyScherzinger commented 6 years ago

No divider looks good to me :+1:

I did a grid on top of your screenshot so it is easier to explain: grid

As for the default theme, yes I'd also think the light theme should be the default one.

Here are the changes I made to have a flexible headline + fixed fav icon positioning: screenshot_nextcloud_single_sign_on_20180920-141829 commit: https://github.com/nextcloud/news-android/commit/0c6dfd44533d585f9c4c4159469370d98aab6d86 on a separate branch :)

David-Development commented 6 years ago

@AndyScherzinger works like charm! Thank you!! 👍 And it looks great!

David-Development commented 6 years ago

I update the detail view a little to make it look more "modern" as well. (And I got rid of the colored bar on the left side of the header). Unordered lists now have the ">" icon instead of the bullet point html design.

Here are some screenshots:

New Dark/Light Theme for detail view

Unordered list design

AndyScherzinger commented 6 years ago

Looks great! Only thing I found is that for the headline and the meta data line is that the left padding seems to be more than the 16dp which is why it isn't aligned with the rest of the content below it and it also thus isn't aligned with the back navigation arrow in the toolbar.

Like I said my guess is the padding is to large.

David-Development commented 6 years ago

@AndyScherzinger Just fixed the padding issues you mentioned. I tested the other/old layouts and they're still working just fine. I also removed the "title-line-count" option as I think it isn't used by many users anymore and it isn't supported by our new default layout.

Thank you for your help and feedback! :) Closing this..

PR: https://github.com/nextcloud/news-android/pull/658

AndyScherzinger commented 6 years ago

My pleasure, happy to contribute :)

jancborchardt commented 6 years ago

Sorry for the late reply, was on vacation in december. :) The last screenshot of the detail view looks good! I'd also say in the list view the feed title (favicon, site name and date) should be below the article title and image. :)