- 
                Notifications
    
You must be signed in to change notification settings  - Fork 1.4k
 
fix(iOS): swipe back adds weird padding to Messages List #6676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
          
WalkthroughAdds a new iOS back-swipe handling hook and integrates it into the message composer input; the composer checks the hook's  Changes
 Sequence Diagram(s)sequenceDiagram
  autonumber
  actor U as User (iOS)
  participant N as Navigation
  participant H as useIOSBackSwipeHandler
  participant C as ComposerInput
  participant D as Device / Keyboard
  U->>N: Begin back-swipe (navigate back)
  N-->>H: transitionStart (closing)
  activate H
  H->>H: set iOSBackSwipe.current = true
  deactivate H
  U->>C: Input loses focus (blur)
  C->>H: check iOSBackSwipe.current
  alt Back-swipe active
    C-->>C: skip clearing focus / stop autocomplete
    C->>H: (may) let hook handle blur
    H->>D: dismiss keyboard (blur handler)
  else No back-swipe
    C-->>C: clear focus and stop autocomplete
    C->>D: dismiss keyboard (normal)
  end
  N-->>H: transitionEnd (closing finished)
  H->>H: set iOSBackSwipe.current = false
  Note over H,C: listeners removed on unmount
    Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
 Pre-merge checks and finishing touches✅ Passed checks (5 passed)
 ✨ Finishing touches
 🧪 Generate unit tests (beta)
 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources: 
 
 📒 Files selected for processing (1)
 🚧 Files skipped from review as they are similar to previous changes (1)
 ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
 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   | 
    
| 
           iOS Build Available Rocket.Chat 4.65.0.107359  | 
    
| 
           iOS Build Available Rocket.Chat Experimental 4.65.0.107497  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/containers/MessageComposer/hooks/useIOSBackSwipeHandler.ts (1)
14-14: Consider adding type definitions for transition events.Using
as anybypasses TypeScript's type checking. While this works, it reduces type safety and IDE support. Consider defining proper types for the transition events:type TransitionEvent = { data?: { closing?: boolean; }; }; const transitionStartListener = navigation.addListener( 'transitionStart' as any, (e: TransitionEvent) => { if (e?.data?.closing) { iOSBackSwipe.current = true; Keyboard.dismiss(); } } );Apply similar typing to the
transitionEndlistener. This provides better IDE autocomplete and catches potential bugs at compile time.Also applies to: 21-21
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
 
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/containers/MessageComposer/hooks/useIOSBackSwipeHandler.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
 
🔇 Additional comments (1)
app/containers/MessageComposer/hooks/useIOSBackSwipeHandler.ts (1)
14-19: LGTM! Correctly handles the start of back-swipe gestures.The logic properly:
- Sets the flag only during closing transitions (back navigation)
 - Dismisses the keyboard immediately to prevent layout issues
 - Uses optional chaining to safely check the event data
 
| 
           iOS Build Available Rocket.Chat Experimental 4.65.0.107508  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/containers/MessageComposer/hooks/useIOSBackSwipeHandler.ts (1)
14-14: Consider replacingas anywith properly typed event names.The
as anyassertions bypass TypeScript checks. If React Navigation v7 exports types fortransitionStartandtransitionEndevents (or you can define them), replace these assertions for better type safety.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
 
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/containers/MessageComposer/hooks/useIOSBackSwipeHandler.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
 - GitHub Check: format
 
🔇 Additional comments (1)
app/containers/MessageComposer/hooks/useIOSBackSwipeHandler.ts (1)
21-23: LGTM! Correctly resets the flag after any transition.Unconditionally resetting
iOSBackSwipe.currentensures the flag is cleared whether the back-swipe completes or is cancelled. This addresses the critical bug flagged in previous reviews.
| 
           iOS Build Available Rocket.Chat Experimental 4.65.0.107509  | 
    
| 
           iOS Build Available Rocket.Chat Experimental 4.65.0.107510  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working fine here!
Can you test Android just in case?
I'm just missing a root cause analysis.
What introduced this issue? Did you create an issue on another repo, so we can track and remove this workaround in the future?
| 
           Android Build Available Rocket.Chat Experimental 4.65.0.107513 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTHfsqQASVlbcH39tJeTUlKpkh0vKvSnXrRXhCtdL2XifqZnKBTtclwacfuckGRPFjC1Qv1pxGs6-aT_AGf  | 
    
* fix: on navigate with keyboard opened and go back add a padding on keyboard manager * fix: onBlur changing isFocused and breaking the composer * chore: code improvements * fix: snapshot test * fix: setFocused and useOnBlur * fix: snapshot test * fix: navigation issue * fix: edge case on navigate userInfo * chore: code improvements * chore: code improvements * chore: code improvements * fix: iOSBackSwipe reset * fix: iOSBackSwipe reset * fix: onback dismiss * fix: blur
Proposed changes
Navigating on iOS is causing a weird issue, such as adding extra padding to the list and breaking the Composer component layout.
This PR prevents premature onBlur events and properly dismisses the keyboard on iOS.
Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1028
How to test or reproduce
Case I - Swipe back to RoomView;
Case II - Swipe back to RoomsListView and navigate to RoomView;
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
Bug Fixes
Improvements