Skip to content

Fix #5940: Add edge-to-edge window insets for drawer-based activities#6214

Draft
Neer-rn wants to merge 3 commits intooppia:developfrom
Neer-rn:fix-5940-edge-to-edge-toolbar-insets
Draft

Fix #5940: Add edge-to-edge window insets for drawer-based activities#6214
Neer-rn wants to merge 3 commits intooppia:developfrom
Neer-rn:fix-5940-edge-to-edge-toolbar-insets

Conversation

@Neer-rn
Copy link
Copy Markdown
Collaborator

@Neer-rn Neer-rn commented Apr 11, 2026

Explanation

Fixes part of #5940

This PR adds edge-to-edge window inset handling for all drawer-based screens in the app. When the EnableEdgeToEdge feature flag is ON, each screen properly handles the transparent status bar and navigation bar that Android 15+ enforces.

What's done so far:

  • Added edge-to-edge insets to all 5 drawer-based activity presenters
    (Home, ClassroomList, Options, Help, DeveloperOptions, AdministratorControls)
  • Added edge-to-edge insets to the shared NavigationDrawerFragmentPresenter
  • Gated all changes behind the EnableEdgeToEdge feature flag
  • Disabled deprecated StatusBarColor.statusBarColorUpdate() calls when
    the flag is ON

What's left:

  • Apply edge-to-edge insets to the remaining ~38 AppBarLayout-based screens
    (non-drawer screens like topic, exploration, story, etc.)

Depends on #6209 (feature flag PR must be merged first).

Essential Checklist

  • The PR title starts with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The explanation section above starts with "Fixes #bugnum: " (If this PR fixes part of an issue, use instead: "Fixes part of #bugnum: ...".)
  • Any changes to scripts/assets files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR does not contain any unnecessary code changes from Android Studio (reference).
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

Screenshots will be added during review once the full implementation is ready. Currently still working on remaining screens.

@Neer-rn
Copy link
Copy Markdown
Collaborator Author

Neer-rn commented Apr 11, 2026

Current Testing Results

Before (No changes, with values-v35/themes.xml keeping edge-to-edge opted out)

This is how the app looks right now without any fix, the status bar and navigation bar work the old way.

Screenshot from 2026-04-11 13-21-14 Screenshot from 2026-04-11 13-21-58 Screenshot from 2026-04-11 13-22-03 Screenshot from 2026-04-11 13-22-11 Screenshot from 2026-04-11 13-22-24 Screenshot from 2026-04-11 13-22-31 Screenshot from 2026-04-11 13-25-01

After (Feature flag ON, values-v35/themes.xml deleted)

This is how the app looks with the edge-to-edge fix enabled. The status bar shows the correct dark teal color behind the toolbar, and the navigation drawer shows its own dark blue-black color in the status bar area. Bottom navigation bar area is also handled properly.

Screenshot from 2026-04-11 13-28-37 Screenshot from 2026-04-11 13-28-41 Screenshot from 2026-04-11 13-28-43 Screenshot from 2026-04-11 13-28-48 Screenshot from 2026-04-11 13-28-55 Screenshot from 2026-04-11 13-28-59 Screenshot from 2026-04-11 13-29-20

Note on the dual-tone status bar when drawer is open

I noticed that when the drawer is open, the status bar shows two different colors, dark blue-black on the drawer side and dark teal on the activity side. This is different from before where the entire status bar was one uniform dark color.

I did some research on this and found that this is actually the correct behavior for edge-to-edge mode.

Why it was one color before:
The old system used window.statusBarColor which painted the entire status bar with one solid color. DrawerLayout also used setStatusBarBackgroundColor() with fitsSystemWindows="true" to draw one color across the full width. But all of this is deprecated and doesn't work on Android 15+.

Why it's two colors now:
In edge-to-edge mode, the status bar is fully transparent. The app content draws behind it. Since the drawer and the main content are separate views with their own background colors, each side naturally shows its own color through the transparent status bar.

What the official Android docs say about this:
The edge-to-edge design guidelines say that navigation drawers should have "a separate protection from the rest of the app." The docs also show examples of this dual-tone status bar for multi-pane layouts and describe it as "an edge-to-edge app with dual tone gradient protection spanning two panes behind the system status bar."

So the two-tone status bar is the officially recommended behavior and I think we should keep it as-is.

@subhajitxyz
Copy link
Copy Markdown
Collaborator

Could you please check in landscape mode, specially drawers, In my case, I observed some unexpected shades.

@Neer-rn
Copy link
Copy Markdown
Collaborator Author

Neer-rn commented Apr 13, 2026

Could you please check in landscape mode, specially drawers, In my case, I observed some unexpected shades.

@subhajitxyz I have been working on the landscape drawer overlay issue you pointed out in the last meeting. Spent quite a bit of time investigating this but have not been able to resolve this issue. Thinking of opening a discussion and surely will bring this issue up in tomorrow's meeting so that we can further discuss this.

@subhajitxyz
Copy link
Copy Markdown
Collaborator

I have tried many things, but they didn’t work for me.
I think the problem is in the drawer layout.
The current layout uses two NavigationViews:

  • The outer one is used to handle the drawer’s open/close behavior.

  • The inner one actually contains the header, menu items, and footer.

Since NavigationView automatically handles insets, I suspect that this is causing the shading issue in the inner layout.
If this is the case, I think we need to redesign it properly or find an alternative approach

@Neer-rn
Copy link
Copy Markdown
Collaborator Author

Neer-rn commented Apr 14, 2026

Hey @subhajitxyz, I was able to fix it! Let me explain what happened in detail.

So after you mentioned this in the last meeting about this issue, I spent a lot of time trying to fix it. Like I was working on this particular issue from last week.

