Skip to content

fix(ADFA-3943): Avoid collapsing the toolbar when in windowed mode#1332

Open
dara-abijo-adfa wants to merge 1 commit into
stagefrom
ADFA-3943-unexpected-fullscreen-mode
Open

fix(ADFA-3943): Avoid collapsing the toolbar when in windowed mode#1332
dara-abijo-adfa wants to merge 1 commit into
stagefrom
ADFA-3943-unexpected-fullscreen-mode

Conversation

@dara-abijo-adfa
Copy link
Copy Markdown
Contributor

  • Remove the scroll flags from the toolbar when app is in not in fullscreen mode
  • Re-add the scroll flags before toggling fullscreen mode on

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 22, 2026

Review Change Stack

📝 Walkthrough

Release Notes

Bug Fixes

  • Toolbar Pinning in Windowed Mode: The toolbar and tabs are now pinned to the top of the screen when the app is not in fullscreen mode, preventing unwanted collapse/scroll behavior in normal operation.
  • Scroll Flags Management: Scroll flags are dynamically removed from the toolbar when the app bar is fully expanded in windowed mode, and re-applied when entering fullscreen mode.

Technical Changes

  • Introduced FullscreenManager class to centralize fullscreen mode state management and transitions.
  • Implemented effective scroll range calculation that falls back to app bar height when total scroll range is invalid.
  • Added dynamic bottom margin adjustment for the editor container based on visible app bar height in windowed mode.
  • Utilized sealed interface pattern for UI state and render command management, improving type safety and state transitions.

Risks & Best Practices Considerations

  • Performance: The offsetListener updates layout parameters on every scroll event (bottomMargin changes during scrolling). This frequent layout modification could impact scroll performance on lower-end devices. Consider monitoring frame drops during scrolling.
  • State Synchronization: The isFullscreenState mutable variable must be kept in sync with the actual UI state through syncToggleUi(). Any missing calls to this method could lead to inconsistent behavior.
  • Layout Parameter Casting: Direct casting of layout params (lines 134-137) assumes correct layout type. If layout hierarchy changes, this could cause runtime exceptions. Consider adding type-safe wrappers.
  • Integration Point: New FullscreenManager is instantiated in BaseEditorActivity. Verify that the manager's lifecycle (bind() and destroy()) is properly called during activity setup and teardown.
  • Architectural Complexity: While the sealed interface command pattern is robust, it adds complexity. Ensure thorough testing of all state transitions, particularly during rapid toggle operations.

Walkthrough

FullscreenManager enhances AppBar scroll behavior by introducing fallback logic for scroll range calculations in the offset listener and integrating scroll flag setup into fullscreen transitions. The listener now computes an effective scroll range to handle edge cases and manages flag clearing when the app bar is fully expanded.

Changes

AppBar Scroll and Fullscreen Behavior

Layer / File(s) Summary
AppBar offset listener scroll range and flag management
app/src/main/java/com/itsaky/androidide/activities/editor/FullscreenManager.kt
The offsetListener now computes an effectiveScrollRange that falls back to appBarLayout.height when totalScrollRange is non-positive, uses this range to drive appBarContent alpha and editor container bottom margin, and clears appBarContent scroll flags when the app bar reaches full expansion outside fullscreen mode.
Fullscreen transition scroll flag setup
app/src/main/java/com/itsaky/androidide/activities/editor/FullscreenManager.kt
applyFullscreen() now invokes setupScrollFlags() at the start of the fullscreen transition to re-apply scroll flag configuration before collapsing the top bar and hiding the bottom sheet.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • appdevforall/CodeOnTheGo#1295: Both PRs modify FullscreenManager.kt with overlapping changes to AppBar offsetListener logic and fullscreen state handling.

Suggested reviewers

  • jatezzz
  • Daniel-ADFA
  • jomen-adfa
  • itsaky-adfa

Poem

A rabbit hops through scroll ranges swift,
With fallback logic—quite a gift!
When bars expand and fullscreen calls,
Flags reset within these halls. 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: preventing toolbar collapse in windowed (non-fullscreen) mode, which aligns with the core objective of removing scroll flags when not in fullscreen.
Description check ✅ Passed The description directly relates to the changeset by explaining both modifications: removing scroll flags in windowed mode and re-adding them before fullscreen mode activation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 ADFA-3943-unexpected-fullscreen-mode

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/activities/editor/FullscreenManager.kt (1)

133-139: ⚡ Quick win

Consider adding a comment to explain the scroll flag clearing logic.

The logic correctly pins the toolbar once fully expanded in windowed mode, but the intent could be clearer for future maintainers. A brief comment explaining that this prevents the toolbar from collapsing again until entering fullscreen would improve readability.

📝 Suggested comment
         if (verticalOffset == 0) {
+            // Pin the toolbar in windowed mode by clearing scroll flags once fully expanded.
+            // Flags are restored when entering fullscreen (see applyFullscreen).
             val params = appBarContent.layoutParams as AppBarLayout.LayoutParams
             if (params.scrollFlags != 0) {
                 params.scrollFlags = 0
🤖 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
`@app/src/main/java/com/itsaky/androidide/activities/editor/FullscreenManager.kt`
around lines 133 - 139, Add a brief inline comment above the verticalOffset == 0
branch in FullscreenManager (where appBarContent.layoutParams is cast to
AppBarLayout.LayoutParams and params.scrollFlags is set to 0) explaining that
clearing params.scrollFlags pins the toolbar when the AppBar is fully expanded
(verticalOffset == 0) to prevent it from collapsing again in windowed mode until
entering fullscreen; reference appBarContent, params.scrollFlags,
AppBarLayout.LayoutParams and verticalOffset in the comment so future
maintainers understand the intent.
🤖 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.

Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/FullscreenManager.kt`:
- Around line 133-139: Add a brief inline comment above the verticalOffset == 0
branch in FullscreenManager (where appBarContent.layoutParams is cast to
AppBarLayout.LayoutParams and params.scrollFlags is set to 0) explaining that
clearing params.scrollFlags pins the toolbar when the AppBar is fully expanded
(verticalOffset == 0) to prevent it from collapsing again in windowed mode until
entering fullscreen; reference appBarContent, params.scrollFlags,
AppBarLayout.LayoutParams and verticalOffset in the comment so future
maintainers understand the intent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c4a6b8ea-e350-482e-8dcb-e75aef8f8e1c

📥 Commits

Reviewing files that changed from the base of the PR and between a479086 and e6c9776.

📒 Files selected for processing (1)
  • app/src/main/java/com/itsaky/androidide/activities/editor/FullscreenManager.kt

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.

2 participants