wordpress-mobile / WordPress-Android

WordPress for Android
http://android.wordpress.org
GNU General Public License v2.0
2.92k stars 1.3k forks source link

Custom notifications filter layout with wrapping Fixes #3481 #3749

Closed hypest closed 8 years ago

hypest commented 8 years ago

This is a custom, RadioGroup-like layout that allows the filters list to wrap if space is not enough for the filter names.

Unfortunately, the filter borders are emulated (using margins and a background colour for the parent container) and support for dedicated border for the selected filter is not supported. Instead, the background of the selected filter is made a bit more dark than it used to be (grey_lighten_30 instead of grey_lighten). For the same reason, the rounded corners of the "radiogroup" are not supported.

This PR introduces a few design changes so, it would be useful if a designer could chime into this as well (@mattmiklic maybe?).

This fixes #3481.

Happy to hear to any comments!

Here's how it looks on the Nexus 5X (with a few artificially long filter names to trigger the wrapping):

wrapping-filters-group

mattmiklic commented 8 years ago

This PR introduces a few design changes so, it would be useful if a designer could chime into this as well (@mattmiklic maybe?).

Hey @hypest happy to weigh in here but wondering if you could provide a little more context on the impetus for the change.

I'm a little bit concerned about the usability of a segmented control that wraps to two lines, because of how small the tap targets for each link will be. I think it will be very easy to mistakenly select the wrong option.

@drw158 is the designer of the Notifications section, though, so he should weigh in on this.

mattmiklic commented 8 years ago

Just saw the ticket linked in the original post.

I think having each option in the segmented control truncate is probably the correct design decision, so I don't think the best solution is to expand the control to a second line. I think there are some other ways to fix this issue but they'll require some further thought given to the design.

One thing to consider is that the segmented control is not a native control on Android. Android does have tabs that can horizontally scroll, which could be a better solution. But if we make a change here thought needs to be given to how the issue is handled on iOS, too, so I think Dave should weigh in there.

davewhitley commented 8 years ago

@jleandroperez Question: how does the iOS segmented control in Notifications handle long strings of text? Does it get truncated?

mattmiklic commented 8 years ago

@drw158 they truncate; here's a screenshot of the app in Turkish on an iPhone 5:

simulator screen shot feb 15 2016 12 25 14 pm

jleandroperez commented 8 years ago

@drw158 just stepping by to agree with @mattmiklic. Untested scenario there. Perhaps the cleaner solution is to detect such scenario, and reduce the number of filters.

Not absolutely happy with it, but just a thought!

mattmiklic commented 8 years ago

Would it be reasonable to try to reduce the font size when text overflows?

jleandroperez commented 8 years ago

It's just a flag. But it'd look awful, most likely

mattmiklic commented 8 years ago

But it'd look awful, most likely

If it only resized the segments that overflowed, yeah, it would lead to a ransom note kind of look. If all of the segments were the same smaller size, it would work. If that's not possible I wouldn't try the resizing route, though.

jleandroperez commented 8 years ago

Agreed. iOS Issue filed here, i'll try to squeeze it into Release 6.1.

Thank you both =)

davewhitley commented 8 years ago

I'd be open to try tabs for Android, although I imagine it would be a bit of time to get it working

mattmiklic commented 8 years ago

I'd be open to try tabs for Android, although I imagine it would be a bit of time to get it working

It could be strange, too, since there's already a row of tabs -- I'm not sure if there's any precedent for two rows of tabs in material design. This is a tricky place to strike a balance between native UI elements and the structural needs of our app.

nbradbury commented 8 years ago

It could be strange, too, since there's already a row of tabs

I agree that would be strange. A few other options:

Personally, I wouldn't mind replacing the segmented control on Android since it feels very iOS-ish.

roundhill commented 8 years ago

Use icons for each of the filters

+1, I think we have an icon representation for most of the filters

davewhitley commented 8 years ago

It could be strange, too, since there's already a row of tabs -- I'm not sure if there's any precedent for two rows of tabs in material design.

I think if we used text for notification tabs, it would feel fine since the main navigation tabs use only icons. But we would have to figure out the swiping gestures.

I think it would feel strange to have 2 rows of icon tabs (the action bar, and the notification filter).

nbradbury commented 8 years ago

