ADFA-2646 | Fix window insets accumulation and landscape appbar behavior#1114
Merged
ADFA-2646 | Fix window insets accumulation and landscape appbar behavior#1114
Conversation
…pe mode Refactor inset extensions to cache initial padding and prevent redundant padding calculations
jatezzz
requested review from
a team,
Daniel-ADFA,
dara-abijo-adfa,
itsaky-adfa and
jomen-adfa
Contributor
📝 WalkthroughRelease Notes - Window Insets Accumulation & Landscape AppBar Fix (PR
|
| Cohort / File(s) | Summary |
|---|---|
Editor activity inset flow app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt |
Replaced prior inset application (orientation-dependent top padding, sidebar incremental recompute) with simplified applyStandardInsets(systemBars) and calls to extensions: applyResponsiveAppBarInsets, applyRootSystemInsetsAsPadding, and delegated applyImmersiveModeInsets. Reapplies insets on configuration changes and reanchors bottom sheet. |
Window-insets utilities & binding extensions app/src/main/java/com/itsaky/androidide/utils/WindowInsetsExtensions.kt |
New utilities: InitialPadding data class, View.getOrStoreInitialPadding(), View.applyResponsiveAppBarInsets(...), View.applyRootSystemInsetsAsPadding(...), and ContentEditorBinding extensions (applyImmersiveModeInsets, refreshBottomSheetAnchor, resetBottomSheetAnchor, applyBottomSheetAnchorForOrientation) to preserve initial padding and apply system-bar insets responsively. |
Bottom sheet anchor control app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt |
Added resetOffsetAnchor() to clear previously set anchor offsets and restore collapsed/expanded offsets and associated padding/height adjustments. |
Resource ids resources/src/main/res/values/ids.xml |
Added exported resource id @id/tag_initial_padding used to store initial padding on views to prevent accumulation. |
Sequence Diagram(s)
sequenceDiagram
participant Activity as BaseEditorActivity
participant ViewUtils as WindowInsetsExtensions
participant Binding as ContentEditorBinding
participant BottomSheet as EditorBottomSheet
Activity->>ViewUtils: request applyInsets / requestApplyInsets()
ViewUtils->>Binding: applyResponsiveAppBarInsets / applyRootSystemInsetsAsPadding
Binding->>Binding: applyImmersiveModeInsets(systemBars)
Binding->>BottomSheet: applyBottomSheetAnchorForOrientation(orientation)
BottomSheet->>BottomSheet: setOffsetAnchor() / resetOffsetAnchor()
Activity->>Activity: onConfigurationChanged -> request reapply and reanchor
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- ADFA-3207: Resolve window inset issues #1058: Related window-insets changes in BaseEditorActivity and inset-padding helpers.
- ADFA-3048 | Implement landscape immersive mode for editor #1101: Overlapping immersive/window-inset refactors and bottom-sheet anchoring adjustments.
- ADFA-1788 | Fix sidebar overlap with status bar on split-screen resize #840: Prior sidebar inset handling changes that this PR replaces/refactors.
Suggested reviewers
- itsaky-adfa
- Daniel-ADFA
- dara-abijo-adfa
Poem
🐰 I nudged the padding, stored each tiny gap,
Now bars and sheets fit snug in every app.
Portrait or wide — I hop and I prune,
Insets aligned under the code-moon 🌙
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and specifically describes the main changes: fixing window insets accumulation and landscape appbar behavior. |
| Description check | ✅ Passed | The description is directly related to the changeset, explaining the refactoring of window inset handling, the caching mechanism, and the specific issues being addressed. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
fix/ADFA-2646-window-insets-accumulation
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt`:
- Line 691: The app bar gets double-padded because
applyResponsiveAppBarInsets(...) applies the portrait top inset to
editorAppBarLayout while the swipe-reveal drag animation still targets
editorAppbarContent; update the code so the inset and the swipe-reveal
padding/translation target the same view: either call
applyResponsiveAppBarInsets(content.editorAppbarContent) to move the inset to
the child, or change the swipe-reveal animation to animate
content.editorAppBarLayout instead of content.editorAppbarContent (make the
choice consistently everywhere where swipe-reveal padding/translation is
applied, e.g., in the drag/animation helper that references
editorAppbarContent).
In `@app/src/main/java/com/itsaky/androidide/utils/WindowInsetsExtensions.kt`:
- Around line 58-83: applyRootSystemInsetsAsPadding currently samples
ViewCompat.getRootWindowInsets only once on attach (via applyNow and a single
posted retry) and therefore doesn't react to later inset changes; update it to
register an inset listener (similar to applyResponsiveAppBarInsets) using
ViewCompat.setOnApplyWindowInsetsListener or View.setOnApplyWindowInsetsListener
so that getOrStoreInitialPadding() is combined with the incoming
WindowInsetsCompat on every onApplyWindowInsets call and updatePadding is
invoked each time; keep the existing doOnAttach logic to initialize initial
padding but remove the one-shot getRootWindowInsets sampling in favor of the
reactive listener and ensure you convert the incoming WindowInsets to insets via
getInsets(WindowInsetsCompat.Type.systemBars()) before applying
left/top/right/bottom based on applyLeft/applyTop/applyRight/applyBottom.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f0ad1b9-d06f-4656-b5ae-ee99bd4f4770
📒 Files selected for processing (3)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/utils/WindowInsetsExtensions.ktresources/src/main/res/values/ids.xml
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt (1)
676-687: Consider adding a retry limit to prevent unbounded recursion.If
ViewCompat.getRootWindowInsets(root)returns null, the method posts itself recursively without a guard. While this is unlikely to loop indefinitely in practice, adding a simple retry counter would make this more defensive.🛡️ Optional: Add retry limit
+private var insetsRetryCount = 0 +private val maxInsetsRetries = 3 + private fun reapplySystemBarInsetsFromRoot() { val root = _binding?.root ?: return val rootInsets = ViewCompat.getRootWindowInsets(root) if (rootInsets == null) { - root.post { reapplySystemBarInsetsFromRoot() } + if (insetsRetryCount < maxInsetsRetries) { + insetsRetryCount++ + root.post { reapplySystemBarInsetsFromRoot() } + } return } + insetsRetryCount = 0 val systemBars = rootInsets.getInsets(WindowInsetsCompat.Type.systemBars()) applyStandardInsets(systemBars) applyImmersiveModeInsets(systemBars) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt` around lines 676 - 687, reapplySystemBarInsetsFromRoot currently reposts itself whenever ViewCompat.getRootWindowInsets(root) is null which can cause unbounded retries; add a retry limit (e.g., maxRetries = 5) tracked either by an optional parameter to reapplySystemBarInsetsFromRoot(retryCount: Int = 0) or a private field, increment the counter each time you post the retry, and stop reposting once the limit is reached (optionally log a warning); keep the existing applyStandardInsets(systemBars) and applyImmersiveModeInsets(systemBars) calls unchanged when insets are available.app/src/main/java/com/itsaky/androidide/utils/WindowInsetsExtensions.kt (1)
36-52: Consider preserving initial padding for robustness.The function directly sets padding to
0orinsets.topwithout accounting for any initial top padding the views might have. IfeditorAppBarLayoutoreditorAppbarContentever have non-zero initial top padding (from XML or programmatic setup), it would be lost after rotation.This may be intentional if those views are guaranteed to have zero initial padding, but using
getOrStoreInitialPadding()here would make the function more robust.♻️ Optional: Preserve initial padding
fun View.applyResponsiveAppBarInsets(appbarContent: View) { + val initialParent = getOrStoreInitialPadding() + val initialContent = appbarContent.getOrStoreInitialPadding() + ViewCompat.setOnApplyWindowInsetsListener(this) { view, windowInsets -> val insets = windowInsets.getInsets(WindowInsetsCompat.Type.systemBars()) val isLandscape = resources.configuration.orientation == Configuration.ORIENTATION_LANDSCAPE if (isLandscape) { - view.updatePadding(top = 0) - appbarContent.updatePadding(top = insets.top) + view.updatePadding(top = initialParent.top) + appbarContent.updatePadding(top = initialContent.top + insets.top) } else { - view.updatePadding(top = insets.top) - appbarContent.updatePadding(top = 0) + view.updatePadding(top = initialParent.top + insets.top) + appbarContent.updatePadding(top = initialContent.top) } windowInsets }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/utils/WindowInsetsExtensions.kt` around lines 36 - 52, The applyResponsiveAppBarInsets function wipes any pre-existing top padding by setting it to 0 or insets.top; change it to preserve each view's initial top padding by reading/storing the original top padding (use a helper like getOrStoreInitialPadding or similar) and then apply updatePadding(top = initialTop + insets.top) for the view that needs the inset and updatePadding(top = initialTop) for the other case; update references to view and appbarContent in applyResponsiveAppBarInsets so they use the stored initialTop values instead of hard-coded 0 or insets.top.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt`:
- Around line 676-687: reapplySystemBarInsetsFromRoot currently reposts itself
whenever ViewCompat.getRootWindowInsets(root) is null which can cause unbounded
retries; add a retry limit (e.g., maxRetries = 5) tracked either by an optional
parameter to reapplySystemBarInsetsFromRoot(retryCount: Int = 0) or a private
field, increment the counter each time you post the retry, and stop reposting
once the limit is reached (optionally log a warning); keep the existing
applyStandardInsets(systemBars) and applyImmersiveModeInsets(systemBars) calls
unchanged when insets are available.
In `@app/src/main/java/com/itsaky/androidide/utils/WindowInsetsExtensions.kt`:
- Around line 36-52: The applyResponsiveAppBarInsets function wipes any
pre-existing top padding by setting it to 0 or insets.top; change it to preserve
each view's initial top padding by reading/storing the original top padding (use
a helper like getOrStoreInitialPadding or similar) and then apply
updatePadding(top = initialTop + insets.top) for the view that needs the inset
and updatePadding(top = initialTop) for the other case; update references to
view and appbarContent in applyResponsiveAppBarInsets so they use the stored
initialTop values instead of hard-coded 0 or insets.top.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c8c938bb-9294-4923-9133-0a3d724a4d53
📒 Files selected for processing (3)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.ktapp/src/main/java/com/itsaky/androidide/utils/WindowInsetsExtensions.kt
Daniel-ADFA
approved these changes
Mar 25, 2026
This was referenced Mar 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR refactors how window insets are handled in
BaseEditorActivityto fix several UI bugs related to app recreation, landscape mode, and the bottom navigation bar.Instead of manually calculating and accumulating padding values, we introduced a caching mechanism in
WindowInsetsExtensions.ktthat safely stores the view's original padding using a dedicated resource ID (R.id.tag_initial_padding). This guarantees that system insets are always added to a clean baseline, preventing the UI from overlapping the status bar or leaving gaps above the bottom navigation bar when the app is rotated or recreated.Details
applyRootSystemInsetsAsPaddingextension to prevent infinite padding growth.Screen_Recording_20260324_164005_One.UI.Home.mp4
Ticket
ADFA-2646