What I tried (and what didn't work):

  1. Set fitsSystemWindows = false on both NavigationViews --> didn't fix it
  2. Set drawerLayout.drawerElevation = 0f --> didn't fix it
  3. Set drawerLayout.setStatusBarBackground(null) --> didn't fix it
  4. Set app:insetForeground="@android:color/transparent" on outer NavigationView --> didn't fix it
  5. Set android:background on FragmentContainerView in XML --> didn't fix it
  6. Set background programmatically on FragmentContainerView --> didn't fix it

I searched a lot online but couldn't find anything about this exact issue. I tried the best and most powerful LLMs too and Gemini gave terrible answers, ChatGPT suggested things like setTopInsetScrimEnabled() which doesn't even exist in Material Components 1.3.0. Most of them were just hallucinating generating bunch of nonsense most of the code they gave I didnt even tried to implement and test as I could tell their approach was so wrong but Claude's research was actually the most helpful, it pointed me towards ScrimInsetsFrameLayout source code and the relationship between OnApplyWindowInsetsListener and fitsSystemWindows. I then went and verified this myself by reading the actual source code it seemed so complicated but I used claude to understand the code, and I thought this seems to be the most correct one.

The actual root cause:

You were right, the issue is with the inner NavigationView (fragment_drawer_nav_view) and how it handles insets.

NavigationView extends ScrimInsetsFrameLayout. In its constructor, ScrimInsetsFrameLayout calls ViewCompat.setOnApplyWindowInsetsListener(this, ...). This listener stores system bar inset dimensions and later draws a #4000 (25% black) scrim in those areas. The #4000 value comes from the Widget.Design.ScrimInsetsFrameLayout style, which Widget.Design.NavigationView inherits from.

Why fitsSystemWindows = false didn't stop it:

I checked the AOSP source code for View.dispatchApplyWindowInsets():

public WindowInsets dispatchApplyWindowInsets(WindowInsets insets) {
    if (mOnApplyWindowInsetsListener != null) {
        return mOnApplyWindowInsetsListener.onApplyWindowInsets(this, insets);
    } else {
        return onApplyWindowInsets(insets);
    }
}

The fitsSystemWindows flag only affects onApplyWindowInsets() (the else branch). But ScrimInsetsFrameLayout sets an OnApplyWindowInsetsListener in the constructor, so the listener always fires regardless of fitsSystemWindows. Ian Lake's article also confirms that setOnApplyWindowInsetsListener() takes priority over the view's own inset handling.

(The above is the only explanation that I was convinced and at this point Claude and I both agree on this because this is the only way I was able to fix this issue)

Why it only showed in landscape:

In portrait, the status bar inset is at the top and the drawer header covers it, so the #4000 scrim was hidden. In landscape with edge-to-edge, system bar insets are on the sides. The scrim was drawn on the side edges with nothing covering it, that was the dark shade.

Why the outer NavigationView didn't have this problem:

We were already setting ViewCompat.setOnApplyWindowInsetsListener(binding.root) for handling bottom padding and this replaced ScrimInsetsFrameLayout's constructor-set listener automatically. We just needed to do the same for the inner one.

The fix (3 lines):

ViewCompat.setOnApplyWindowInsetsListener(binding.fragmentDrawerNavView) { _, insets ->
    insets
}

This replaces ScrimInsetsFrameLayout's constructor-set listener with a no-op, so it never stores insets and the #4000 scrim is never drawn. I confirmed this by testing systematically added the fix, shade gone. Removed it, shade came back. Added again, shade gone.

@BenHenning I would like to request you to specifically review the listener override in applyEdgeToEdgeInsetsToDrawer() in NavigationDrawerFragmentPresenter.kt. Not sure if I should have pinged you here before the PR is ready for review but this is a non-obvious fix and I want to make sure the approach is correct so please take more time to verify this approach when I mark this PR as Ready for review.

Thank you @subhajitxyz . Please have a look at my approach, What do you think about this??

Here is Before
Screenshot from 2026-04-14 11-09-59
Screenshot from 2026-04-14 11-10-08

After
Screenshot from 2026-04-14 11-18-05

@subhajitxyz
Copy link
Copy Markdown
Collaborator

subhajitxyz commented Apr 14, 2026

Thanks @Neer-rn . It is actually a deep finding and a detailed explanation, and the solution is quite tricky. I really appreciate that 🙌

I think I also need to start using LLMs to get deeper insights like this. I would love to learn from you.

I took a quick look at your PR, and it looks good overall 👍
One thing seems to be missing: handling padding in landscape mode. In that case, the display cutout and the 3-button navigation bar can appear on the left and right sides of the device, so we should also apply horizontal padding there.

// system bar inset areas. Setting fitsSystemWindows=false alone is not sufficient because
// the OnApplyWindowInsetsListener set in ScrimInsetsFrameLayout's constructor is
// independent of the fitsSystemWindows flag.
ViewCompat.setOnApplyWindowInsetsListener(binding.fragmentDrawerNavView) { _, insets ->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this and the other two: is it expected that we need to override the listeners in this way? It feels rather hacky and difficult to reason about:

  • What if a different part of the lifecycle overrides this again?
  • What are the implications of overriding this across different device and SDK level configurations?
  • Does this introduce any RTL or accessibility problems?
  • What if the implementation being altered changes its behaviors to use something else as a hook in the future?

I find it quite reasonable to have something like this as a medium-term solution (provided we don't find any issues with it while testing) so long as we know what the long-term replacement is. Otherwise, we may well want to consider what work may be needed to do a more "proper" fix (if three is one) that doesn't require overriding the insets listeners.

@BenHenning
Copy link
Copy Markdown
Member

Thanks @Neer-rn and @subhajitxyz! I agree that your investigation was very well done @Neer-rn! I looked at the requested method and left one comment with more thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants