chore: sync upstream PR #8454 - Fix navigation bar overlay in edge-to-edge mode on Android - #67
Conversation
Added navigation bar visibility handling and adjusted safe area calculations. Updated insets handling for improved layout compatibility.
📝 WalkthroughWalkthroughThe Android ChangesSystemBars Insets and Style Refactor
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: 🟡 Moderate · up to This change can produce incorrect Android system-bar visibility, safe-area padding, or startup behavior in affected configurations. The PR is not merge-ready until the two bar/inset correctness issues are fixed or explicitly accepted, with the startup injection issue also addressed. Sequence Diagram(s)sequenceDiagram
participant SystemBars
participant WebViewParent
participant RootWindow
participant PageCSS
SystemBars->>WebViewParent: requestApplyInsets()
WebViewParent->>RootWindow: read root window insets
RootWindow-->>WebViewParent: provide safe-area source
WebViewParent->>PageCSS: inject safe-area CSS
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
d6113cc to
6136d3c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java (1)
353-359:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
navBarVisiblenot updated whenshow(BAR_STATUS_BAR)implicitly reveals the navigation bar
windowInsetsControllerCompat.show(WindowInsetsCompat.Type.systemBars())(line 354) encompasses both status and navigation bars. Whenbar = BAR_STATUS_BARand the navigation bar was previously hidden, the nav bar becomes physically visible, but the secondifblock (line 356) is skipped, leavingnavBarVisible = false. On pre-API 30 devices this causesgetNavBarHeightFromResources()to return0, so the fallback bottom safe-area inset will be under-reported despite the nav bar being on screen.🐛 Proposed fix
if (bar.isEmpty() || bar.equals(BAR_STATUS_BAR)) { windowInsetsControllerCompat.show(WindowInsetsCompat.Type.systemBars()); } if (bar.isEmpty() || bar.equals(BAR_GESTURE_BAR)) { windowInsetsControllerCompat.show(WindowInsetsCompat.Type.navigationBars()); navBarVisible = true; }Either align the show path with the hide path (use
statusBars()forBAR_STATUS_BAR):if (bar.isEmpty() || bar.equals(BAR_STATUS_BAR)) { - windowInsetsControllerCompat.show(WindowInsetsCompat.Type.systemBars()); + windowInsetsControllerCompat.show(WindowInsetsCompat.Type.statusBars()); } if (bar.isEmpty() || bar.equals(BAR_GESTURE_BAR)) { windowInsetsControllerCompat.show(WindowInsetsCompat.Type.navigationBars()); navBarVisible = true; }Or, if retaining
systemBars()is intentional, also updatenavBarVisiblethere:if (bar.isEmpty() || bar.equals(BAR_STATUS_BAR)) { windowInsetsControllerCompat.show(WindowInsetsCompat.Type.systemBars()); + navBarVisible = true; // systemBars() restores navigation bar as well } if (bar.isEmpty() || bar.equals(BAR_GESTURE_BAR)) { windowInsetsControllerCompat.show(WindowInsetsCompat.Type.navigationBars()); navBarVisible = true; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java` around lines 353 - 359, The show logic in SystemBars.java incorrectly leaves navBarVisible false when calling windowInsetsControllerCompat.show(WindowInsetsCompat.Type.systemBars()) for BAR_STATUS_BAR because systemBars() reveals both status and navigation bars; update the navBarVisible state when showing systemBars() (set navBarVisible = true) or change the BAR_STATUS_BAR branch to call windowInsetsControllerCompat.show(WindowInsetsCompat.Type.statusBars()) instead so the navBarVisible flag remains correct for getNavBarHeightFromResources() and subsequent safe-area calculations; adjust the conditional handling around BAR_STATUS_BAR, BAR_GESTURE_BAR and the navBarVisible variable in the same method to keep hide and show paths symmetric.
🧹 Nitpick comments (1)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java (1)
166-182: ⚡ Quick winExtract duplicated window-flag setup into a shared helper
The window initialization block in
handleOnResume()(lines 170–177) is identical to the one insideinitSystemBars()(lines 116–123). If either copy drifts it will silently break one of the two code paths.♻️ Suggested refactor
+ private void applyEdgeToEdgeWindowFlags() { + Window window = getActivity().getWindow(); + WindowCompat.setDecorFitsSystemWindows(window, false); + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.VANILLA_ICE_CREAM) { + window.setNavigationBarColor(android.graphics.Color.TRANSPARENT); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + window.setNavigationBarContrastEnforced(false); + } + } + } private void initSystemBars() { ... getBridge().executeOnMainThread(() -> { - Window window = getActivity().getWindow(); - WindowCompat.setDecorFitsSystemWindows(window, false); - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.VANILLA_ICE_CREAM) { - window.setNavigationBarColor(android.graphics.Color.TRANSPARENT); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { - window.setNavigationBarContrastEnforced(false); - } - } + applyEdgeToEdgeWindowFlags(); setStyle(style, ""); setHidden(hidden, ""); ViewCompat.requestApplyInsets((View) getBridge().getWebView().getParent()); }); } `@Override` protected void handleOnResume() { super.handleOnResume(); getBridge().executeOnMainThread(() -> { - Window window = getActivity().getWindow(); - WindowCompat.setDecorFitsSystemWindows(window, false); - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.VANILLA_ICE_CREAM) { - window.setNavigationBarColor(android.graphics.Color.TRANSPARENT); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { - window.setNavigationBarContrastEnforced(false); - } - } + applyEdgeToEdgeWindowFlags(); setStyle(currentGestureBarStyle, BAR_GESTURE_BAR); setStyle(currentStatusBarStyle, BAR_STATUS_BAR); ViewCompat.requestApplyInsets((View) getBridge().getWebView().getParent()); }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java` around lines 166 - 182, The window initialization block duplicated in handleOnResume() and initSystemBars() should be extracted into a single private helper (e.g., configureWindowFlags(Window window) or setupSystemBarWindowFlags(Window window)); move the logic that calls WindowCompat.setDecorFitsSystemWindows(window, false), sets transparent navigation bar for older SDKs and toggles navigationBarContrastEnforced for Q+, into that helper and replace the duplicated blocks with calls to it from both handleOnResume() and initSystemBars(), preserving current Build.VERSION.SDK_INT checks and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java`:
- Around line 353-359: The show logic in SystemBars.java incorrectly leaves
navBarVisible false when calling
windowInsetsControllerCompat.show(WindowInsetsCompat.Type.systemBars()) for
BAR_STATUS_BAR because systemBars() reveals both status and navigation bars;
update the navBarVisible state when showing systemBars() (set navBarVisible =
true) or change the BAR_STATUS_BAR branch to call
windowInsetsControllerCompat.show(WindowInsetsCompat.Type.statusBars()) instead
so the navBarVisible flag remains correct for getNavBarHeightFromResources() and
subsequent safe-area calculations; adjust the conditional handling around
BAR_STATUS_BAR, BAR_GESTURE_BAR and the navBarVisible variable in the same
method to keep hide and show paths symmetric.
---
Nitpick comments:
In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java`:
- Around line 166-182: The window initialization block duplicated in
handleOnResume() and initSystemBars() should be extracted into a single private
helper (e.g., configureWindowFlags(Window window) or
setupSystemBarWindowFlags(Window window)); move the logic that calls
WindowCompat.setDecorFitsSystemWindows(window, false), sets transparent
navigation bar for older SDKs and toggles navigationBarContrastEnforced for Q+,
into that helper and replace the duplicated blocks with calls to it from both
handleOnResume() and initSystemBars(), preserving current Build.VERSION.SDK_INT
checks and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12280262-28ba-4398-9ac2-b99f1d39613a
📒 Files selected for processing (1)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java
RobinWitch
left a comment
RobinWitch
left a comment
There was a problem hiding this comment.
Requesting changes because the required lint job is failing on formatting in the file changed by this PR.
The lint check gets through eslint and then fails at the repository's Prettier check:
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java:1:1: error: Line needs to be added (prettier/insert)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java:199:24: error: Code style (prettier/format)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java:8184:1: error: Line needs to be added (prettier/insert)
Please run the configured Prettier command on SystemBars.java before merging; as-is the PR blocks the downstream test jobs because lint never completes.
# Conflicts: # android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Beta npm buildMaintainers can publish one Capacitor Plus workspace package from this PR to npm for fast testing. Comment Examples: /publish-beta core
/publish-beta cli
/publish-beta @capacitor-plus/coreIf exactly one workspace package changed, Packages:
The workflow will:
Security note: beta publish is only enabled for branches inside this repository. |
Conflict resolved and rebuilt, but blocked by an upstream defectWhat I did: the branch now carries only upstream #8454's change to Resolving it needed a judgment call.
Why it is still blocked: That comes from Your options:
I did not pick one, because both 1 and 2 change behaviour on real Android devices across API 31–35 in ways I cannot verify here. Tell me which you want and I will push it. Related: #94 is a competing implementation of this same code path; see my comment there. Posted by an AI agent (Cursor). |
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
…llback Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Unblocked — pushed the lint fix, will merge once CI is greenFollowing up on my previous comment: I went with option 1 and annotated the offending method, since it is the only choice that keeps the upstream patch's behaviour byte-for-byte. // Only reached on API < 30 when the window reports no inset on any edge, so the
// choice is this approximation or a hardcoded 0. Lint rejects the internal
// resource because OEMs may change it; a wrong height still beats no inset.
@SuppressLint("InternalInsetResource")
private int getNavBarHeightFromResources() {Option 2 (dropping the fallback) would have gutted the fix. The obvious "clean" third option — swapping the internal resource for The suppression is narrow. For the record, this branch is purely additive against
The Posted by an AI agent (Cursor). |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java (2)
117-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated window configuration.
initSystemBars()andhandleOnResume()repeat the same decor-fitting, transparent navigation-bar, and contrast-enforcement setup. Move this logic into one private helper and call it from both methods.Also applies to: 167-183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java` around lines 117 - 127, Extract the shared window configuration from initSystemBars() and handleOnResume() into a private helper that applies decor fitting, transparent navigation-bar color, and API-gated contrast enforcement. Replace both duplicated blocks with calls to this helper, preserving the existing SDK checks and behavior.
357-363: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
Type.statusBars()for status-bar-only calls.
Type.systemBars()also shows the navigation bar. After hiding the navigation bar, showing only the status bar leavesnavBarVisiblefalse. On API levels below 30, the navigation-height fallback can therefore return 0 while the navigation bar is visible. UseWindowInsetsCompat.Type.statusBars()in this branch and add a regression test for this sequence.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java`:
- Around line 117-127: Extract the shared window configuration from
initSystemBars() and handleOnResume() into a private helper that applies decor
fitting, transparent navigation-bar color, and API-gated contrast enforcement.
Replace both duplicated blocks with calls to this helper, preserving the
existing SDK checks and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f27b9730-bc08-4698-ba53-062a780904d9
📒 Files selected for processing (1)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java (3)
237-242: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHonor
insetsHandling: "disable"before mutating insets.When
insetHandlingEnabledis false,initSystemBars()still installs the listener. The listener still applies IME padding and replaces system-bar insets with zero. Return before installing the listener.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java` around lines 237 - 242, Update initWindowInsetsListener to return immediately when insetHandlingEnabled is false, before calling ViewCompat.setOnApplyWindowInsetsListener or otherwise mutating insets. Preserve the existing listener behavior when inset handling is enabled.Source: MCP tools
352-362: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winShow only the requested system bar.
When
barisBAR_STATUS_BAR, useType.statusBars()instead ofType.systemBars().systemBars()also shows the navigation bar, whilenavBarVisibleremains unchanged.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java` around lines 352 - 362, Update the bar visibility logic to call Type.statusBars() when bar equals BAR_STATUS_BAR, while retaining Type.navigationBars() for BAR_GESTURE_BAR and both calls for an empty bar. Ensure requesting the status bar alone does not show the navigation bar or alter navBarVisible.Source: MCP tools
226-233: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the initial CSS injection until a document exists.
When root insets are available during plugin load,
injectSafeAreaCSS()can run beforedocument.documentElementexists. The current check only validates theWebView, so the script logs a startupTypeError. Add adocument.documentElementcheck or defer injection untilonPageCommitVisible().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java` around lines 226 - 233, Update initSafeAreaCSSVariables so injectSafeAreaCSS runs only after the loaded document has a document.documentElement; otherwise defer the injection until onPageCommitVisible. Preserve the existing insetHandlingEnabled, WebView, and root-insets checks.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java`:
- Around line 237-242: Update initWindowInsetsListener to return immediately
when insetHandlingEnabled is false, before calling
ViewCompat.setOnApplyWindowInsetsListener or otherwise mutating insets. Preserve
the existing listener behavior when inset handling is enabled.
- Around line 352-362: Update the bar visibility logic to call Type.statusBars()
when bar equals BAR_STATUS_BAR, while retaining Type.navigationBars() for
BAR_GESTURE_BAR and both calls for an empty bar. Ensure requesting the status
bar alone does not show the navigation bar or alter navBarVisible.
- Around line 226-233: Update initSafeAreaCSSVariables so injectSafeAreaCSS runs
only after the loaded document has a document.documentElement; otherwise defer
the injection until onPageCommitVisible. Preserve the existing
insetHandlingEnabled, WebView, and root-insets checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1ce0b1fc-9600-4f1e-a64f-7361dccf5f54
📒 Files selected for processing (1)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
4 participants
Upstream PR Sync
This PR syncs changes from an external contributor's PR on the official Capacitor repository.
Original PR
Automation
Synced from upstream by Capacitor+ Bot
Summary by CodeRabbit
New Features
Bug Fixes