ADFA-4320 Paper cut: Fix for swiping bottom sheet tabs incorrectly opening file menu#1390
ADFA-4320 Paper cut: Fix for swiping bottom sheet tabs incorrectly opening file menu#1390hal-eisen-adfa wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthrough
ChangesDrawer Gesture Suppression for Bottom-Sheet Tabs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt`:
- Around line 271-285: The drawerLockCallback object only updates the drawer
lock mode when onStateChanged is invoked, which means if the bottom sheet is
already in an expanded or half-expanded state when the callback is registered,
the drawer will remain unlocked until the next state transition occurs. After
the callback registration at the .also block where
behavior.addBottomSheetCallback is called, immediately apply the drawer lock
mode based on the bottom sheet behavior's current state using the same lock
logic (checking if the current state is STATE_COLLAPSED or STATE_HIDDEN to
unlock, otherwise lock). This ensures the drawer lock state is correct
regardless of whether the sheet is already expanded when the callback is
attached.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ccadc028-3ecf-4f41-a82a-dfc0dd0b2135
📒 Files selected for processing (1)
app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt
Tab-strip flings have enough horizontal velocity to trigger the editor activity's drawer-open gesture detector, opening the file tree when the user is just navigating between bottom-pane tabs. In dispatchTouchEvent, check on ACTION_DOWN whether the touch falls inside the bottom sheet TabLayout's global visible rect; if so, stop forwarding the touch sequence to the gesture detector. The hamburger button and the existing no-files horizontal-fling shortcut remain untouched. Reverts the bottom-sheet-state drawer lock from da1e435 — that approach broke the hamburger because ActionBarDrawerToggle treats LOCK_MODE_LOCKED_CLOSED as "do not open" and the lock also fired on gestures outside the tab strip.
Code review surfaced three issues with the previous implementation: 1. content.bottomSheet.binding.tabs reads a binding chain whose root getter throws IllegalStateException once _binding is cleared in preDestroy. Touches dispatched between preDestroy and window teardown would crash. Switched to contentOrNull, the file's existing defensive helper. 2. The dispatchTouchEvent flag was set only on ACTION_DOWN, so ACTION_POINTER_DOWN (multi-touch) could not re-evaluate it; a second finger could leak past or stay stuck-suppressed. 3. The check belonged with the other fling filters (startedNearTopEdge, noFilesOpen, etc.) in onFling, not in dispatchTouchEvent — and onFling already tracks the down event as e1, so no extra state is needed. Folded the check into the isDrawerOpenFling branch using e1. Dropped suppressDrawerGesture and bottomSheetTabsHitRect fields; restored dispatchTouchEvent to its pre-PR form. The hit-test now runs only when a drawer-open fling is actually detected, not on every ACTION_DOWN.
Problem
Fling-strength swipes between bottom-pane tabs (Build Output / IDE Logs / Diagnostics / Git etc.) were triggering
BaseEditorActivity's drawer-open gesture detector and popping the file tree open mid-swipe. No edge proximity required — pure horizontal velocity on the tab strip.Change
In
BaseEditorActivity.onFling, the existingisDrawerOpenFlingbranch now also checks!isTouchOnBottomSheetTabs(e1)before callingopenDrawer. The hit-test readscontentOrNull?.bottomSheet?.binding?.tabs(safe during teardown), grabs the TabLayout'sgetGlobalVisibleRect, and tests(e1.rawX, e1.rawY)against it.The check lives alongside the other fling filters (
startedNearTopEdge,noFilesOpen, …) so the suppression rule is visible to anyone editingsetupGestureDetector.Scope
Test plan