Feature/enter key behavior tablet #6843
Conversation
WalkthroughThe changes implement an Enter key behavior preference feature allowing users to select between sending messages or creating new lines when pressing Enter. This includes preference storage via a new constant key, tablet-specific UI settings, complex Enter key interception in the message composer, and localization strings. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
ios/RocketChatRN.xcodeproj/project.pbxproj (1)
1737-1738: Prefer keeping these paths quoted for consistency with the rest of the project fileThe unquoted
$TARGET_BUILD_DIR/$INFOPLIST_PATHand$PODS_CONFIGURATION_BUILD_DIR/Firebaseentries will likely still work, but almost all otherinputPaths/HEADER_SEARCH_PATHShere are quoted. Re-quoting them would avoid surprises with Xcode parsing and keep the file style uniform.Also applies to: 1823-1824, 2605-2606, 2683-2684
app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx (1)
10-11: Generic ListPicker works for current uses but typing could be tightenedThe genericization and dual option sets (
alertvsenterKey) look correct for the two current call sites, and the runtime behavior is sound. Two follow-ups you might consider:
- Tie
typeandTmore strongly in the types so it’s impossible to accidentally callListPickerwithvalue: TEnterKeyBehaviorbuttypeomitted (which would silently pickALERT_OPTIONS). For example, a discriminated union like:and then drop the generic.type ListPickerProps = | { type?: 'alert'; value: TAlertDisplayType; onChangeValue: (v: TAlertDisplayType) => void } | { type: 'enterKey'; value: TEnterKeyBehavior; onChangeValue: (v: TEnterKeyBehavior) => void };- Hoist
ALERT_OPTIONSandENTER_KEY_OPTIONSoutside the component so they aren’t recreated on every render (tiny perf/readability win).Also applies to: 38-39, 41-44, 46-55, 58-59, 71-82, 84-87, 97-100
app/views/AccessibilityAndAppearanceView/index.tsx (1)
17-21: Tablet enter-key preference wiring looks correctImporting
ENTER_KEY_BEHAVIOR_PREFERENCES_KEY, definingTEnterKeyBehavior, and bindingenterKeyBehaviorto a tablet-onlyListPickermatches the intended feature: tablets can choose between send vs newline while phones remain unchanged. Updating theuseLayoutEffectdeps to includeisMasterDetailis also a good correctness fix.You may later want to centralize
TEnterKeyBehaviorin a shared types module so ComposerInput can reuse it instead of repeating'SEND' | 'NEW_LINE', but that’s purely a maintainability improvement.Also applies to: 23-25, 32-39, 60-68, 126-139
📜 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 ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
app/containers/MessageComposer/components/ComposerInput.tsx(7 hunks)app/i18n/locales/en.json(1 hunks)app/lib/constants/keys.ts(1 hunks)app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx(4 hunks)app/views/AccessibilityAndAppearanceView/index.tsx(4 hunks)ios/RocketChatRN.xcodeproj/project.pbxproj(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/views/AccessibilityAndAppearanceView/index.tsx (2)
app/lib/methods/userPreferences.ts (1)
useUserPreferences(10-10)app/lib/constants/keys.ts (1)
ENTER_KEY_BEHAVIOR_PREFERENCES_KEY(23-23)
app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx (1)
app/views/AccessibilityAndAppearanceView/index.tsx (2)
TAlertDisplayType(23-23)TEnterKeyBehavior(24-24)
🔇 Additional comments (3)
app/i18n/locales/en.json (1)
796-798: Enter-key i18n strings are consistent and clearKeys and copy match existing naming/style and cover the behavior, send, and newline labels needed by the new picker.
app/lib/constants/keys.ts (1)
18-25: New ENTER_KEY_BEHAVIOR preference key is consistentThe new
ENTER_KEY_BEHAVIOR_PREFERENCES_KEYfollows the existing naming and namespacing pattern for preferences keys and is suitable for reuse across views and the composer.app/containers/MessageComposer/components/ComposerInput.tsx (1)
1-1: Enter-to-send implementation on tablets is well integrated; consider minor simplifications and manual QAThe new behavior in
ComposerInput—readingENTER_KEY_BEHAVIOR_PREFERENCES_KEY, intercepting a trailing\non tablets when behavior isSEND, trimming it, and delegating tosendMessage()—aligns with PR goals and reuses the existing send pipeline viaMessageInnerContext, which is good.A few minor points to consider:
- The combination of
shouldInterceptEnterRef,previousTextRef, and thetext.endsWith('\n') && text.slice(0, -1) === previousTextcheck provides cross-platform Enter detection (onKeyPress for Android, diff-based for iOS). Before merging, explicitly verify:
- iOS/iPadOS with hardware keyboard (ensure onKeyPress + diff behave consistently).
- Editing in the middle of a message vs at the end (current logic only intercepts appended newlines, which is correct but worth confirming in QA).
isUpdatingNativePropsRefwith the 0mssetTimeoutguard prevents recursiveonChangeTextcalls but adds complexity. Monitor for flakiness on rapid typing; a more targeted approach (tracking "last programmatic text" and ignoring only that exact next call) may be preferable if issues arise.- The union type
'SEND' | 'NEW_LINE'is hard-coded here whileTEnterKeyBehavioris exported fromAccessibilityAndAppearanceView. Consolidate by moving the type to a shared module and importing it in both places to prevent future drift.Recommended manual tests:
- iPad/tablet, setting = "Send message": Type "hello" and press Enter → message sends, input clears. Press Enter on empty input → nothing sent. Move cursor mid-text and press Enter → newline inserted (no send).
- iPad/tablet, setting = "New line": Press Enter anywhere → newline inserted, never sends.
- Phone (non-tablet), any keyboard: Enter always inserts newline; sending requires tapping send button.
- External hardware keyboard on iPad: Repeat tests (1)–(2) to ensure behavior matches on-screen keyboard.
1 participant
This update adds configurable Enter-key behavior for tablet and iPad users. They can now choose whether pressing Enter should send the message or insert a newline, directly from the Preferences tab. The feature is tablet and iPad -only and mirrors the behaviour of the send button whenever “Enter to Send” is enabled.
Closes #746
Key Changes
sendViaEnterKeyinComposerInputto update state and run the sharedsendMessagepipeline.Demo Videos
For Ipad
For Tablet
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.