I think it would feel strange to have 2 rows of icon tabs (the action bar, and the notification filter).

You're right - I rescind that suggestion :)

roundhill commented 8 years ago

Maybe we could show the options in a drop down... but only if we detect that the strings are too long to fit properly?

hypest commented 8 years ago

Great discussion! Thanks for chiming in guys!

There's been some interesting suggestions already but all seem to be a compromise and an attempt to "squeeze" the filters somehow. I personally feel that the issue here is the existence of the filters in the first place. If we're going to depart from the current solution (segmented controls) we might as well consider the alternative of losing the filters altogether.

Lemme elaborate: I presume we offer filters in an attempt to give the user a way to tame a big list of notifications - if the list is short, there is no real need for filtering it anyway. Being a timeline, the notifications list's value mostly lives at its top where the most recent events are displayed and so, we should "protect" the first few slots by trying to deliver enough value in the first, say, 5 of them.

I think a good way of doing that would be by grouping the notifications to behave more like gmail threads (a new response brings the thread to the top) or the FB notifications (grouping per event type and/or post) rather than just individual events/notifications. I think Calypso already does that grouping (even though it also offers the filters as well). Perhaps users with high volumes of recent notifications will going to need something more than smart grouping, but that's a different use case that would warrant a more sophisticated solution.

To me, filters feel like a "desktop" pattern, in the sense that instead of solving the problem (big list/timeline) we just drop more "buttons" to the user to do the job on her own.

I can be very wrong here. Anyone has some research at hand that shows that users actually flip often between the filters? It doesn't seem to track that event on WPAndroid.

Until a really better solution sees its way to a PR, I kinda like the solution the current PR offers: it builds on top of the current pattern by naturally extending it and it's solving the issue and restores clarity only for the users that have the truncated filter texts. Except for the rounded corners and the dark border, everyone else sees no change in functionality or UX. I'm not against seeing any other solution being implemented. I just think that this PR is a good-enough fix and doesn't prevent us from working on redesigning the feature :)

nbradbury commented 8 years ago

@hypest I agree with many of your points. If we're going to have a filter, I'd prefer to see a different solution than a segmented control. I think @roundhill could tell you I was a bit grumpy when it was added :)

But for this specific issue, I don't think wrapping the segmented control to multiple lines is much of an improvement, especially since it adds org.apmem.tools:layouts as a dependency.

I know you spent quite a bit of time on this so I hate to :poop: on it, but I'd prefer not to merge it. As with all things, though, I could be convinced otherwise if other Android devs feel differently.

roundhill commented 8 years ago

I think @roundhill could tell you I was a bit grumpy when it was added :)

Just think of it as a calypso component instead of an iOS one! :)

I'm not a huge fan of how the component looks with these changes, especially when it wraps. So I'm inclined to just leave it as-is, or do something more drastic like changing it to a drop-down spinner.

roundhill commented 8 years ago

Another thought, the 'Follows' filter realistically will only have one entry for the average user. I wonder how much space we'd save if we removed that one?

nbradbury commented 8 years ago

Another thought, the 'Follows' filter realistically will only have one entry for the average user. I wonder how much space we'd save if we removed that one?

:boom:

daniloercoli commented 8 years ago

Jumping on this thread without reading with much attention the whole convo above. @hypest don't get me wrong, i'm not judging the quality of your code :) Also, i'm a master on getting conversations derailed in these days, so ignore me without problems if you think my comments are superfluous. I just want to ask if we have some numbers about the usage of this feature (notifications filter) on mobile apps. I bet a simple switch all/unread is suffice on mobile for 99% of our users.

hypest commented 8 years ago

but I'd prefer not to merge it

and

So I'm inclined to just leave it as-is

@nbradbury, @roundhill no prob here. TBH, the issue at hand is not that critical and can probably be left unattended until a solution gets PR'ed that has consensus.

I bet a simple switch all/unread is suffice on mobile for 99% of our users

@daniloercoli hah, that's quite in line with what others have said and even my proposal of losing the filters altogether, so +1

maxme commented 8 years ago

TBH, the issue at hand is not that critical and can probably be left unattended until a solution gets PR'ed that has consensus.

+1

hypest commented 8 years ago

Closing this PR as a no-go. Feel free to open a new PR with a different proposal. Cheers!