Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Wrong WindowInsets after navigating from multi-window to single-window #236

Open
azizbekian opened this issue Dec 18, 2017 · 1 comment · May be fixed by #837
Open

Wrong WindowInsets after navigating from multi-window to single-window #236

azizbekian opened this issue Dec 18, 2017 · 1 comment · May be fixed by #837
Assignees
@nickbutcher
Labels
Up for grabs 🤚

Comments

@azizbekian
Copy link

azizbekian commented Dec 18, 2017 •
edited
Loading

Assuming it is multi-window mode, where first app is an arbitrary app and second app is Plaid, then as soon as Plaid becomes the only app in foreground (mode changes to single-window), then Plaid gets wrong WindowInsets, which results toolbar not being correctly padded.

ezgif-5-fc533d73b8

Note, had Plaid been the first app (in the top of the screen), then WindowInsets are correctly dispatched.

Thoughts to workaround

If inside onApplyWindowInsets() checked:

drawer.setOnApplyWindowInsetsListener(new View.OnApplyWindowInsetsListener() {
    @Override
    public WindowInsets onApplyWindowInsets(View v, WindowInsets insets) {
        if (insets.getSystemWindowInsetLeft() == 0 &&
                insets.getSystemWindowInsetTop() == 0 &&
                insets.getSystemWindowInsetRight() == 0 &&
                insets.getSystemWindowInsetBottom() == 0) {
            ViewCompat.requestApplyInsets(drawer);
            return insets.consumeSystemWindowInsets();
        }
        ...
    }
});

This will solve the issue, but the side effect is, that this will result in an infinite loop in multi-window mode, because those insets are 0 in multi-window mode, which is ok. Thus, we should check whether we are in multi- or single-window mode:

if (!isInMultiWindowMode() &&
        insets.getSystemWindowInsetLeft() == 0 &&
        insets.getSystemWindowInsetTop() == 0 &&
        insets.getSystemWindowInsetRight() == 0 &&
        insets.getSystemWindowInsetBottom() == 0) {
	
	    ViewCompat.requestApplyInsets(drawer);
	    return insets.consumeSystemWindowInsets();
}

If we are in a single-window mode and insets are equal to 0, only then request for a new dispatch.

Theoretically, this should work. In practice isInMultiWindowMode() still returns true when in single-window mode. Stale value issue of isInMultiWindowMode() is described in CommonsBlog.

On my machine 250ms is the delay, that is needed for the value to reflect real multi-window state. Following piece of code would make WindowInsets correctly applied:

@Override
public WindowInsets onApplyWindowInsets(View v, final WindowInsets insets) {

    if (isInMultiWindowMode()) {
        new Handler().postDelayed(new Runnable() {
            @Override
            public void run() {
                if (!isInMultiWindowMode() && insets.getSystemWindowInsetLeft() == 0 &&
                        insets.getSystemWindowInsetTop() == 0 &&
                        insets.getSystemWindowInsetRight() == 0 &&
                        insets.getSystemWindowInsetBottom() == 0) {
                    ViewCompat.requestApplyInsets(drawer);
                }
            }
        }, 250);
        return insets.consumeSystemWindowInsets();
    }
    ...
}
radekkozak added a commit to radekkozak/plaid that referenced this issue Jul 19, 2020
@radekkozak
Fix: multi-window mode insets problems (nickbutcher#236)
52d6b44
radekkozak added a commit to radekkozak/plaid that referenced this issue Jul 19, 2020
@radekkozak
Fix: multi-window mode insets problems (nickbutcher#236)
df695e8 
Takes care of few WindowInsets problems with HomeActivity layout:

-  toolbar not being correctly padded when returning from multi-window mode (bottom position)
- toolbar not being correctly laid out when back to multi-window after activity was “hidden” with second activity

Resolves: nickbutcher#236
@radekkozak radekkozak linked a pull request Jul 19, 2020 that will close this issue
Open
6 tasks
@radekkozak
Copy link

Chiming in here as the issue is still pervading the current implementation and can be quite unnerving when using multi-window mode. I studied the issue and i came up with the solution which i think is most elegant and doesn’t require listeners “racing” with each other and setting special timings (not a fan of those). As a “side-effect” it happens to clean up the code a bit as well in HomeActivity as well.

Solution re-uses already existing (in the core module utils) BindingAdapters as advocated by @chrisbanes (https://chris.banes.dev/2019/04/12/insets-listeners-to-layouts/) and - to my surprise used in other parts of codebase but not here - which i suspect might be just simply legacy code. Apart from issue described by @azizbekian there were also few others layout rendering problems i stumbled upon (see attached screenshot) and which are remedied by the PR #837 i’ve already provided.

/cc @nickbutcher

plaid-multi-window-bug

@radekkozak radekkozak mentioned this issue Sep 9, 2022
Open
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nickbutcher nickbutcher

Labels
Up for grabs 🤚
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix: multi-window mode insets problems (#236) radekkozak/plaid
4 participants
@pfmaggi @nickbutcher @radekkozak @azizbekian