ADFA-3569 | Refactor editor fullscreen state handling#1149
Open
ADFA-3569 | Refactor editor fullscreen state handling#1149
Conversation
jatezzz
requested review from
a team,
Daniel-ADFA,
dara-abijo-adfa and
itsaky-adfa
Contributor
📝 WalkthroughRelease Notes - Editor Fullscreen State Handling RefactorFeatures & Improvements
Changes Summary
|
| Cohort / File(s) | Summary |
|---|---|
Fullscreen State Management app/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.kt, app/src/main/java/com/itsaky/androidide/activities/editor/FullscreenManager.kt |
Added UiState model and StateFlow<UiState> to EditorViewModel with isFullscreen property and control methods (toggleFullscreen(), exitFullscreen()). New FullscreenManager class orchestrates fullscreen/windowed transitions, manages AppBarLayout offset listeners, BottomSheetBehavior callbacks, and animates layout/visibility changes. |
Editor Activity Core Logic app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt |
Replaced immersiveController with fullscreenManager in lifecycle management (onCreate(), preDestroy()). Added setupFullscreenObserver() to render UI state while activity is STARTED. Refactored gesture/swipe detection: added upward-inward dismissal from bottom edge and inward dismissal from top edge (both trigger exitFullscreen()), tightened drawer-open fling conditions, removed old showTopBar() calls. |
Removed Immersive Controller app/src/main/java/com/itsaky/androidide/activities/editor/LandscapeImmersiveController.kt |
Entire class removed (344 lines). Previously managed landscape immersive behavior, app bar offset binding, bottom sheet callbacks, and top/bottom bar toggle buttons. |
Bottom Sheet UI Behavior app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt |
Simplified onSlide() callback: removed COLLAPSE_HEADER_AT_OFFSET constant and piecewise scaling logic; now uses normalized clamping [0f, 1f] for linear heightScale and paddingScale calculations. |
Inset & Layout Utilities app/src/main/java/com/itsaky/androidide/utils/WindowInsetsExtensions.kt |
Removed orientation-based padding branching in applyResponsiveAppBarInsets(). Replaced btnToggleTopBar/btnToggleBottomBar layout parameter updates with btnFullscreenToggle adjustments. Removed refreshBottomSheetAnchor() and resetBottomSheetAnchor() public functions; applyBottomSheetAnchorForOrientation() now unconditionally calls resetOffsetAnchor(). |
Layout Resources app/src/main/res/layout/content_editor.xml, app/src/main/res/layout-land/content_editor.xml |
Changed editor container and bottom sheet dimensions to use editor_sheet_collapsed_height instead of editor_sheet_peek_height. Replaced hidden btn_toggle_top_bar and btn_toggle_bottom_bar with visible btn_fullscreen_toggle (48dp, ic_fullscreen icon, updated content description). |
Icon Resources resources/src/main/res/drawable/ic_fullscreen.xml, resources/src/main/res/drawable/ic_fullscreen_exit.xml |
Added two new vector drawable resources for fullscreen entry/exit icons (24dp, tinted with colorControlNormal). |
Removed & Updated Resources app/src/main/res/drawable/bg_hollow_green_circle.xml, resources/src/main/res/values/strings.xml, resources/src/main/res/values/dimens.xml |
Removed hollow green circle drawable. Updated string resources: removed desc_toggle_top_bar/desc_toggle_bottom_bar, added desc_enter_fullscreen/desc_exit_fullscreen. Increased editor_sheet_collapsed_height from 56dp to 100dp. |
Sequence Diagram
sequenceDiagram
participant User
participant BaseEditorActivity
participant FullscreenManager
participant EditorViewModel
participant AppBarLayout
participant BottomSheetBehavior
User->>BaseEditorActivity: onCreate()
BaseEditorActivity->>FullscreenManager: constructor + bind()
FullscreenManager->>AppBarLayout: registerOnOffsetChangedListener()
FullscreenManager->>BottomSheetBehavior: addBottomSheetCallback()
FullscreenManager->>BaseEditorActivity: onFullscreenToggleRequested callback
BaseEditorActivity->>EditorViewModel: setupFullscreenObserver()
EditorViewModel-->>BaseEditorActivity: uiState.isFullscreen updates
User->>FullscreenManager: toggleFullscreenButton clicked
FullscreenManager->>EditorViewModel: onFullscreenToggleRequested()
EditorViewModel->>EditorViewModel: toggleFullscreen()
EditorViewModel-->>BaseEditorActivity: emit new uiState
BaseEditorActivity->>FullscreenManager: render(isFullscreen=true, animate=true)
FullscreenManager->>AppBarLayout: collapse/expand with animation
FullscreenManager->>BottomSheetBehavior: hide/show sheet
FullscreenManager->>BottomSheetBehavior: update hideable state
FullscreenManager->>BaseEditorActivity: closeDrawerAction callback
User->>User: Swipe upward from bottom (fullscreen mode)
BaseEditorActivity->>EditorViewModel: exitFullscreen()
EditorViewModel-->>FullscreenManager: render(isFullscreen=false, animate=true)
BaseEditorActivity->>FullscreenManager: preDestroy()
FullscreenManager->>AppBarLayout: unregisterOnOffsetChangedListener()
FullscreenManager->>BottomSheetBehavior: removeBottomSheetCallback()
FullscreenManager->>FullscreenManager: destroy()
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
This change involves heterogeneous modifications across multiple architectural layers: new stateful component creation (FullscreenManager), ViewModel state-management patterns (StateFlow + UiState), complex lifecycle synchronization, gesture-detection logic with new conditions, removal of a 344-line class requiring verification of all reference removal, and coordinated layout/resource updates. The interdependencies between components and the density of logic in FullscreenManager (listeners, callbacks, render state machine, animation coordination) demand careful verification of state transitions and side-effect ordering.
Possibly related PRs
- ADFA-3048 | Implement landscape immersive mode for editor #1101 — Directly removes and replaces
LandscapeImmersiveControllerwithFullscreenManagerand updates UI wiring for toggle buttons and inset handling. - ADFA-2646 | Fix window insets accumulation and landscape appbar behavior #1114 — Modifies
WindowInsetsExtensionsandContentEditorBindingusage for bottom-sheet anchoring and app bar inset application in the same activity. - ADFA-1788 | Fix sidebar overlap with status bar on split-screen resize #840 — Both modify
BaseEditorActivity's window/system-bar inset handling and replace immersive controller logic with alternative state management.
Suggested reviewers
- itsaky-adfa
- jomen-adfa
- dara-abijo-adfa
- Daniel-ADFA
Poem
🐰 A fullscreen dream we've long awaited,
No more landscape, immerse-related,
With gestures swift and buttons bright,
The editor flows day and night,
State flows smooth, the controller's gone,
Refactored clean, let's press on! ✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 7.50% 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 accurately and concisely describes the main change: refactoring editor fullscreen state handling by replacing the old immersive controller with a new manager. |
| Description check | ✅ Passed | The description clearly explains the refactoring of immersive mode, replacement of LandscapeImmersiveController with FullscreenManager, centralized state management via EditorViewModel with StateFlow, and consolidation of toggle buttons. |
✏️ 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
feat/ADFA-3569-fullscreen-portrait-and-landscape
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.
🧹 Nitpick comments (2)
app/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.kt (1)
62-67: Consider consolidating to a single state source.The fullscreen state is maintained in both
_isFullscreen(LiveData) and_uiState(StateFlow). WhileupdateFullscreenkeeps them synchronized, this dual-source pattern increases maintenance overhead and risk of divergence.If the LiveData is still needed for legacy observers, consider deriving it from the StateFlow or document why both are required. Otherwise, consolidating to StateFlow alone would simplify the state management.
🤖 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/viewmodel/EditorViewModel.kt` around lines 62 - 67, Consolidate fullscreen state into the single source _uiState: remove the backing MutableLiveData _isFullscreen and have updateFullscreen update only _uiState (UiState.fullscreen flag); if legacy LiveData observers are required, expose a derived LiveData by converting _uiState (e.g., via asLiveData or a mapping) so observers read from that derived LiveData instead of a separate _isFullscreen, ensuring updateFullscreen no longer needs to sync two sources and eliminating divergence risk.app/src/main/java/com/itsaky/androidide/utils/WindowInsetsExtensions.kt (1)
109-113: Unused parameterorientationshould be removed.The
orientationparameter is no longer used after simplifying to always callresetOffsetAnchor(). This creates a misleading API signature.♻️ Proposed fix to remove unused parameter
-fun ContentEditorBinding.applyBottomSheetAnchorForOrientation(orientation: Int) { +fun ContentEditorBinding.applyBottomSheetAnchor() { bottomSheet.resetOffsetAnchor() }Note: This would also require updating the call sites in
BaseEditorActivity.kt(lines 679 and 1402).🤖 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 109 - 113, Remove the unused orientation parameter from the function signature of ContentEditorBinding.applyBottomSheetAnchorForOrientation and update all call sites to the new zero-arg function; specifically, change the method to take no parameters and keep the body as bottomSheet.resetOffsetAnchor(), then update callers (e.g., in BaseEditorActivity where applyBottomSheetAnchorForOrientation(...) is invoked) to call applyBottomSheetAnchorForOrientation() with no argument. Ensure method references/imports/overloads are adjusted accordingly so there are no compilation errors.
🤖 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/utils/WindowInsetsExtensions.kt`:
- Around line 109-113: Remove the unused orientation parameter from the function
signature of ContentEditorBinding.applyBottomSheetAnchorForOrientation and
update all call sites to the new zero-arg function; specifically, change the
method to take no parameters and keep the body as
bottomSheet.resetOffsetAnchor(), then update callers (e.g., in
BaseEditorActivity where applyBottomSheetAnchorForOrientation(...) is invoked)
to call applyBottomSheetAnchorForOrientation() with no argument. Ensure method
references/imports/overloads are adjusted accordingly so there are no
compilation errors.
In `@app/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.kt`:
- Around line 62-67: Consolidate fullscreen state into the single source
_uiState: remove the backing MutableLiveData _isFullscreen and have
updateFullscreen update only _uiState (UiState.fullscreen flag); if legacy
LiveData observers are required, expose a derived LiveData by converting
_uiState (e.g., via asLiveData or a mapping) so observers read from that derived
LiveData instead of a separate _isFullscreen, ensuring updateFullscreen no
longer needs to sync two sources and eliminating divergence risk.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ecb6ff41-e949-4b60-9a4a-162f92e75e9c
📒 Files selected for processing (13)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/activities/editor/FullscreenManager.ktapp/src/main/java/com/itsaky/androidide/activities/editor/LandscapeImmersiveController.ktapp/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.ktapp/src/main/java/com/itsaky/androidide/utils/WindowInsetsExtensions.ktapp/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.ktapp/src/main/res/drawable/bg_hollow_green_circle.xmlapp/src/main/res/layout-land/content_editor.xmlapp/src/main/res/layout/content_editor.xmlresources/src/main/res/drawable/ic_fullscreen.xmlresources/src/main/res/drawable/ic_fullscreen_exit.xmlresources/src/main/res/values/dimens.xmlresources/src/main/res/values/strings.xml
💤 Files with no reviewable changes (2)
- app/src/main/res/drawable/bg_hollow_green_circle.xml
- app/src/main/java/com/itsaky/androidide/activities/editor/LandscapeImmersiveController.kt
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
Refactored the editor's immersive mode by replacing the orientation-specific
LandscapeImmersiveControllerwith a universalFullscreenManager. Fullscreen state is now centrally managed and observed viaEditorViewModelusing aStateFlow. Additionally, the disparate top and bottom bar toggle buttons were replaced with a single, unified fullscreen toggle button.Details
Dex/Tablet
document_5075574213318805643.mp4
Phone
Screen_Recording_20260402_161348_Code.on.the.Go.mp4
Ticket
ADFA-3569
Observation
The previous implementation restricted immersive capabilities to landscape mode and relied on direct view manipulation. Moving the state to the ViewModel and utilizing a dedicated
FullscreenManagercreates a more predictable, reactive, and orientation-independent user experience.