Skip to content
This repository has been archived by the owner on Sep 30, 2023. It is now read-only.

Edge-to-edge behaviour on Android R #4

Open
mr-thierry opened this issue Sep 4, 2020 · 7 comments
Open

Edge-to-edge behaviour on Android R #4

mr-thierry opened this issue Sep 4, 2020 · 7 comments

Comments

@mr-thierry
Copy link

On Android R, the behaviour of ViewCompat.setOnApplyWindowInsetsListener has changed to support IME (see for example https://proandroiddev.com/exploring-windowinsets-on-android-11-a80cf8fe19be).

I currently have a fragment that is used in two different location in my app:

  1. As the main fragment of a single activity
  2. As a fragment inside a CoordinatorLayout in another activity

My app is build like so:

  1. The fragment calls edgeToEdge() to include padding for the top system bar
  2. The CoordinatorLayout also calls edgeToEdge() to include padding for the top system bar

In API 29, the fragment inside the CoordinatorLayout won't get OnApplyWindowInsetsListener called
In API 30, the fragment do get OnApplyWindowInsetsListener called (probably due to the change for the new IME code)

Now, my fix right now is just to check if the fragment is not inside a CoordinatorLayout to call fragment.edgeToEdge().

But I'm wondering if there is anything that could be done in the edge-to-edge library to fix this instead. What do you think?

@sergejsha
Copy link
Owner

sergejsha commented Sep 6, 2020

My first reaction would be that API30 has the expected behavior, since top padding is applied twice.

I added a new example called "Issue 4" where I tried to reproduce the issue you described. Here is the screenshot of the screens with the CoordinatorLayout on API21, API29 and API30. They look consistently "broken" everywhere - the padding is applied twice.

The code can be found in issue4 branch. It should have some differences to how you have it in you app. Could you change it in a way, that the issue can be reproduced? Then we can think about a fix.

@mr-thierry
Copy link
Author

I was able to tweak the sample app to replicate the issue.

It seem the behaviour of WindowInsets have changed in R, but only if your app also target R.

While stepping through the code, I saw that CollapsingToolbarLayout consume the insets (see https://android.googlesource.com/platform/frameworks/support/+/6ba61c5/design/src/android/support/design/widget/CollapsingToolbarLayout.java#167). From what I see, in R this "consume" is ignored and the insets are still propagated to the child view.

Below R this consume is not ignored and the children never get the WindowInset.

That is why in R the Fragment padding is set. But not in Q.

Let me know of what you think of the sample and my findings! :)

@sergejsha
Copy link
Owner

sergejsha commented Sep 11, 2020

Thanks for debugging it Thierry-Dimitri! Your findings make perfectly sense.

Initially, to avoid such inconsistencies, I decided to always install the WindowInsetsListener at the top most view, wich is the content view of the activity. This would also ensure that the listener is always invoked, independently where in the view hierarchy edgeToEdge extension gets applied.

Unfortunately Fragment.edgeToEdge function was still using fragment's content view instead, which, in this case, caused inconsistencies between API. I enhanced the extension function (see the same issue4 branch) and FragmentConstraintLayout.setPadding is called all the time for all API levels now. This should be the desired default behavior in my opinion.

That do you think? Would you be able to adopt the change in you app without adding any custom if-else code?

@mr-thierry
Copy link
Author

Hi @beworker
I'm not sure FragmentConstraintLayout.setPadding should be called if the fragment is below the AppBar.

The latest release of androidx.core (1.5-alpha02) provide more functionalities for the WindowInset (to support the new IME). But I'm always hesitant to include alpha library in production. Plus, I haven't taken the time to see what has changed really in v1.5. So I'm not sure the behaviour that I'm looking for (not calling FragmentConstraintLayout.setPadding if the fragment is below the AppBar is possible)

What do you think? :)

@mr-thierry
Copy link
Author

@beworker I found another problem with R :(
When the software keyboard is shown, the bottom padding now include the keyboard height. Again, probably that androidx.core v1.5 contains way to better handle the soft keyboard.

@sergejsha
Copy link
Owner

@ThierryD Both issues should have the same reason - the library ignores inset consumption. It can be fixed rather easily, but I'm short on time at the moment. I might find some time on the weekend however.

@mr-thierry
Copy link
Author

@beworker Hey no worry. I understand that sometimes we do not have time to work on our hobby project. I just wanted to raise this new issue :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants