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

Improve swiping through notifications behaviour #14919

Closed ashiagr closed 1 year ago

ashiagr commented 2 years ago

When swiping between the screens for e.g. swiping through notifications on the Notification tab, the foreground and the background screens are not distinguishable (See https://github.com/wordpress-mobile/WordPress-Android/pull/14906#pullrequestreview-690723314).

Screenshot 2021-06-24 at 9 25 03 AM

Giving such screens an elevation doesn't look great either (See https://github.com/wordpress-mobile/WordPress-Android/pull/14906#issuecomment-867333493).

device-2021-06-24-102355

The goal of this task to explore our options to further improve this swiping behavior.

ssinha2103 commented 2 years ago

I would like to work on it.

ashiagr commented 2 years ago

Thanks for your interest in this issue!

I've assigned it to you. Let me know if you have any questions on this issue, Project Setup, or the Contributing Guide.

ssinha2103 commented 2 years ago

Hii @ashiagr , can you please elaborate what is the issue, coz I am now using the app on emulator, and I cannot find any transition issues with the app. I mean, the swipe feels perfect. What is the exact change that you want ? Screenshot 2021-08-29 at 1 32 40 PM

ashiagr commented 2 years ago

👋 @ssinha2103

Thank you for trying it out 👍.

the swipe feels perfect

The only issue is that the limit between the foreground and the background screens are not distinguishable on areas without content (see https://github.com/wordpress-mobile/WordPress-Android/pull/14906#pullrequestreview-690723314). I've highlighted it in the screenshot you shared.

One way is to add an elevation to the screens (see https://github.com/wordpress-mobile/WordPress-Android/pull/14906#issuecomment-867333493) but the common app bar doesn't look great with it. Can you try anything that can improve this transition?

ssinha2103 commented 2 years ago

Hii @ashiagr , is it fine now ?

Screenshot 2021-08-29 at 9 41 19 PM Hii @ashiagr , is it fine now ?

Screenshot 2021-08-29 at 9 42 34 PM Screenshot 2021-08-29 at 9 42 49 PM

ashiagr commented 2 years ago

Looks better 👍. Can you create a PR? We can test and discuss it there.

ssinha2103 commented 2 years ago

I created One , can you please check.

ashiagr commented 2 years ago

Thank you! I'll check it tomorrow.

ssinha2103 commented 2 years ago

Hii @ashiagr Can you please check it and merge the PR.

ashiagr commented 2 years ago

We try not to add custom colors and instead reuse colors like colorSurface or colorOnSurface which are present in values/styles.xmland values-night/styles.xml. Let me know if you cannot utilize any of the existing colors.

ssinha2103 commented 2 years ago

android:background="@color/quick_start_task_card_illustration_edit_site_icon_mountain_fill" Will it be fine @ashiagr ?

ashiagr commented 2 years ago

Can you try @color/placeholder? Add a screenshot for both light and dark mode. I'll get some design feedback.

ssinha2103 commented 2 years ago

Screenshot 2021-08-30 at 12 58 58 PM Screenshot 2021-08-30 at 12 57 18 PM

Is it fine ?

ashiagr commented 2 years ago

Thanks, @ssinha2103!

👋 @osullivanchris, pinging you for a little help. Can you advise us of the right color that we're trying to use here? Currently we've used view pager background as @color/placeholder (Light: #1A000000, Dark: #1AFFFFFF) in the screenshots above.

ssinha2103 commented 2 years ago

Hii @ashiagr and @osullivanchris. currently I have modified notification_detail_activity.xml with android:background="@color/quick_start_task_card_illustration_edit_site_icon_mountain_stroke" Please guide further ? Working as shown in the screenshots above.

ashiagr commented 2 years ago

@ssinha2103, can you add screenshots for @color/placeholder as well, please? quick_start_task_card_illustration_edit_site_icon_mountain_stroke looks dark in the light mode.

ssinha2103 commented 2 years ago

Sure, here are the screenshots for @color/placeholder as well @ashiagr and @osullivanchris

Screenshot 2021-08-30 at 1 20 06 PM Screenshot 2021-08-30 at 1 20 36 PM

osullivanchris commented 2 years ago

Hi @ashiagr and @ssinha2103 ! Thanks for the ping and for the work on this!

I have a slightly different approach I'd like to suggest. Sorry to throw a curve ball in. Let me know what you think. If you don't agree, I can have a further look at the overlay colours/shadow behaviour.

Currently

Here's a sketch and a very rough video to try to clarify what I'm describing.

Screenshot 2021-08-30 at 17 39 55

https://user-images.githubusercontent.com/50576199/131374146-93a0a1c2-36a3-4574-a500-f074bd584b3f.mp4

Issues

Proposal

Screenshot 2021-08-30 at 17 45 32

https://user-images.githubusercontent.com/50576199/131374650-2794b1b3-fccd-415a-b9e6-ae6c09fbe64f.mp4

Apologies if this is much more design input than you were looking for! But it struck me as a general possible improvement. Noting that we use the same system in My Site -> Comments. And there might be native/component conventions that conflict with what I'm saying, I'm open to feedback.

ssinha2103 commented 2 years ago

@osullivanchris The issue you have mentioned is already resolved. I request you to build and try the app from my PR. Currently the notification seems to be at same level.

ashiagr commented 2 years ago

👋 @osullivanchris

Thank you so much for a detailed explanation of your thoughts on animation! 🙇‍♀️ I totally get your point. I see a slide animation similar to yours in the Gmail app when we horizontally swipe through emails and I agree it gives a better effect of screens being at the same level.

We'll try it out, and get back to you.


@ssinha2103

The issue you have mentioned is already resolved.

Not sure if you got the idea. Right now, the left screen goes deeper on the z-axis and it looks like it the right screen is sliding over it. They do not appear to be at the same level. Chris has included videos that demonstrate it very well, I'd suggest you carefully observe it.

It seems we can get the suggested sliding effect using a different PageTransformer (E.g. see screen slide in this Guide).

Can you give it a try? Let me know if it is still not clear.

ssinha2103 commented 2 years ago

Hii @ashiagr , can u merge the pr, coz something is better than nothing, and I am working on the suggestion by @osullivanchris . Kindly merge the PR and enrich the UI experience until I finish .

ashiagr commented 2 years ago

👋 @ssinha2103!

Alright, I'll merge it and we can create a separate PR for the suggested animation. Thanks for working on it!

thehenrybyrd commented 1 year ago

@ashiagr looks like this issue was addressed in #15253 , and we'd wanted to work on an animation as well. Can we go ahead and close this issue? And, is there a new issue for the animation? Thanks!

ashiagr commented 1 year ago

Thanks for the ping @thehenrybyrd!

https://github.com/wordpress-mobile/WordPress-Android/pull/15253 addresses the original issue:

the foreground and the background screens are not distinguishable

So I'll go ahead and close this issue. I've created a new issue for the animation: https://github.com/wordpress-mobile/WordPress-Android/issues/16600.