ADFA-3048 | Implement landscape immersive mode for editor#1101
Open
ADFA-3048 | Implement landscape immersive mode for editor#1101
Conversation
Adds LandscapeImmersiveController, touch observation, and UI toggles to handle auto-hiding toolbars.
jatezzz
force-pushed
the
feat/ADFA-3048-landscape-immersive-mode
branch
from
afdeec9 to
f8d0878
Compare
jatezzz
requested review from
Daniel-ADFA,
dara-abijo-adfa,
itsaky-adfa and
jomen-adfa
Contributor
📝 Walkthrough
WalkthroughAdds a landscape-only immersive UI controller and integrates it into BaseEditorActivity, refactors window-inset handling, adds touch-observing layout and toggle button views, and introduces a hollow green circle drawable for toggle backgrounds. Changes
Sequence Diagram(s)sequenceDiagram
participant Activity as BaseEditorActivity
participant Controller as LandscapeImmersiveController
participant TouchLayout as TouchObservingLinearLayout
participant AppBar as AppBarLayout
participant BottomSheet as BottomSheetBehavior
Activity->>Controller: bind() / onCreate()
Controller->>TouchLayout: set onTouchEventObserved callback
Controller->>AppBar: add offset & layout listeners
Controller->>BottomSheet: add state & slide listeners
Activity->>Controller: onSystemBarInsetsChanged(topInset)
Controller->>Controller: update stored inset, adjust paddings/margins
Note over TouchLayout,Controller: User interacts with app bar
TouchLayout->>Controller: onTouchEventObserved(MotionEvent)
Controller->>Controller: cancel auto-hide
Controller->>Controller: schedule auto-hide after delay
Note over AppBar,BottomSheet: Auto-hide triggers
Controller->>AppBar: collapse/hide top bar
Controller->>BottomSheet: force collapse / hide peek view
Activity->>Controller: onPause() / preDestroy()
Controller->>AppBar: remove listeners
Controller->>BottomSheet: remove listeners
Controller->>TouchLayout: clear callback
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
app/src/main/res/drawable/bg_hollow_green_circle.xml (1)
1-8: Place this drawable in the sharedresourcesmodule.This is reusable UI chrome referenced from multiple layouts, so keeping it under
app/src/main/res/drawable/makes it harder to reuse and maintain consistently across modules.Based on learnings, drawable resources in CodeOnTheGo should reside in
./resources/src/main/res/drawable/, notapp/src/main/res/drawable/, so they stay reusable across the application.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/drawable/bg_hollow_green_circle.xml` around lines 1 - 8, Move the shape drawable resource bg_hollow_green_circle.xml out of the app module and into the shared resources module so it can be reused; specifically, relocate the file (the oval shape with stroke and transparent solid) from app/src/main/res/drawable to the resources module at resources/src/main/res/drawable, update any references to R.drawable.bg_hollow_green_circle if needed (ensuring the shared resources module is added as a dependency to modules that use it), and verify builds/imports so layouts referencing bg_hollow_green_circle continue to resolve.app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt (1)
514-521: Prefer end-aware insets for the toggle margins.
layout_gravity="...|end"follows layout direction, butsystemBars.rightdoes not. If this screen is shown in RTL, the toggles will ignore the left-side safe inset.Possible fix
private fun applyImmersiveModeInsets(systemBars: Insets) { val content = _binding?.content ?: return val baseMargin = SizeUtils.dp2px(16f) + val endInset = + if (content.root.layoutDirection == View.LAYOUT_DIRECTION_RTL) { + systemBars.left + } else { + systemBars.right + } content.btnToggleTopBar.updateLayoutParams<ViewGroup.MarginLayoutParams> { topMargin = baseMargin + systemBars.top - marginEnd = baseMargin + systemBars.right + marginEnd = baseMargin + endInset } content.btnToggleBottomBar.updateLayoutParams<ViewGroup.MarginLayoutParams> { bottomMargin = baseMargin + systemBars.bottom - marginEnd = baseMargin + systemBars.right + marginEnd = baseMargin + endInset }🤖 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 514 - 521, The toggle margin calculations use the absolute inset (systemBars.right) so they break in RTL; update the two places where content.btnToggleTopBar and content.btnToggleBottomBar call updateLayoutParams to use systemBars.end (not systemBars.right) when computing marginEnd (i.e., set marginEnd = baseMargin + systemBars.end) so the end-aware inset respects layout direction.
🤖 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`:
- Around line 494-503: applyStandardInsets currently moves the status-bar top
inset onto editorAppbarContent when present but onSwipeRevealDragProgress still
animates padding on editorAppBarLayout, causing double top padding during drawer
drag; fix by making both places target the same view: either keep
applyStandardInsets applying top inset to editorAppBarLayout and update
onSwipeRevealDragProgress to animate editorAppBarLayout, or (preferred) keep
applyStandardInsets applying the inset to editorAppbarContent and change
onSwipeRevealDragProgress so it animates editorAppbarContent instead of
editorAppBarLayout; update all references to updatePadding calls in
applyStandardInsets and the padding animation in onSwipeRevealDragProgress to
use the same view (editorAppbarContent or editorAppBarLayout) and remove the
duplicated top inset application.
- Around line 693-696: The activity handles orientation changes without
recreation, but onConfigurationChanged in BaseEditorActivity calls
immersiveController?.onConfigurationChanged(newConfig) while the inflated
portrait layout lacks the required editor_appbar_content
wrapper/TouchObservingLinearLayout used by immersive mode; fix by detecting
orientation change in BaseEditorActivity.onConfigurationChanged and
rebinding/reinflating the appropriate layout (or swapping to the landscape
binding) before calling immersiveController?.onConfigurationChanged so the
editor_appbar_content and its scroll flags are present; target symbols:
onConfigurationChanged, immersiveController, editor_appbar_content,
TouchObservingLinearLayout, and the activity’s view-binding/rebind logic.
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/LandscapeImmersiveController.kt`:
- Around line 128-132: onPause() currently cancels the auto-hide job
(cancelAutoHide()) and clears isUserInteractingWithTopBar without a resume path,
so if the top bar was visible it will remain pinned after returning; add an
onResume() method on LandscapeImmersiveController that checks the top-bar
visibility state (e.g., isTopBarVisible or similar) and re-schedules the
auto-hide job (call the existing schedule/start auto-hide helper) only when the
top bar is visible and the user is not interacting, then invoke
immersiveController?.onResume() from BaseEditorActivity.onResume() to restore
the auto-hide behavior.
In `@app/src/main/res/layout-land/content_editor.xml`:
- Around line 168-188: Replace the two plain View elements with accessible
clickable widgets (e.g., ImageButton or MaterialButton) for btn_toggle_top_bar
and btn_toggle_bottom_bar, make them focusable/clickable, add appropriate
android:contentDescription for TalkBack, and ensure a minimum 48dp touch target
by increasing layout_width/layout_height or adding extra padding (or wrap with a
larger touch container) while preserving the existing background drawable
(bg_hollow_green_circle) and visibility attributes; apply the same changes to
both layout variants so the toggles gain proper role, label, and hit area.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt`:
- Around line 514-521: The toggle margin calculations use the absolute inset
(systemBars.right) so they break in RTL; update the two places where
content.btnToggleTopBar and content.btnToggleBottomBar call updateLayoutParams
to use systemBars.end (not systemBars.right) when computing marginEnd (i.e., set
marginEnd = baseMargin + systemBars.end) so the end-aware inset respects layout
direction.
In `@app/src/main/res/drawable/bg_hollow_green_circle.xml`:
- Around line 1-8: Move the shape drawable resource bg_hollow_green_circle.xml
out of the app module and into the shared resources module so it can be reused;
specifically, relocate the file (the oval shape with stroke and transparent
solid) from app/src/main/res/drawable to the resources module at
resources/src/main/res/drawable, update any references to
R.drawable.bg_hollow_green_circle if needed (ensuring the shared resources
module is added as a dependency to modules that use it), and verify
builds/imports so layouts referencing bg_hollow_green_circle continue to
resolve.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 75c9dfb6-7c68-4222-8e81-b26cdceb78eb
📒 Files selected for processing (6)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/activities/editor/LandscapeImmersiveController.ktapp/src/main/res/drawable/bg_hollow_green_circle.xmlapp/src/main/res/layout-land/content_editor.xmlapp/src/main/res/layout/content_editor.xmlcommon/src/main/java/com/itsaky/androidide/ui/TouchObservingLinearLayout.kt
Comment on lines
+494
to
+503
| private fun applyStandardInsets(systemBars: Insets, windowInsets: WindowInsetsCompat) { | ||
| val content = _binding?.content ?: return | ||
|
|
||
| val appBarContent = content.editorAppbarContent | ||
| if (appBarContent != null) { | ||
| content.editorAppBarLayout.updatePadding(top = 0) | ||
| appBarContent.updatePadding(top = systemBars.top) | ||
| } else { | ||
| content.editorAppBarLayout.updatePadding(top = systemBars.top) | ||
| } |
Contributor
There was a problem hiding this comment.
Landscape drawer drag will now apply the top inset twice.
These lines move the status-bar padding onto editorAppbarContent, but onSwipeRevealDragProgress() still animates editorAppBarLayout padding. Once the drawer starts dragging in landscape, both containers contribute top inset and the toolbar jumps downward.
Possible fix
private fun onSwipeRevealDragProgress(progress: Float) {
_binding?.apply {
contentCard.progress = progress
val insetsTop = systemBarInsets?.top ?: 0
- content.editorAppBarLayout.updatePadding(
- top = (insetsTop * (1f - progress)).roundToInt(),
- )
+ val topInset = (insetsTop * (1f - progress)).roundToInt()
+ (content.editorAppbarContent ?: content.editorAppBarLayout).updatePadding(top = topInset)
memUsageView.chart.updateLayoutParams<ViewGroup.MarginLayoutParams> {
topMargin = (insetsTop * progress).roundToInt()
}
}
}🤖 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 494 - 503, applyStandardInsets currently moves the status-bar top
inset onto editorAppbarContent when present but onSwipeRevealDragProgress still
animates padding on editorAppBarLayout, causing double top padding during drawer
drag; fix by making both places target the same view: either keep
applyStandardInsets applying top inset to editorAppBarLayout and update
onSwipeRevealDragProgress to animate editorAppBarLayout, or (preferred) keep
applyStandardInsets applying the inset to editorAppbarContent and change
onSwipeRevealDragProgress so it animates editorAppbarContent instead of
editorAppBarLayout; update all references to updatePadding calls in
applyStandardInsets and the padding animation in onSwipeRevealDragProgress to
use the same view (editorAppbarContent or editorAppBarLayout) and remove the
duplicated top inset application.
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt
Show resolved
Hide resolved
Comment on lines
+128
to
+132
| fun onPause() { | ||
| cancelAutoHide() | ||
| isUserInteractingWithTopBar = false | ||
| cancelBottomSheetAnimation() | ||
| } |
Contributor
There was a problem hiding this comment.
Pausing the app cancels top-bar auto-hide permanently.
onPause() clears the auto-hide job, but there is no matching resume path that schedules it again when the top bar is still visible. Repro: show the top bar, background the app before 3.5s elapses, return — the bar stays pinned until another tap.
Possible fix
fun onPause() {
cancelAutoHide()
isUserInteractingWithTopBar = false
cancelBottomSheetAnimation()
}
+
+fun onResume() {
+ if (isLandscape && isTopBarRequestedVisible && !isUserInteractingWithTopBar) {
+ scheduleTopBarAutoHide()
+ }
+}And call it from BaseEditorActivity.onResume():
immersiveController?.onResume()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fun onPause() { | |
| cancelAutoHide() | |
| isUserInteractingWithTopBar = false | |
| cancelBottomSheetAnimation() | |
| } | |
| fun onPause() { | |
| cancelAutoHide() | |
| isUserInteractingWithTopBar = false | |
| cancelBottomSheetAnimation() | |
| } | |
| fun onResume() { | |
| if (isLandscape && isTopBarRequestedVisible && !isUserInteractingWithTopBar) { | |
| scheduleTopBarAutoHide() | |
| } | |
| } |
🤖 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/LandscapeImmersiveController.kt`
around lines 128 - 132, onPause() currently cancels the auto-hide job
(cancelAutoHide()) and clears isUserInteractingWithTopBar without a resume path,
so if the top bar was visible it will remain pinned after returning; add an
onResume() method on LandscapeImmersiveController that checks the top-bar
visibility state (e.g., isTopBarVisible or similar) and re-schedules the
auto-hide job (call the existing schedule/start auto-hide helper) only when the
top bar is visible and the user is not interacting, then invoke
immersiveController?.onResume() from BaseEditorActivity.onResume() to restore
the auto-hide behavior.
Comment on lines
+168
to
+188
| <View | ||
| android:id="@+id/btn_toggle_top_bar" | ||
| android:layout_width="24dp" | ||
| android:layout_height="24dp" | ||
| android:layout_gravity="top|end" | ||
| android:layout_marginTop="16dp" | ||
| android:layout_marginEnd="16dp" | ||
| android:background="@drawable/bg_hollow_green_circle" | ||
| android:visibility="gone" | ||
| tools:visibility="visible" /> | ||
|
|
||
| <View | ||
| android:id="@+id/btn_toggle_bottom_bar" | ||
| android:layout_width="24dp" | ||
| android:layout_height="24dp" | ||
| android:layout_gravity="bottom|end" | ||
| android:layout_marginBottom="16dp" | ||
| android:layout_marginEnd="16dp" | ||
| android:background="@drawable/bg_hollow_green_circle" | ||
| android:visibility="gone" | ||
| tools:visibility="visible" /> |
Contributor
There was a problem hiding this comment.
These toggles need real button semantics and a larger hit area.
They are interactive in immersive mode, but plain Views at 24dp here (32dp in the default variant) expose no accessible role/label and miss the 48dp minimum touch target. That makes the bars hard or impossible to toggle for TalkBack and motor-impaired users. Please switch both layout variants to ImageButton/MaterialButton (or similar) and add contentDescription.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/res/layout-land/content_editor.xml` around lines 168 - 188,
Replace the two plain View elements with accessible clickable widgets (e.g.,
ImageButton or MaterialButton) for btn_toggle_top_bar and btn_toggle_bottom_bar,
make them focusable/clickable, add appropriate android:contentDescription for
TalkBack, and ensure a minimum 48dp touch target by increasing
layout_width/layout_height or adding extra padding (or wrap with a larger touch
container) while preserving the existing background drawable
(bg_hollow_green_circle) and visibility attributes; apply the same changes to
both layout variants so the toggles gain proper role, label, and hit area.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
app/src/main/res/layout-land/content_editor.xml (1)
168-188:⚠️ Potential issue | 🟠 MajorToggle buttons lack accessible semantics and minimum touch target.
These toggle
Viewelements at 24×24dp expose no accessible role, label, or focus state, and miss the 48dp minimum touch target recommended by Android accessibility guidelines. This makes the toggles difficult or impossible to use for TalkBack and motor-impaired users.Consider switching to
ImageButtonwithcontentDescription, and ensure a minimum 48dp touch area (via size or padding).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/layout-land/content_editor.xml` around lines 168 - 188, Replace the bare 24×24dp View toggles (btn_toggle_top_bar and btn_toggle_bottom_bar) with accessible clickable controls (e.g., ImageButton or AppCompatImageButton), add contentDescription for TalkBack, set clickable="true" and focusable="true" (or android:importantForAccessibility="yes"), and ensure a minimum 48dp touch target by increasing layout_width/layout_height to 48dp or keeping 24dp visual size but adding 12dp padding/inset to reach 48dp; also preserve the bg_hollow_green_circle background and gravity/margins so layout position is unchanged.app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt (3)
854-871:⚠️ Potential issue | 🟡 MinorMissing
onResume()call to restore auto-hide behavior.
onPause()(line 845) cancels the top-bar auto-hide job, butonResume()doesn't reschedule it. If the user backgrounds the app while the top bar is visible, upon return the bar will remain pinned indefinitely.Suggested fix
Add a call to restore auto-hide in
onResume():override fun onResume() { super.onResume() invalidateOptionsMenu() + immersiveController?.onResume() memoryUsageWatcher.listener = memoryUsageListenerAnd add
onResume()toLandscapeImmersiveController:fun onResume() { if (isLandscape && isTopBarRequestedVisible && !isUserInteractingWithTopBar) { scheduleTopBarAutoHide() } }🤖 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 854 - 871, onResume() in BaseEditorActivity doesn't restore the top-bar auto-hide scheduled by onPause(); update BaseEditorActivity.onResume() to call the immersive controller's resume logic and add a new LandscapeImmersiveController.onResume() that re-schedules the auto-hide when appropriate. Specifically, in BaseEditorActivity.onResume() invoke the controller method (e.g., landscapeImmiveController.onResume()) after other resume work, and implement LandscapeImmersiveController.onResume() to check isLandscape, isTopBarRequestedVisible and !isUserInteractingWithTopBar and call scheduleTopBarAutoHide() when those conditions are met.
749-760:⚠️ Potential issue | 🟡 MinorDrawer drag still applies top inset to
editorAppBarLayoutinstead ofeditorAppbarContent.
applyStandardInsets()(lines 497-503) conditionally applies the status-bar padding toeditorAppbarContentwhen present, butonSwipeRevealDragProgress()still animates padding oneditorAppBarLayout. In landscape, both containers will contribute top inset during drawer drag, causing the toolbar to jump.Suggested fix
private fun onSwipeRevealDragProgress(progress: Float) { _binding?.apply { contentCard.progress = progress val insetsTop = systemBarInsets?.top ?: 0 - content.editorAppBarLayout.updatePadding( - top = (insetsTop * (1f - progress)).roundToInt(), - ) + val targetView = content.editorAppbarContent ?: content.editorAppBarLayout + targetView.updatePadding( + top = (insetsTop * (1f - progress)).roundToInt(), + ) memUsageView.chart.updateLayoutParams<ViewGroup.MarginLayoutParams> { topMargin = (insetsTop * progress).roundToInt() } } }🤖 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 749 - 760, onSwipeRevealDragProgress is animating top padding on editorAppBarLayout which duplicates applyStandardInsets behavior that applies status-bar inset to editorAppbarContent when present; change the padding update to target editorAppbarContent when that view exists (fall back to editorAppBarLayout only if editorAppbarContent is null) so the animated inset is applied to the same container used by applyStandardInsets (update the content.editorAppBarLayout.updatePadding call to conditionally update content.editorAppbarContent.updatePadding instead), leaving the memUsageView.chart topMargin animation unchanged.
693-696:⚠️ Potential issue | 🟠 MajorLandscape immersive features require layout rebind after in-place rotation.
The activity handles
orientation|screenSizeviaconfigChanges, so it won't recreate on rotation. The portrait layout lackseditor_appbar_content(theTouchObservingLinearLayoutwrapper), meaning touch observation and scroll-based collapse won't function after rotating from portrait to landscape without rebinding.Consider either adding
editor_appbar_contentto the portrait layout (even if with different scroll flags) or implementing a rebind/reinflate path inonConfigurationChanged().🤖 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 693 - 696, The portrait->landscape rotation leaves the UI without the TouchObservingLinearLayout wrapper ("editor_appbar_content"), so after in-place rotation touch observation and scroll-based collapse break; update BaseEditorActivity.onConfigurationChanged to rebind the layout components after rotation (or alternatively add editor_appbar_content to the portrait layout). Specifically, in onConfigurationChanged (in BaseEditorActivity) either inflate/re-attach the layout that contains editor_appbar_content and re-run the findViewById/initialization logic for the TouchObservingLinearLayout and AppBar behaviors, or modify the portrait XML to include the editor_appbar_content wrapper (TouchObservingLinearLayout) so the view hierarchy is consistent across orientations; ensure immersiveController is reinitialized/connected to the new view references after the rebind.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/activities/editor/LandscapeImmersiveController.kt (1)
38-44: View binding access usesrunCatchingbut could fail silently.Line 39 wraps
editorAppbarContentaccess withrunCatching { ... }.getOrNull(), which swallows any exception and returns null. While this handles the portrait layout case where the view doesn't exist, it also silently ignores other potential issues. Consider using direct nullable access if available via view binding, or at minimum logging when the view is unexpectedly missing.🤖 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/LandscapeImmersiveController.kt` around lines 38 - 44, Replace the runCatching/getOrNull pattern around contentBinding.editorAppbarContent in LandscapeImmersiveController with a deterministic check and logging: attempt direct access to editorAppbarContent (or fallback to contentBinding.root.findViewById(...) if the generated binding property isn't nullable), assign the result to appBarContent, and if it's null log a warning (use the class's logger or Android Log) so missing-view cases are explicit instead of silently swallowed; remove runCatching and getOrNull and reference the editorAppbarContent symbol in the fix so the change is easy to find.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt`:
- Around line 854-871: onResume() in BaseEditorActivity doesn't restore the
top-bar auto-hide scheduled by onPause(); update BaseEditorActivity.onResume()
to call the immersive controller's resume logic and add a new
LandscapeImmersiveController.onResume() that re-schedules the auto-hide when
appropriate. Specifically, in BaseEditorActivity.onResume() invoke the
controller method (e.g., landscapeImmiveController.onResume()) after other
resume work, and implement LandscapeImmersiveController.onResume() to check
isLandscape, isTopBarRequestedVisible and !isUserInteractingWithTopBar and call
scheduleTopBarAutoHide() when those conditions are met.
- Around line 749-760: onSwipeRevealDragProgress is animating top padding on
editorAppBarLayout which duplicates applyStandardInsets behavior that applies
status-bar inset to editorAppbarContent when present; change the padding update
to target editorAppbarContent when that view exists (fall back to
editorAppBarLayout only if editorAppbarContent is null) so the animated inset is
applied to the same container used by applyStandardInsets (update the
content.editorAppBarLayout.updatePadding call to conditionally update
content.editorAppbarContent.updatePadding instead), leaving the
memUsageView.chart topMargin animation unchanged.
- Around line 693-696: The portrait->landscape rotation leaves the UI without
the TouchObservingLinearLayout wrapper ("editor_appbar_content"), so after
in-place rotation touch observation and scroll-based collapse break; update
BaseEditorActivity.onConfigurationChanged to rebind the layout components after
rotation (or alternatively add editor_appbar_content to the portrait layout).
Specifically, in onConfigurationChanged (in BaseEditorActivity) either
inflate/re-attach the layout that contains editor_appbar_content and re-run the
findViewById/initialization logic for the TouchObservingLinearLayout and AppBar
behaviors, or modify the portrait XML to include the editor_appbar_content
wrapper (TouchObservingLinearLayout) so the view hierarchy is consistent across
orientations; ensure immersiveController is reinitialized/connected to the new
view references after the rebind.
In `@app/src/main/res/layout-land/content_editor.xml`:
- Around line 168-188: Replace the bare 24×24dp View toggles (btn_toggle_top_bar
and btn_toggle_bottom_bar) with accessible clickable controls (e.g., ImageButton
or AppCompatImageButton), add contentDescription for TalkBack, set
clickable="true" and focusable="true" (or
android:importantForAccessibility="yes"), and ensure a minimum 48dp touch target
by increasing layout_width/layout_height to 48dp or keeping 24dp visual size but
adding 12dp padding/inset to reach 48dp; also preserve the
bg_hollow_green_circle background and gravity/margins so layout position is
unchanged.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/LandscapeImmersiveController.kt`:
- Around line 38-44: Replace the runCatching/getOrNull pattern around
contentBinding.editorAppbarContent in LandscapeImmersiveController with a
deterministic check and logging: attempt direct access to editorAppbarContent
(or fallback to contentBinding.root.findViewById(...) if the generated binding
property isn't nullable), assign the result to appBarContent, and if it's null
log a warning (use the class's logger or Android Log) so missing-view cases are
explicit instead of silently swallowed; remove runCatching and getOrNull and
reference the editorAppbarContent symbol in the fix so the change is easy to
find.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ca02ac84-6dd8-4984-870b-557eca13aa21
📒 Files selected for processing (6)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/activities/editor/LandscapeImmersiveController.ktapp/src/main/res/drawable/bg_hollow_green_circle.xmlapp/src/main/res/layout-land/content_editor.xmlapp/src/main/res/layout/content_editor.xmlcommon/src/main/java/com/itsaky/androidide/ui/TouchObservingLinearLayout.kt
✅ Files skipped from review due to trivial changes (1)
- common/src/main/java/com/itsaky/androidide/ui/TouchObservingLinearLayout.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/res/drawable/bg_hollow_green_circle.xml
- app/src/main/res/layout/content_editor.xml
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
1 participant
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 introduces an immersive mode for the editor when in landscape orientation. It adds the
LandscapeImmersiveControllerto manage the auto-hiding and toggling of the top and bottom toolbars. A newTouchObservingLinearLayoutwas implemented to wrap the app bar content, ensuring that the auto-hide delay is paused while the user is actively interacting with the top bar. Additionally, system window inset handling was updated to properly apply padding and margins during the immersive state.Details
content_editor.xml(using a new hollow green circle drawable).BottomSheetBehavior.Screen_Recording_20260320_161536_Code.on.the.Go.mp4
Ticket
ADFA-3048
Observation
The
LandscapeImmersiveControllerbinds directly toAppBarLayout.OnOffsetChangedListenerandBottomSheetBehavior.BottomSheetCallbackto calculate dynamic layout changes smoothly. Note that the toggle buttons are hidden by default in portrait mode and managed dynamically when configuration changes occur.