chore: enable edge to edge on android#7157
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughEnables Android edge-to-edge display support, removes ChangesEdge-to-Edge Android Configuration
Action Sheet Refactoring
Safe-Area Inset Consolidation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 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. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (14)
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/containers/ActionSheet/useActionSheetDetents.ts (1)
48-87:⚠️ Potential issue | 🟠 MajorAdd
fontScaleto the useMemo dependency array.
CANCEL_HEIGHTdepends onfontScalefromuseWindowDimensions(), but the memo doesn't includefontScalein its dependencies. When the user changes text size (accessibility setting),fontScaleupdates but the memo won't recompute, leaving detent calculations stale.Fix
- }, [contentHeight, hasCancel, headerHeight, itemHeight, optionsLength, snaps, windowHeight]); + }, [contentHeight, fontScale, hasCancel, headerHeight, itemHeight, optionsLength, snaps, windowHeight]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/ActionSheet/useActionSheetDetents.ts` around lines 48 - 87, The memo currently computes CANCEL_HEIGHT from fontScale (via useWindowDimensions()) but does not include fontScale in the useMemo dependency array, so detents won't update when fontScale changes; update the dependency array for the useMemo that returns { detents, maxHeight, scrollEnabled } to include fontScale (alongside contentHeight, hasCancel, headerHeight, itemHeight, optionsLength, snaps, windowHeight) so that CANCEL_HEIGHT and all derived calculations (maxSnap, measuredHeight, etc.) are recomputed when fontScale changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/containers/ActionSheet/useActionSheetDetents.ts`:
- Around line 48-87: The memo currently computes CANCEL_HEIGHT from fontScale
(via useWindowDimensions()) but does not include fontScale in the useMemo
dependency array, so detents won't update when fontScale changes; update the
dependency array for the useMemo that returns { detents, maxHeight,
scrollEnabled } to include fontScale (alongside contentHeight, hasCancel,
headerHeight, itemHeight, optionsLength, snaps, windowHeight) so that
CANCEL_HEIGHT and all derived calculations (maxSnap, measuredHeight, etc.) are
recomputed when fontScale changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 924d26f6-ed5c-4379-aa92-9aaff1894492
📒 Files selected for processing (8)
android/gradle.propertiesapp/containers/ActionSheet/ActionSheet.tsxapp/containers/ActionSheet/BottomSheetContent.tsxapp/containers/ActionSheet/useActionSheetDetents.test.tsxapp/containers/ActionSheet/useActionSheetDetents.tsapp/containers/MessageComposer/hooks/useEmojiKeyboard.tsxapp/views/DirectoryView/index.tsxapp/views/JitsiMeetView/index.tsx
📜 Review details
⏰ 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 context used
📓 Path-based instructions (5)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/views/JitsiMeetView/index.tsxapp/views/DirectoryView/index.tsxapp/containers/ActionSheet/BottomSheetContent.tsxapp/containers/MessageComposer/hooks/useEmojiKeyboard.tsxapp/containers/ActionSheet/useActionSheetDetents.tsapp/containers/ActionSheet/ActionSheet.tsxapp/containers/ActionSheet/useActionSheetDetents.test.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
**/*.{ts,tsx}: Use TypeScript with strict mode enabled and baseUrl set to app/ for module imports
Support iOS 13.4+ and Android 6.0+ as minimum target platforms
Files:
app/views/JitsiMeetView/index.tsxapp/views/DirectoryView/index.tsxapp/containers/ActionSheet/BottomSheetContent.tsxapp/containers/MessageComposer/hooks/useEmojiKeyboard.tsxapp/containers/ActionSheet/useActionSheetDetents.tsapp/containers/ActionSheet/ActionSheet.tsxapp/containers/ActionSheet/useActionSheetDetents.test.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use tabs for indentation with single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses when possible
Use ESLint with@rocket.chat/eslint-configbase including React, React Native, TypeScript, and Jest plugins
Files:
app/views/JitsiMeetView/index.tsxapp/views/DirectoryView/index.tsxapp/containers/ActionSheet/BottomSheetContent.tsxapp/containers/MessageComposer/hooks/useEmojiKeyboard.tsxapp/containers/ActionSheet/useActionSheetDetents.tsapp/containers/ActionSheet/ActionSheet.tsxapp/containers/ActionSheet/useActionSheetDetents.test.tsx
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Create view components (screens) in app/views/ directory
Files:
app/views/JitsiMeetView/index.tsxapp/views/DirectoryView/index.tsx
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Create reusable UI components in app/containers/ directory
Files:
app/containers/ActionSheet/BottomSheetContent.tsxapp/containers/MessageComposer/hooks/useEmojiKeyboard.tsxapp/containers/ActionSheet/useActionSheetDetents.tsapp/containers/ActionSheet/ActionSheet.tsxapp/containers/ActionSheet/useActionSheetDetents.test.tsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:06.688Z
Learning: The RocketChat/Rocket.Chat.ReactNative repository uses a fork of react-native-image-crop-picker (RocketChat/react-native-image-crop-picker) with custom Android edge-to-edge fixes, not the upstream ivpusic/react-native-image-crop-picker package. Dependencies should reference commit pins from the RocketChat fork.
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to {app/sagas/videoConf.ts,app/lib/methods/videoConf.ts} : Implement video conferencing in app/sagas/videoConf.ts and app/lib/methods/videoConf.ts using Redux actions, reducers, and sagas for server-managed Jitsi integration
Applied to files:
app/views/JitsiMeetView/index.tsx
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/AppContainer.tsx : Use AppContainer.tsx as the root navigation container that switches between authentication states
Applied to files:
app/views/JitsiMeetView/index.tsx
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/index.tsx : Set up Redux provider, theme, navigation, and notifications in app/index.tsx
Applied to files:
app/views/JitsiMeetView/index.tsxapp/containers/MessageComposer/hooks/useEmojiKeyboard.tsxapp/containers/ActionSheet/ActionSheet.tsx
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Use React 19 with React Native 0.79 and Expo 53
Applied to files:
app/views/JitsiMeetView/index.tsxapp/containers/MessageComposer/hooks/useEmojiKeyboard.tsx
📚 Learning: 2026-04-04T21:34:30.268Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6808
File: app/containers/MessageComposer/components/ComposerInput.tsx:337-341
Timestamp: 2026-04-04T21:34:30.268Z
Learning: In Rocket.Chat React Native, the markdown composer's autocomplete insertion (ComposerInput.tsx onAutocompleteItemSelected) does NOT need to add a space between an underscore italic delimiter `_` and a `@` or `#` mention sigil. The web platform (using the same rocket.chat/message-parser) does not add such a space either, so parity with web is the correct behavior. The previous learning about "space between `_` and mention sigil" applies only to test/story file content strings, not to the composer's runtime autocomplete behavior.
Applied to files:
app/containers/MessageComposer/hooks/useEmojiKeyboard.tsx
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to **/*.{ts,tsx} : Support iOS 13.4+ and Android 6.0+ as minimum target platforms
Applied to files:
app/containers/MessageComposer/hooks/useEmojiKeyboard.tsxapp/containers/ActionSheet/ActionSheet.tsx
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/theme.tsx : Define theming context in app/theme.tsx
Applied to files:
app/containers/MessageComposer/hooks/useEmojiKeyboard.tsx
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/hooks/useResponsiveLayout/**/*.{ts,tsx} : Implement responsive layouts using useResponsiveLayout hook to switch between master-detail on tablets and single stack on phones
Applied to files:
app/containers/ActionSheet/ActionSheet.tsx
🔇 Additional comments (7)
app/views/JitsiMeetView/index.tsx (1)
5-7: Import update looks good.No concerns in this changed segment; this is a clean import adjustment.
app/containers/ActionSheet/BottomSheetContent.tsx (1)
38-43: Looks good.Using the safe-area bottom inset as the only bottom padding source matches the edge-to-edge refactor and keeps spacing tied to the device inset.
app/containers/MessageComposer/hooks/useEmojiKeyboard.tsx (1)
79-98: Looks good.The new
bottom-based notch handling keeps keyboard spacing consistent across platforms and lines up with the safe-area driven refactor.app/containers/ActionSheet/useActionSheetDetents.test.tsx (1)
41-71: Looks good.The updated expectations now match the hook's no-
bottomInsetformula.app/containers/ActionSheet/ActionSheet.tsx (1)
4-9: Looks good.The new
disableContentPanningwiring and iOS-onlycontentMinHeightbehavior align with the updated detent and inset flow.Also applies to: 79-92
app/views/DirectoryView/index.tsx (1)
172-185: Looks good.Removing the explicit
enableContentPanningGestureoverride keeps this screen aligned with the new ActionSheet default behavior.android/gradle.properties (1)
48-51: Verify the host activity before enabling this flag.
edgeToEdgeEnabledonly takes effect withReactActivity; if the Android entry point uses a custom activity, this property will be ignored and the edge-to-edge change won't ship.
| }} | ||
| ItemSeparatorComponent={List.Separator} | ||
| contentContainerStyle={{ backgroundColor: colors.surfaceRoom }} | ||
| contentContainerStyle={{ backgroundColor: colors.surfaceRoom, paddingBottom: bottom }} |
There was a problem hiding this comment.
To prevent last item from hiding behind nav bar
| import { type RouteProp, useNavigation, useRoute } from '@react-navigation/native'; | ||
| import { activateKeepAwake, deactivateKeepAwake } from 'expo-keep-awake'; | ||
| import { useCallback, useEffect, useState, type ReactElement } from 'react'; | ||
| import { ActivityIndicator, Linking, SafeAreaView, StyleSheet, View } from 'react-native'; |
There was a problem hiding this comment.
SafeAreaView from react-native is deprecated
| const { submit } = useConnectServer({ workspaceUrl, certificate, previousServer }); | ||
| const phoneMarginTop = previousServer ? 32 : 84; | ||
| const marginTop = isTablet ? 0 : phoneMarginTop; | ||
| const formContainerStyle = previousServer ? { paddingBottom: 100 } : {}; |
There was a problem hiding this comment.
I have verified and there is no UI difference
| export default connect(mapStateToProps)(withTheme(ReadReceiptView)); | ||
| const ReadReceiptViewWithInsets = withSafeAreaInsets(ReadReceiptView); | ||
| hoistNonReactStatics(ReadReceiptViewWithInsets, ReadReceiptView); | ||
| export default connect(mapStateToProps)(withTheme(ReadReceiptViewWithInsets)); |
There was a problem hiding this comment.
Added hoistNonReactStatics because withSafeAreaInsets was removing the static navigation options, causing the header title to disappear.
There was a problem hiding this comment.
You're having to repeat yourself on other places like RoomActionsView. I'd patch withSafeAreaInsets in the lib instead. It should be simple to add hoistNonReactStatics there.
| export default connect(mapStateToProps)(withTheme(withActionSheet(withDimensions(withMasterDetail(RoomActionsView))))); | ||
| const RoomActionsViewWithInsets = withSafeAreaInsets(RoomActionsView); | ||
| hoistNonReactStatics(RoomActionsViewWithInsets, RoomActionsView); | ||
| export default connect(mapStateToProps)(withTheme(withActionSheet(withDimensions(withMasterDetail(RoomActionsViewWithInsets))))); |
There was a problem hiding this comment.
Added hoistNonReactStatics because withSafeAreaInsets was removing the static navigation options, causing the header title to disappear.
| export default StyleSheet.create({ | ||
| scrollContentContainer: { | ||
| paddingTop: 16 | ||
| }, |
There was a problem hiding this comment.
For some reason, the room action view lost the top padding, so added padding top to get the old UI
There was a problem hiding this comment.
"For some reason" always gets me 🤔 or 🙈
| export default connect(mapStateToProps)(withTheme(withMasterDetail(SearchMessagesView))); | ||
| const SearchMessagesViewWithInsets = withSafeAreaInsets(SearchMessagesView); | ||
| hoistNonReactStatics(SearchMessagesViewWithInsets, SearchMessagesView); | ||
| export default connect(mapStateToProps)(withTheme(withMasterDetail(SearchMessagesViewWithInsets))); |
There was a problem hiding this comment.
Added hoistNonReactStatics because withSafeAreaInsets was removing the static navigation options, causing the header title to disappear.
|
@coderabbitai review |
✅ Action performedReview finished.
|
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/views/MessagesView/index.tsx`:
- Line 392: The `withSafeAreaInsets` HOC is dropping static properties like
`navigationOptions` because the wrapped `MessagesView` class is hidden behind
`React.forwardRef`. Update the export chain around `MessagesView` so the final
connected component preserves statics by using `hoistNonReactStatics` after
composing `withSafeAreaInsets`, `withMasterDetail`, `withActionSheet`,
`withTheme`, and `connect(mapStateToProps)`. Make sure the hoisted component
keeps `MessagesView`’s static keys available to react-navigation.
🪄 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: b9d9cb28-6da5-42ae-96e4-fc0291275f1c
📒 Files selected for processing (20)
android/app/src/main/res/values/styles.xmlapp/lib/methods/helpers/theme.tsapp/views/AddExistingChannelView/index.tsxapp/views/CannedResponsesListView/index.tsxapp/views/DirectoryView/index.tsxapp/views/DiscussionsView/index.tsxapp/views/MessagesView/index.tsxapp/views/NewMessageView/index.tsxapp/views/ProfileView/index.tsxapp/views/RoomActionsView/index.tsxapp/views/RoomActionsView/styles.tsapp/views/RoomMembersView/index.tsxapp/views/RoomView/index.tsxapp/views/RoomsListView/index.tsxapp/views/SearchMessagesView/index.tsxapp/views/SelectListView.tsxapp/views/SettingsView/index.tsxapp/views/StatusView/index.tsxapp/views/TeamChannelsView.tsxapp/views/ThreadMessagesView/index.tsx
✅ Files skipped from review due to trivial changes (3)
- app/views/RoomActionsView/styles.ts
- app/views/DiscussionsView/index.tsx
- android/app/src/main/res/values/styles.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- app/views/SearchMessagesView/index.tsx
- app/views/RoomActionsView/index.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/views/AddExistingChannelView/index.tsxapp/views/StatusView/index.tsxapp/views/SettingsView/index.tsxapp/views/CannedResponsesListView/index.tsxapp/views/NewMessageView/index.tsxapp/lib/methods/helpers/theme.tsapp/views/RoomView/index.tsxapp/views/ThreadMessagesView/index.tsxapp/views/RoomsListView/index.tsxapp/views/MessagesView/index.tsxapp/views/SelectListView.tsxapp/views/ProfileView/index.tsxapp/views/TeamChannelsView.tsxapp/views/DirectoryView/index.tsxapp/views/RoomMembersView/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/views/AddExistingChannelView/index.tsxapp/views/StatusView/index.tsxapp/views/SettingsView/index.tsxapp/views/CannedResponsesListView/index.tsxapp/views/NewMessageView/index.tsxapp/lib/methods/helpers/theme.tsapp/views/RoomView/index.tsxapp/views/ThreadMessagesView/index.tsxapp/views/RoomsListView/index.tsxapp/views/MessagesView/index.tsxapp/views/SelectListView.tsxapp/views/ProfileView/index.tsxapp/views/TeamChannelsView.tsxapp/views/DirectoryView/index.tsxapp/views/RoomMembersView/index.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/views/AddExistingChannelView/index.tsxapp/views/StatusView/index.tsxapp/views/SettingsView/index.tsxapp/views/CannedResponsesListView/index.tsxapp/views/NewMessageView/index.tsxapp/lib/methods/helpers/theme.tsapp/views/RoomView/index.tsxapp/views/ThreadMessagesView/index.tsxapp/views/RoomsListView/index.tsxapp/views/MessagesView/index.tsxapp/views/SelectListView.tsxapp/views/ProfileView/index.tsxapp/views/TeamChannelsView.tsxapp/views/DirectoryView/index.tsxapp/views/RoomMembersView/index.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/views/AddExistingChannelView/index.tsxapp/views/StatusView/index.tsxapp/views/SettingsView/index.tsxapp/views/CannedResponsesListView/index.tsxapp/views/NewMessageView/index.tsxapp/lib/methods/helpers/theme.tsapp/views/RoomView/index.tsxapp/views/ThreadMessagesView/index.tsxapp/views/RoomsListView/index.tsxapp/views/MessagesView/index.tsxapp/views/SelectListView.tsxapp/views/ProfileView/index.tsxapp/views/TeamChannelsView.tsxapp/views/DirectoryView/index.tsxapp/views/RoomMembersView/index.tsx
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in 'app/views/' directory
Files:
app/views/AddExistingChannelView/index.tsxapp/views/StatusView/index.tsxapp/views/SettingsView/index.tsxapp/views/CannedResponsesListView/index.tsxapp/views/NewMessageView/index.tsxapp/views/RoomView/index.tsxapp/views/ThreadMessagesView/index.tsxapp/views/RoomsListView/index.tsxapp/views/MessagesView/index.tsxapp/views/SelectListView.tsxapp/views/ProfileView/index.tsxapp/views/TeamChannelsView.tsxapp/views/DirectoryView/index.tsxapp/views/RoomMembersView/index.tsx
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/views/AddExistingChannelView/index.tsxapp/views/StatusView/index.tsxapp/views/SettingsView/index.tsxapp/views/CannedResponsesListView/index.tsxapp/views/NewMessageView/index.tsxapp/lib/methods/helpers/theme.tsapp/views/RoomView/index.tsxapp/views/ThreadMessagesView/index.tsxapp/views/RoomsListView/index.tsxapp/views/MessagesView/index.tsxapp/views/SelectListView.tsxapp/views/ProfileView/index.tsxapp/views/TeamChannelsView.tsxapp/views/DirectoryView/index.tsxapp/views/RoomMembersView/index.tsx
🔇 Additional comments (16)
app/views/DirectoryView/index.tsx (1)
28-28: LGTM!Also applies to: 40-40, 43-50, 56-72, 203-203
app/views/AddExistingChannelView/index.tsx (1)
24-24: LGTM!Also applies to: 42-42, 44-54, 180-180
app/views/CannedResponsesListView/index.tsx (1)
28-28: LGTM!Also applies to: 70-71, 276-276
app/views/NewMessageView/index.tsx (1)
21-21: LGTM!Also applies to: 37-37, 39-47, 110-110
app/views/ProfileView/index.tsx (1)
37-37: LGTM!Also applies to: 62-62, 85-85, 285-285, 297-300
app/views/ThreadMessagesView/index.tsx (1)
47-47: LGTM!Also applies to: 70-70, 496-496, 515-515, 538-541
app/views/RoomView/index.tsx (1)
44-45: LGTM!Also applies to: 1529-1531, 1537-1537, 1552-1552, 1567-1587, 1597-1597, 1759-1761
app/views/RoomMembersView/index.tsx (1)
83-92: LGTM!Also applies to: 439-439
app/views/RoomsListView/index.tsx (1)
45-48: LGTM!Also applies to: 138-138
app/views/StatusView/index.tsx (1)
112-112: LGTM!Also applies to: 142-142, 249-249
app/views/MessagesView/index.tsx (1)
59-59: LGTM!Also applies to: 363-363, 377-377
app/views/SelectListView.tsx (1)
42-42: LGTM!Also applies to: 195-195, 204-204, 212-212
app/views/TeamChannelsView.tsx (1)
98-98: LGTM!Also applies to: 548-576, 604-606
app/views/SettingsView/index.tsx (1)
141-141: 🩺 Stability & Availability
List.ContainersupportscontentContainerStyle.The
List.Containerimplementation maps directly to aScrollViewthat spreads all received props ({...props}) onto the native component. This ensures that passingcontentContainerStyle={{ paddingBottom: bottom }}correctly applies the padding to the underlying scrollable content without being overwritten.app/lib/methods/helpers/theme.ts (2)
3-3: LGTM!Also applies to: 53-54
56-59: 🩺 Stability & AvailabilityRemove suggestion to use
.catch()for synchronous method.
NavigationBar.setBarStylefrom@zoontek/react-native-navigation-baris a synchronous function that returnsvoid, not a Promise. Therefore, the existingtry/catchblock is correct for handling potential synchronous errors, and no asynchronous error handling (like.catch()) is required.> Likely an incorrect or invalid review comment.
|
Android Build Available Rocket.Chat 4.74.0.109206 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNQXaiekQW0E3WLU3EYTNN9QH32NUiUMAsQvmUo6BkT8j2tDzttbuln2moucc9l5CL8FEl9_kcg95DWBiu4F |
|
iOS Build Available Rocket.Chat 4.74.0.109207 |
diegolmello
left a comment
There was a problem hiding this comment.
Almost there!
Nice job with the screenshots.
Just to be sure: we should test on landscape and on tablets.
| - name: Checkout Repository | ||
| uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 | ||
| with: | ||
| persist-credentials: false |
There was a problem hiding this comment.
looks unrelated to the pr. what does it do?
| export default connect(mapStateToProps)(withTheme(ReadReceiptView)); | ||
| const ReadReceiptViewWithInsets = withSafeAreaInsets(ReadReceiptView); | ||
| hoistNonReactStatics(ReadReceiptViewWithInsets, ReadReceiptView); | ||
| export default connect(mapStateToProps)(withTheme(ReadReceiptViewWithInsets)); |
There was a problem hiding this comment.
You're having to repeat yourself on other places like RoomActionsView. I'd patch withSafeAreaInsets in the lib instead. It should be simple to add hoistNonReactStatics there.
| export default StyleSheet.create({ | ||
| scrollContentContainer: { | ||
| paddingTop: 16 | ||
| }, |
There was a problem hiding this comment.
"For some reason" always gets me 🤔 or 🙈
diegolmello
left a comment
There was a problem hiding this comment.
Code review — Edge-to-edge Android (#7157)
Consolidated from two independent review passes. Inline comments below.
Correctness (worth fixing before merge):
- JitsiMeetView —
SafeAreaViewfromreact-native-safe-area-contextwith noedgesinsets the WebView on all four sides on Android. - SettingsView (+ a
ListContainermerge fix) —contentContainerStyleoverride drops the defaultpaddingVertical: 16. - FormContainer —
{ paddingBottom: bottom }overrides the basepaddingBottom: 24across all form screens. - useEmojiKeyboard — Android loses its
Platformgating → composer spacer + double-counted notch. - useActionSheetDetents — dropped
HANDLE_HEIGHTcan clip the last row of custom-content sheets. - EmojiCategory — inset double-counted against the parent's
marginBottom. - E2EEncryptionSecurityView — padding on the wrong element (outer frame, not scroll content).
- BottomSheetContent — Android lost the Cancel-row bottom padding.
Confirm intent / device-check: ActionSheet content-panning, NavBottomFAB offset, NewServerView cert-picker clearance.
Inset-dependent — only bites where insets.bottom is 0 (hardware buttons, landscape, tablet master-detail): RoomActionsView, and the LanguageView / AutoTranslate / DefaultBrowser / SelectServer / SelectList FlatLists.
Minor: RoomActionsView insets should use the EdgeInsets type.
Note: CI sets edgeToEdgeEnabled=true both in android/gradle.properties and by echoing it in the two build workflows — two sources of truth, so a future toggle in gradle.properties won't take effect in CI.
Verified-correct, no action: the added hoistNonReactStatics calls, the zoontek setBarStyle API, the RoomView footer change (paddingBottom is additive to the existing marginBottom), and FormContainer's flexGrow change.
|
|
||
| return ( | ||
| <View style={styles.container}> | ||
| <View style={[styles.container, { bottom: EDGE_DISTANCE + bottom }]}> |
There was a problem hiding this comment.
bottom: EDGE_DISTANCE + bottom — this FAB is absolutely positioned inside the message List view, whose bottom edge is the top of the composer (RoomView's SafeAreaView uses edges={['right','left']}, so the list doesn't extend into the nav-bar area; the composer is a sibling below it). Adding the nav-bar inset bottom here looks like it offsets the FAB ~bottompx too high, leaving a gap above the composer on nav-bar devices. Please confirm the FAB still sits just above the composer.
|
Android Build Available Rocket.Chat 4.74.0.109215 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNTSjwJZ7RTEvqpD3KCxuHI9GdRcDNufZmp4NUW-3jUZNYUDZTHBfD5d0ZsgOwftWkgOlFbj--nwGfXNit1z |
Proposed changes
Replace
expo-navigation-barwith@zoontek/react-native-navigation-barand add safe area bottom insets across 30+ views to enable proper edge-to-edge display on Android. This prevents content from being hidden behind the system navigation bar on devices with gesture navigation.Key changes:
expo-navigation-barwith@zoontek/react-native-navigation-barfor better edge-to-edge supportpaddingBottom: bottom(viauseSafeAreaInsets/withSafeAreaInsets) to FlatLists, ScrollViews, and content containers across the apppaddingBottom: 100inNewServerViewin favor ofFormContainers built-in safe area handlingJitsiMeetViewfrom RNs built-inSafeAreaViewtoreact-native-safe-area-contextsRoomViewfooter sections to respect bottom safe areaIssue(s)
https://rocketchat.atlassian.net/browse/CORE-1967
How to test or reproduce
Screenshots
Summary by CodeRabbit