feat: quick actions #6902
feat: quick actions #6902
Conversation
…eactNative into feat/quickactions_native
|
Caution Review failedAn error occurred during the review process. Please try again later. WalkthroughAdds cross-platform quick actions: native intent/shortcut handling (iOS/Android), Redux state and sagas to process quick actions, tracking of recent room visits, UI integration to register/update shortcuts, and persistence of recent/last-visited rooms. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Native as Native Layer (Android/iOS)
participant AppInit as App Init (index.tsx / AppContainer)
participant Redux as Redux Store
participant Saga as QuickActions Saga
participant Nav as Navigation
User->>Native: Trigger quick action shortcut
Native->>Native: Set pending quick action / normalize intent
Native->>AppInit: App launches or receives intent
AppInit->>Redux: Dispatch quickActionHandle({action})
Redux->>Saga: QUICK_ACTION_HANDLE
alt action == recent
Saga->>Redux: select current server/room
Saga->>AppInit: wait for NAVIGATION_READY
Saga->>Nav: navigate to room view
else action == contact
Saga->>Native: open mailto / send email
else action == add-server or search
Saga->>Nav: navigate to add-server/search
end
Saga->>Redux: dispatch QUICK_ACTION_HANDLED
Nav->>Saga: emit roomVisited {rid,name}
Saga->>Redux: dispatch roomsStoreLastVisited
Saga->>Native: persist lastVisited & recentRooms
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 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 |
…eactNative into feat/quickactions_native
… instead of store.dispatch
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/views/RoomsListView/hooks/useSearch.ts (1)
62-83: "use memo" directive is valid; however, wrap the debounced callback inuseCallbackto prevent unnecessary effect re-runs.The
'use memo'directive is correct React Compiler syntax and correctly placed at line 62. However, the debouncedsearchcallback (lines 77–83) is recreated every render because it's not memoized. Sincesearchis listed as a dependency in the effect at line 104, the effect runs every render, even whenstate.searchEnabledhasn't changed. Wrap the callback inuseCallbackwith[state.searchEnabled]as dependencies:Suggested fix
const search = useDebounce( useCallback(async (text: string) => { if (!state.searchEnabled) return; dispatch({ type: 'SET_SEARCHING' }); const result = await searchLib({ text }); dispatch({ type: 'SEARCH_SUCCESS', payload: result as IRoomItem[] }); announceSearchResultsForAccessibility(result.length); }, [state.searchEnabled, announceSearchResultsForAccessibility]), 500 );app/AppContainer.tsx (1)
40-57: Guard against empty-string room name producing a blank quick-action title.
IflastVisitedRoomNamecan ever be'',recentRoomName ?? ...will keep it and show an empty title.Proposed tweak
useEffect(() => { - updateQuickActions({ recentRoomName: lastVisitedRoomName }); + updateQuickActions({ recentRoomName: lastVisitedRoomName || undefined }); }, [lastVisitedRoomName]);ios/RocketChatRN.xcodeproj/project.pbxproj (1)
1284-1298: Remove miswired build phases from RocketChatRN target.The RocketChatRN target contains two
[Expo] Configure projectphases (IDs:589729E8381BA997CD19EF19and9C104B12BEE385F7555E641F). The second one (line 1295) is miswired—it executes the NotificationService-specific expo-configure script and should not be in this target. Additionally, the unnamed ShellScript at line 1289 (ID:407D3EDE3DABEE15D27BD87D) outputs to a NotificationService-specific manifest file and appears misplaced. Move these phases to their appropriate targets.
🤖 Fix all issues with AI agents
In @android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt:
- Around line 34-49: onCreate currently ends up invoking
NotificationIntentHandler.handleIntent twice on cold start because
handleVideoConfIntent already delegates to
NotificationIntentHandler.handleIntent and onCreate also calls
NotificationIntentHandler.handleIntent directly; remove the redundant direct
call in onCreate (the intent?.let { NotificationIntentHandler.handleIntent(this,
it) } line) so notification intents are only processed once, ensure
handleVideoConfIntent still calls NotificationIntentHandler.handleIntent for
video-conf flows, and add equivalent logic to onNewIntent to call
NotificationIntentHandler.handleIntent(this, intent) for warm starts so all
intent handling flows go through a single place.
- Around line 58-97: Remove or gate the sensitive debug logs in
MainActivity.handleVideoConfIntent so identifiers (rid, callerName, callId,
host) are not printed in production: wrap the Log.d(...) calls that output these
fields in a debug-only check (e.g., if (BuildConfig.DEBUG) { ... }) or remove
them entirely, and apply the same change for any other Log.d/Log.* lines in the
same method range (the other occurrence referenced in lines ~104-170) so
VideoConfModule.storePendingAction still receives full data but no sensitive
identifiers are emitted to production logs.
In @android/app/src/main/res/drawable/ic_quickaction_add.xml:
- Around line 1-5: Remove the redundant android:tint attribute from the root
<vector> element in ic_quickaction_add.xml so the icon matches the other
quick-action drawables; keep the existing path fillColor="@android:color/black"
unchanged and ensure only the android:tint attribute is deleted from the
<vector> tag.
In @app/index.tsx:
- Around line 64-77: In parseDeepLinking, when handling quick-action URLs (the
url.startsWith('rocketchat://quick-action/') branch), avoid using replace(...)
directly; instead extract the raw action by removing the
'rocketchat://quick-action/' prefix, strip any querystring or fragment by
splitting on /[?#]/ and taking the first segment, then URI-decode it with
decodeURIComponent (wrap in try/catch to fall back to the raw value on decode
errors) before returning { type: 'quick-action', action } so action contains the
clean, decoded value.
In @app/lib/methods/helpers/goRoom.ts:
- Around line 141-147: The new getRoom(item.rid) call in goRoom can throw and
will abort navigation; wrap the getRoom and emitter.emit('roomVisited', ...)
sequence in a try-catch so failures are swallowed (or logged) and do not reject
goRoom. Specifically, in goRoom around the getRoom/item.rid section, catch
errors from getRoom, optionally log via the existing logger/emitter, and only
emit roomVisited when getRoom succeeds so navigation continues even if room
lookup fails.
In @app/lib/quickActions/index.ts:
- Around line 1-3: The fix: make updateQuickActions async and await
QuickActions.setItems(...) so the returned Promise<void> is handled (change
function signature of updateQuickActions and await the setItems call inside it);
for addListener(), capture the returned Subscription object (const sub =
QuickActions.addListener(...)) and ensure you call sub.remove() during cleanup
(e.g., when replacing/tearing down listeners or component unmount) to avoid
leaks and duplicate registrations. Ensure references use the exact symbols
updateQuickActions, QuickActions.setItems, QuickActions.addListener, and
Subscription.remove.
In @app/sagas/init.js:
- Line 5: Rename the constant LAST_VISITED_ROOM_Name_KEY to
LAST_VISITED_ROOM_NAME_KEY in the constants definition, update its export, and
change all imports/usages that reference LAST_VISITED_ROOM_Name_KEY (e.g., in
the sagas that reference the last visited room key) to use
LAST_VISITED_ROOM_NAME_KEY instead; ensure import statements and any
destructuring that referenced the old name are updated so the symbol name is
consistent and all references compile.
In @app/sagas/quickActions.ts:
- Around line 82-104: In the 'recent' case of the quickActions saga, don't exit
the saga early with "if (!rid) return;" because that bypasses the final yield
put({ type: QUICK_ACTIONS.QUICK_ACTION_HANDLED });; instead change the
early-return to a switch-case-safe exit (e.g., "if (!rid) { break; }") or wrap
the rest of the case logic in "if (rid) { ... }" so the case completes normally
and the QUICK_ACTION_HANDLED dispatch always runs; update the code around the
rid check in the 'recent' case that uses waitForAppReady/select (variable rid)
to ensure control flows to the end of the saga.
- Around line 61-78: The switch case for 'search' in quickActions uses a lexical
declaration (const currentRoute) without block scoping; wrap the entire 'search'
case body in braces to scope declarations properly (i.e., change case 'search':
... to case 'search': { ... } ), keeping the existing calls to
waitForAppReady(), Navigation.getCurrentRoute(),
Navigation.navigate('RoomsListView') and the yield put({ type: UI.TRIGGER_SEARCH
}) intact so the behavior of quickAction handling remains unchanged.
- Around line 36-44: waitForRoomInDB can hang because take(APP.START) waits
indefinitely for the next APP.START; replace the current try/catch with a
bounded retry loop that calls getRoom up to a fixed maxAttempts and uses a short
delay/race to avoid deadlock: inside waitForRoomInDB attempt yield call(getRoom,
rid) and return on success; on failure yield a race between take(APP.START) and
call(delay, retryDelayMs) (or just call(delay, ms)) then loop until maxAttempts
exhausted; after maxAttempts either throw/return null to fail fast. Keep
references to waitForRoomInDB, getRoom and APP.START so the retry logic replaces
the current try/catch/return flow.
In @app/sagas/roomVisited.ts:
- Line 8: The constant name uses mixed case; rename LAST_VISITED_ROOM_Name_KEY
to LAST_VISITED_ROOM_NAME_KEY in the constant definition
(app/lib/constants/keys.ts) and update all imports/usages (e.g., the import in
app/sagas/roomVisited.ts and any other files) to the new identifier; ensure you
update export(s) and run type-check/grep to catch and fix all references so
builds pass.
In @app/views/SettingsView/index.tsx:
- Around line 36-50: Replace encodeURI with encodeURIComponent for the mailto
query parameters and use the helper values directly: change subject =
encodeURIComponent('Rocket.Chat Mobile App Support') and description =
encodeURIComponent(`version: ${getReadableVersion}\ndevice: ${getDeviceModel}`)
(do not call getReadableVersion/getDeviceModel as functions); keep the
Linking.openURL call using the template string
`mailto:${email}?subject=${subject}&body=${description}` and preserve the
try/catch and logging in sendEmail.
In @ios/QuickActionsConnector.mm:
- Around line 8-16: The getInitialQuickAction method reads and clears the global
RCPendingQuickActionType without synchronization, causing a race; fix by
serializing access to that variable (e.g., create a dedicated serial
dispatch_queue or guard with @synchronized on a shared object) and perform the
read-and-clear on that queue or within the lock so getInitialQuickAction
atomically captures the value and sets RCPendingQuickActionType = nil; update
any other writers/readers of RCPendingQuickActionType to use the same
synchronization primitive.
In @ios/QuickActionsStore.h:
- Around line 1-5: The header QuickActionsStore.h is missing include guards; add
a guard to prevent multiple inclusion (use either #pragma once at the top or
traditional #ifndef/#define/#endif) so the declarations for
RCPendingQuickActionType and RCSetPendingQuickActionType are only processed once
when included via RocketChatRN-Bridging-Header.h; place the guard around the
existing FOUNDATION_EXPORT declarations to enclose the whole header content.
In @ios/RocketChatRN.xcodeproj/project.pbxproj:
- Around line 1567-1583: Remove the entire PBXShellScriptBuildPhase entry with
identifier 1E1EA8082326CCE300E22452 from the project file and delete any
references to that ID from target buildPhases arrays so the shell script is
fully removed; ensure you also remove its block from PBXBuildPhase lists (and
any duplicate occurrences) to avoid dangling references that would break the
project.
In @package.json:
- Line 73: The dependency version for expo-quick-actions in package.json is
incompatible with Expo SDK 53; change the version string for
"expo-quick-actions" from "^6.0.0" to "5.0.0" (or another 5.x release) so it
targets SDK 53, then run your package manager install to update lockfile and
ensure tests/build succeed.
🧹 Nitpick comments (16)
android/app/src/main/res/drawable/ic_quickaction_contact.xml (1)
1-5: Consider standardizing viewport dimensions.The vector drawable is correctly formatted. However, note that
ic_quickaction_contact.xmluses a 24x24 viewport whileic_quickaction_recent.xmluses 512x512. While both will render correctly, standardizing viewports across related icon resources can improve maintainability.♻️ Optional: standardize to 512x512 viewport
If you want consistency with the recent icon, you could rescale the path data to a 512x512 viewport. However, this is purely cosmetic and not required for functionality.
app/actions/actionsTypes.ts (1)
15-15: New action namespaces look consistent; consider tighteningcreateRequestTypestyping.Lines 15 and 63-65 align with the new quick-actions/UI flows. Small TS maintainability improvement:
createRequestTypes(base = {}, ...)should likely takebase: string(default'') to avoid accidental${base}becoming[object Object].Also applies to: 63-65
app/actions/quickActions.ts (1)
5-15: Makeparamsoptional (and avoidparams.actionambiguity).
Right nowIQuickAction.paramsis required (Line 9) andquickActionHandlerequires an argument (Line 11), which makes “no-param” dispatch awkward, andparams.actionis easy to confuse with Redux “action”.Proposed diff
interface IQuickActionParams { - action: string; + action: string; } interface IQuickAction extends Action { - params: Partial<IQuickActionParams>; + params?: Partial<IQuickActionParams>; } -export function quickActionHandle(params: Partial<IQuickActionParams>): IQuickAction { +export function quickActionHandle(params: Partial<IQuickActionParams> = {}): IQuickAction { return { type: QUICK_ACTIONS.QUICK_ACTION_HANDLE, params }; }app/views/RoomsListView/hooks/useSearch.ts (1)
65-104: Type your Redux dispatch and avoid clearing triggers with untyped objects.
SinceAppDispatchnow exists, preferuseDispatch<AppDispatch>()so these dispatches are type-checked (Line 66, Line 96).Proposed diff
-import { useDispatch } from 'react-redux'; +import { useDispatch } from 'react-redux'; +import type { AppDispatch } from '../../../lib/store'; ... - const storeDispatch = useDispatch(); + const storeDispatch = useDispatch<AppDispatch>();app/lib/store/index.ts (1)
29-33: Good addition; watch out forstorename collisions withauxStore.ts.
AppDispatchis a solid typing hook foruseDispatch, but it’s easy to accidentally import the wrongstoregivenapp/lib/store/auxStore.tsalso exportsstore.app/reducers/rooms.ts (1)
9-10: Inconsistent naming: mix of abbreviation and full word.The state fields use inconsistent naming styles:
lastVisitedRid— abbreviation ("Rid" for room ID)lastVisitedName— full word ("Name")The action payload uses full words (
lastVisitedRoomId,lastVisitedRoomName), which is clearer. Consider renaming the state fields to match for consistency:- lastVisitedRid: string; - lastVisitedName: string; + lastVisitedRoomId: string; + lastVisitedRoomName: string;You'll also need to update the initial state (lines 18-19) and the reducer case (lines 54-55) accordingly.
app/AppContainer.tsx (1)
4-25: Consider avoidingconnect(...)+ hooks in the same component.
Not a blocker, but it makes the data-flow harder to follow (props from HOC + hooks from store). Either approach alone would be simpler.app/i18n/locales/en.json (1)
331-331: Same i18n key hygiene note (Something_Wrong?) + optional copy polish.
If you keep the\ninError_finding_room, consider sentence punctuation for UI consistency (non-blocking).Also applies to: 457-457, 698-699, 846-846, 984-984
app/index.tsx (1)
166-184: Replace non-null assertion and add quick-action ID validation.The
deepLinking!non-null assertion is unsafe sinceLinking.getInitialURL()can returnnullorundefined. Additionally, validation here prevents accidental dispatches of unknown quick-action IDs. The handler gracefully ignores them today, but explicit validation at the entry point is clearer and prevents unnecessary Redux actions.Replace the non-null assertion:
const parsedDeepLinkingURL = deepLinking ? parseDeepLinking(deepLinking) : null;And add ID validation before dispatch in the quick-action handler to match the known cases defined in
app/lib/quickActions/index.ts('search', 'add-server', 'recent', 'contact').app/lib/quickActions/getRecentQuickAction.ts (1)
14-19: Consider simplifying the error handling return.The try-catch appropriately returns
nullon any error. The?? nullon line 16 is defensive but potentially redundant, sincegetInitialQuickActionshould return a string or null/undefined. If the native contract guarantees this, you can simplify toreturn action || null.♻️ Optional simplification
try { const action = await QuickActionsConnector.getInitialQuickAction(); - return action ?? null; + return action || null; } catch { return null; }app/reducers/ui.ts (1)
13-28: Strengthen type safety with a discriminated union for UI actions.The reducer uses a generic
Actiontype, preventing TypeScript from verifying thatUI.TRIGGER_SEARCHandUI.CLEAR_TRIGGERED_SEARCHare valid action types. Define a union of specific UI action types for better compile-time safety.♻️ Proposed type-safe action union
+type UIAction = + | { type: typeof UI.TRIGGER_SEARCH } + | { type: typeof UI.CLEAR_TRIGGERED_SEARCH }; + -export default function ui(state = initialState, action: Action): IUIState { +export default function ui(state = initialState, action: UIAction | Action): IUIState { switch (action.type) { case UI.TRIGGER_SEARCH: return { ...state, triggerSearch: true }; case UI.CLEAR_TRIGGERED_SEARCH: return { ...state, triggerSearch: false }; default: return state; } }This approach allows UI actions to be type-checked while still accepting generic actions for Redux compatibility.
app/sagas/roomVisited.ts (1)
21-31: Add error handling for UserPreferences persistence.The
UserPreferences.setStringcalls (lines 28-29) lack error handling. If persistence fails, the saga will crash and stop processing future room visitation events. Consider wrapping these calls in a try-catch to ensure the saga remains resilient.♻️ Proposed error handling
function* watchRoomVisited(): SagaIterator { const channel = yield call(createRoomVisitedChannel); while (true) { const { rid, name } = yield take(channel); yield put(roomsStoreLastVisited(rid, name)); - UserPreferences.setString(LAST_VISITED_ROOM_ID_KEY, rid); - UserPreferences.setString(LAST_VISITED_ROOM_Name_KEY, name); + try { + UserPreferences.setString(LAST_VISITED_ROOM_ID_KEY, rid); + UserPreferences.setString(LAST_VISITED_ROOM_Name_KEY, name); + } catch (error) { + // Log but don't crash the saga + console.warn('Failed to persist last visited room:', error); + } } }app/lib/methods/helpers/goRoom.ts (2)
115-116: Silent catch block may hide unexpected errors.The empty catch block with "do nothing" comment intentionally swallows all errors from the room visitation tracking. While this prevents blocking navigation, it also hides potential bugs (e.g., emitter failures, getRoom issues). Consider logging the error for debugging.
📋 Optional logging addition
} catch { - // do nothing + // Ignore errors in room visit tracking to avoid blocking navigation }Or add minimal logging:
- } catch { - // do nothing + } catch (error) { + // Log but don't block navigation + console.debug('Failed to track room visit:', error); }
141-149: Sequential database queries may impact performance.Lines 141 and 149 make two separate database calls (
getRoomandgetSubscriptionByRoomId) for the sameitem.rid. If these queries are frequently executed, consider optimizing by fetching both in parallel or combining into a single query.Example parallel fetch:
const [room, sub] = await Promise.all([ getRoom(item.rid).catch(() => null), getSubscriptionByRoomId(item.rid) ]); if (room) { emitter.emit('roomVisited', { rid: room.rid, name: room.name }); } if (sub) { _item = sub; }This reduces the total latency when both queries are independent.
app/reducers/quickActions.ts (1)
25-33: Consider refining the type narrowing.The type guard on line 26 correctly checks for the payload, but line 31 still uses a type assertion. Since
QuickActionsActionis a union including the bareActiontype, TypeScript cannot automatically narrow the type after the guard. While the current implementation is functionally correct, you could improve type safety by using a more explicit type predicate or restructuring the action types to avoid the manual cast.♻️ Alternative approach using type predicate
+function isQuickActionPayload(action: QuickActionsAction): action is IQuickActionPayloadAction { + return 'payload' in action && !!action.payload?.action; +} + export default function quickActions(state = initialState, action: QuickActionsAction): IQuickActionsState { switch (action.type) { case QUICK_ACTIONS.QUICK_ACTION_HANDLE: - if (!('payload' in action) || !action.payload?.action) { + if (!isQuickActionPayload(action)) { return state; } return { ...state, - lastAction: (action as IQuickActionPayloadAction).payload.action, + lastAction: action.payload.action, handled: false };ios/AppDelegate.swift (1)
7-7: Optional: Remove redundant Foundation import.The
Foundationimport is typically unnecessary in iOS applications as it's already available throughUIKitand other frameworks imported in this file. While not harmful, removing it can reduce clutter.
📜 Review details
Configuration used: Organization 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 (2)
ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (41)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.ktandroid/app/src/main/res/drawable/ic_quickaction_add.xmlandroid/app/src/main/res/drawable/ic_quickaction_contact.xmlandroid/app/src/main/res/drawable/ic_quickaction_find.xmlandroid/app/src/main/res/drawable/ic_quickaction_recent.xmlapp/AppContainer.tsxapp/actions/actionsTypes.tsapp/actions/quickActions.tsapp/actions/rooms.tsapp/definitions/redux/index.tsapp/i18n/locales/en.jsonapp/i18n/locales/pt-BR.jsonapp/index.tsxapp/lib/constants/keys.tsapp/lib/methods/helpers/emitter.tsapp/lib/methods/helpers/goRoom.tsapp/lib/navigation/addServer/index.tsapp/lib/quickActions/getRecentQuickAction.tsapp/lib/quickActions/index.tsapp/lib/store/index.tsapp/reducers/index.jsapp/reducers/quickActions.tsapp/reducers/rooms.tsapp/reducers/ui.tsapp/sagas/index.jsapp/sagas/init.jsapp/sagas/quickActions.tsapp/sagas/roomVisited.tsapp/sagas/rooms.jsapp/views/RoomsListView/hooks/useSearch.tsapp/views/SettingsView/index.tsxios/AppDelegate.swiftios/Podfileios/QuickActionsConnector.hios/QuickActionsConnector.mmios/QuickActionsStore.hios/QuickActionsStore.mmios/RocketChatRN-Bridging-Header.hios/RocketChatRN.xcodeproj/project.pbxprojios/RocketChatRN/Info.plistpackage.json
🧰 Additional context used
🧬 Code graph analysis (18)
app/lib/store/index.ts (1)
app/lib/store/auxStore.ts (1)
store(6-6)
app/definitions/redux/index.ts (1)
app/reducers/ui.ts (1)
IUIState(5-7)
app/lib/quickActions/index.ts (3)
app/lib/quickActions/getRecentQuickAction.ts (1)
getRecentQuickAction(9-20)app/lib/store/auxStore.ts (1)
store(6-6)app/actions/quickActions.ts (1)
quickActionHandle(11-16)
app/sagas/roomVisited.ts (4)
app/lib/methods/helpers/events.ts (1)
emit(49-59)app/lib/methods/helpers/emitter.ts (1)
emitter(28-28)app/actions/rooms.ts (1)
roomsStoreLastVisited(54-60)app/lib/constants/keys.ts (2)
LAST_VISITED_ROOM_ID_KEY(28-28)LAST_VISITED_ROOM_Name_KEY(29-29)
app/actions/rooms.ts (2)
app/sagas/deepLinking.js (1)
name(62-62)app/actions/actionsTypes.ts (1)
ROOMS(15-15)
app/lib/methods/helpers/goRoom.ts (2)
app/lib/methods/getRoom.ts (1)
getRoom(4-12)app/lib/methods/helpers/emitter.ts (1)
emitter(28-28)
app/actions/quickActions.ts (1)
app/actions/actionsTypes.ts (1)
QUICK_ACTIONS(63-63)
app/reducers/ui.ts (2)
app/reducers/quickActions.ts (1)
initialState(10-13)app/actions/actionsTypes.ts (1)
UI(65-65)
app/lib/navigation/addServer/index.ts (4)
app/lib/store/index.ts (1)
AppDispatch(30-30)app/lib/methods/helpers/log/index.ts (2)
logEvent(35-42)events(7-7)app/actions/app.ts (1)
appStart(31-37)app/actions/server.ts (1)
serverInitAdd(104-109)
app/AppContainer.tsx (5)
app/sagas/init.js (1)
lastVisitedRoomName(56-56)app/lib/hooks/useAppSelector.ts (1)
useAppSelector(6-6)app/definitions/redux/index.ts (1)
IApplicationState(53-81)app/lib/quickActions/index.ts (2)
registerQuickActions(55-69)updateQuickActions(22-53)app/actions/actionsTypes.ts (1)
NAVIGATION(64-64)
app/reducers/rooms.ts (1)
app/actions/actionsTypes.ts (1)
ROOMS(15-15)
app/reducers/quickActions.ts (2)
app/reducers/ui.ts (1)
initialState(9-11)app/actions/actionsTypes.ts (1)
QUICK_ACTIONS(63-63)
app/views/SettingsView/index.tsx (2)
app/lib/methods/helpers/log/index.ts (1)
logEvent(35-42)app/lib/methods/helpers/info.ts (1)
showErrorAlert(5-6)
app/index.tsx (3)
app/lib/quickActions/getRecentQuickAction.ts (1)
getRecentQuickAction(9-20)app/lib/store/auxStore.ts (1)
store(6-6)app/actions/quickActions.ts (1)
quickActionHandle(11-16)
app/sagas/init.js (2)
app/lib/constants/keys.ts (2)
LAST_VISITED_ROOM_ID_KEY(28-28)LAST_VISITED_ROOM_Name_KEY(29-29)app/actions/rooms.ts (1)
roomsStoreLastVisited(54-60)
app/sagas/quickActions.ts (9)
app/actions/app.ts (2)
appInit(45-49)appStart(31-37)app/actions/actionsTypes.ts (4)
APP(35-43)NAVIGATION(64-64)UI(65-65)QUICK_ACTIONS(63-63)app/lib/methods/getRoom.ts (1)
getRoom(4-12)app/sagas/init.js (2)
server(27-27)root(83-87)app/lib/constants/keys.ts (1)
CURRENT_SERVER(26-26)app/actions/server.ts (1)
serverInitAdd(104-109)app/views/SettingsView/index.tsx (1)
sendEmail(36-50)app/actions/rooms.ts (1)
roomsStoreLastVisited(54-60)app/lib/methods/helpers/goRoom.ts (1)
goRoom(87-156)
ios/QuickActionsStore.h (1)
app/sagas/deepLinking.js (1)
type(61-61)
app/views/RoomsListView/hooks/useSearch.ts (3)
app/lib/hooks/useAppSelector.ts (1)
useAppSelector(6-6)app/actions/actionsTypes.ts (1)
UI(65-65)app/lib/methods/search.ts (1)
search(114-185)
🪛 Biome (2.1.2)
app/sagas/quickActions.ts
[error] 71-71: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 Clang (14.0.6)
ios/QuickActionsConnector.h
[error] 1-1: 'React/RCTBridgeModule.h' file not found
(clang-diagnostic-error)
ios/QuickActionsStore.h
[error] 1-1: 'Foundation/Foundation.h' file not found
(clang-diagnostic-error)
🔇 Additional comments (21)
ios/Podfile (1)
12-12: LGTM!The ExpoQuickActions pod declaration is correctly configured with the standard local path to node_modules, aligning with the npm dependency addition in package.json.
android/app/src/main/res/drawable/ic_quickaction_recent.xml (1)
1-9: LGTM!The vector drawable is correctly formatted with standard Android vector attributes and provides a clear recent/clock icon for the quick action.
app/sagas/rooms.js (2)
20-24: Stylistic-only callback param change looks safe.Line 21 changes
record => { ... }formatting only; behavior remains the same.
76-128: Bulk formatting changes in record preparation look safe.Lines 78-127: only arrow-param formatting updates (
sub =>,message =>, etc.). No logic changes spotted.app/sagas/index.js (1)
18-40: No race condition exists here;quickActionsandroomVisitedhave defensive guards or no dependencies oninit()completion.The root saga's
yield all([...])pattern is safe:
- The
init(),quickActions(), androomVisited()generators are watcher sagas that register listeners and return immediately; they don't execute logic on startup.quickActionsincludes defensivewaitForAppReady()checks (lines 70, 80, 83) that block untilAPP.READYis dispatched before handling actions.roomVisitedis purely event-driven fromemitter.on('roomVisited')with no app state dependency.- The
initsaga only responds to anAPP.INITaction dispatched from the React component; actual initialization logic doesn't run until that action arrives.The parallel wiring is intentional and correct.
Likely an incorrect or invalid review comment.
ios/RocketChatRN-Bridging-Header.h (1)
13-13: The import ofQuickActionsStore.his correctly configured. The header exists, is properly exported viaFOUNDATION_EXPORT, and is already referenced multiple times in the project configuration. The import syntax matches existing working imports in the same bridging header.ios/QuickActionsConnector.h (1)
1-4: LGTM! Standard React Native bridge module header.The interface declaration follows React Native conventions for native module bridging. The static analysis error about the missing React header is a false positive—the React Native headers will be available during the Xcode build process.
app/sagas/init.js (1)
55-60: LGTM! Last visited room restoration logic is correct.The logic appropriately:
- Reads both room ID and name from UserPreferences
- Guards against partial data by checking both values exist
- Dispatches the action to restore state
app/lib/navigation/addServer/index.ts (1)
9-16: LGTM! Clean navigation helper with proper batching.The implementation correctly:
- Logs analytics events before navigation
- Uses
batch()to dispatch multiple actions atomically, preventing intermediate re-renders- Provides clear type annotations
app/reducers/rooms.ts (1)
51-56: LGTM! Reducer logic correctly stores last visited room.The reducer case properly:
- Spreads existing state
- Maps action payload to state fields
- Maintains immutability
app/definitions/redux/index.ts (1)
51-51: No review comment was provided for verification and rewriting. Please provide the review comment within<review_comment>tags so I can proceed with the verification and rewriting process.app/AppContainer.tsx (1)
69-72: NAVIGATION_READY dispatch on container ready looks good.
Keeps Redux in sync with navigation readiness and complements the existingemitter.emit('navigationReady').android/app/src/main/res/drawable/ic_quickaction_find.xml (1)
1-5: LGTM for a shortcut icon vector resource.app/i18n/locales/pt-BR.json (1)
322-322: This review comment identifies the wrong lines. Only line 827 contains punctuation in an i18n key (Something_Wrong?), but lines 447, 686-687, and 966 do not. Additionally, keys with?are already present throughout the codebase (22 keys in pt-BR.json alone, and similar patterns across all 25 locales), and the tooling (i18n-js 3.9.2) handles them without issue.Likely an incorrect or invalid review comment.
app/lib/methods/helpers/emitter.ts (1)
20-23: LGTM! Well-typed event addition.The new
roomVisitedevent is properly typed withridandnamefields, consistent with the existing event patterns in the emitter.ios/AppDelegate.swift (1)
46-53: LGTM! Correct iOS quick actions implementation.The quick action handling follows iOS best practices:
- Extracting the shortcut item from launch options and storing it via
RCSetPendingQuickActionType- Returning
falseindidFinishLaunchingWithOptionsto prevent iOS from invokingperformActionForagain- Properly implementing the
performActionForcallback with completion handlerAlso applies to: 90-97
app/actions/rooms.ts (1)
17-20: LGTM! Well-structured action additions.The new
IRoomsLastVisitedinterface androomsStoreLastVisitedaction creator are properly typed and consistent with existing action patterns in the file.Also applies to: 54-60
app/reducers/index.js (1)
29-30: LGTM! Clean reducer integration.The new
quickActionsanduireducers are properly imported and integrated into the root reducer using standard Redux patterns.Also applies to: 40-40, 43-43
app/lib/quickActions/index.ts (1)
22-53: Quick action item definitions look consistent (ids, i18n, platform icons).
Theidvalues align with saga handling (search,add-server,recent,contact), and usingrecentRoomNamefor the “recent” title is a nice touch.ios/RocketChatRN.xcodeproj/project.pbxproj (1)
13-16: QuickActions native files are correctly wired into sources (both app targets).
Looks consistent:.mmfiles are present inPBXSourcesBuildPhaseand headers are referenced.Also applies to: 477-481, 739-762, 2031-2085, 2308-2362
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt (1)
104-146: Verify Expo quick-action intent contract against documentation or custom Expo module configuration.The code assumes a specific Intent action (
"expo.modules.quickactions.SHORTCUT") and Intent extra ("shortcut_data"asPersistableBundle) that cannot be confirmed against publicly available Expo Quick Actions documentation. Standard Expo Quick Actions patterns on Android useAction.paramsmapped to Intent extras viaShortcutManager, with primitives and package-namespaced keys—not a custom"expo.modules.quickactions.SHORTCUT"action or unprefixed bundle keys like"id"or"shortcutId".Confirm whether this Intent contract is defined in Rocket.Chat's Expo module configuration, custom Expo patches, or undocumented Expo behavior before relying on these assumptions.
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt
Outdated
Show resolved
Hide resolved
| 1E1EA8082326CCE300E22452 /* ShellScript */ = { | ||
| isa = PBXShellScriptBuildPhase; | ||
| alwaysOutOfDate = 1; | ||
| buildActionMask = 2147483647; | ||
| files = ( | ||
| ); | ||
| inputFileListPaths = ( | ||
| ); | ||
| inputPaths = ( | ||
| ); | ||
| name = "[Expo] Configure project"; | ||
| outputFileListPaths = ( | ||
| ); | ||
| outputPaths = ( | ||
| ); | ||
| runOnlyForDeploymentPostprocessing = 0; | ||
| shellPath = /bin/sh; | ||
| shellScript = "# This script configures Expo modules and generates the modules provider file.\nbash -l -c \"./Pods/Target\\ Support\\ Files/Pods-defaults-RocketChatRN/expo-configure-project.sh\"\n"; | ||
| shellScript = "echo \"Target architectures: $ARCHS\"\n\nAPP_PATH=\"${TARGET_BUILD_DIR}/${WRAPPER_NAME}\"\n\nfind \"$APP_PATH\" -name '*.framework' -type d | while read -r FRAMEWORK\ndo\nFRAMEWORK_EXECUTABLE_NAME=$(defaults read \"$FRAMEWORK/Info.plist\" CFBundleExecutable)\nFRAMEWORK_EXECUTABLE_PATH=\"$FRAMEWORK/$FRAMEWORK_EXECUTABLE_NAME\"\necho \"Executable is $FRAMEWORK_EXECUTABLE_PATH\"\necho $(lipo -info \"$FRAMEWORK_EXECUTABLE_PATH\")\n\nFRAMEWORK_TMP_PATH=\"$FRAMEWORK_EXECUTABLE_PATH-tmp\"\n\n# remove simulator's archs if location is not simulator's directory\ncase \"${TARGET_BUILD_DIR}\" in\n*\"iphonesimulator\")\necho \"No need to remove archs\"\n;;\n*)\nif $(lipo \"$FRAMEWORK_EXECUTABLE_PATH\" -verify_arch \"i386\") ; then\nlipo -output \"$FRAMEWORK_TMP_PATH\" -remove \"i386\" \"$FRAMEWORK_EXECUTABLE_PATH\"\necho \"i386 architecture removed\"\nrm \"$FRAMEWORK_EXECUTABLE_PATH\"\nmv \"$FRAMEWORK_TMP_PATH\" \"$FRAMEWORK_EXECUTABLE_PATH\"\nfi\nif $(lipo \"$FRAMEWORK_EXECUTABLE_PATH\" -verify_arch \"x86_64\") ; then\nlipo -output \"$FRAMEWORK_TMP_PATH\" -remove \"x86_64\" \"$FRAMEWORK_EXECUTABLE_PATH\"\necho \"x86_64 architecture removed\"\nrm \"$FRAMEWORK_EXECUTABLE_PATH\"\nmv \"$FRAMEWORK_TMP_PATH\" \"$FRAMEWORK_EXECUTABLE_PATH\"\nfi\n;;\nesac\n\necho \"Completed for executable $FRAMEWORK_EXECUTABLE_PATH\"\necho $\n\ndone\n"; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine the script in context
find . -name "project.pbxproj" -type f | head -5Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 122
🏁 Script executed:
# Search for the specific shell script in the pbxproj file
rg "1E1EA8082326CCE300E22452" --type-list | head -5Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 467
🏁 Script executed:
# Look for the file structure
ls -la ios/ 2>/dev/null | head -20Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1367
🏁 Script executed:
# Check if the pbxproj file exists and examine build phases
if [ -f "ios/RocketChatRN.xcodeproj/project.pbxproj" ]; then
wc -l "ios/RocketChatRN.xcodeproj/project.pbxproj"
echo "---"
# Try to extract lines around 1567-1583
sed -n '1560,1590p' "ios/RocketChatRN.xcodeproj/project.pbxproj"
fiRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 2364
🏁 Script executed:
# Search for similar shell scripts or lipo commands in the pbxproj
rg "lipo|ShellScript" ios/RocketChatRN.xcodeproj/project.pbxproj -A 2 -B 2 | head -50Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 3171
🏁 Script executed:
# Check the Podfile for xcframeworks usage
cat ios/Podfile | head -50Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1613
🏁 Script executed:
# Search for any references or comments about this ShellScript or architecture stripping
rg "architecture|lipo|1E1EA8082326CCE300E22452" ios/ --type-listRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 467
🏁 Script executed:
# Look for CI configuration
find . -name ".github" -o -name ".gitlab-ci.yml" -o -name "*.yml" -o -name "*.yaml" | grep -E "(github|gitlab|circle|travis|action)" | head -10Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 485
🏁 Script executed:
# Search for any build configuration or documentation mentioning this
rg "ShellScript|strip.*arch" . -t md -t txt | head -20Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 60
🏁 Script executed:
# Search for references to architecture stripping or this build phase
rg "lipo|verify_arch" ios/ -A 2 -B 2Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1789
🏁 Script executed:
# Check the CI build configuration to understand if this is actually run
cat .github/actions/build-ios/action.yml | head -100Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 3506
🏁 Script executed:
# Look for any other shell scripts in the project that might indicate modern approach
rg "use_frameworks|xcframework" ios/Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 127
🏁 Script executed:
# Search for any issues or TODOs related to architecture stripping
rg "TODO|FIXME|NOTE" ios/RocketChatRN.xcodeproj/ -iRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 60
🏁 Script executed:
# Check if there are any mentions of this approach in recent commits or discussions
git log --oneline ios/RocketChatRN.xcodeproj/project.pbxproj 2>/dev/null | head -20Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 129
🏁 Script executed:
# Let's verify if this script is actually being executed or if it's dead code
# Check if the build phase is referenced in the main target
rg "1E1EA8082326CCE300E22452" ios/RocketChatRN.xcodeproj/project.pbxproj -B 5 -A 5Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1151
🏁 Script executed:
# Check for any modern architecture handling in CocoaPods or Xcode config
rg "ARCHS|VALID_ARCHS|arm64" ios/RocketChatRN.xcodeproj/project.pbxproj | head -20Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1739
🏁 Script executed:
# Verify the exact content of that trailing echo line
python3 << 'EOF'
import re
content = open('ios/RocketChatRN.xcodeproj/project.pbxproj', 'r').read()
# Extract the shellScript value
match = re.search(r'shellScript = "([^"]*echo \$[^"]*)"', content)
if match:
script = match.group(1)
# Decode escape sequences
script = script.encode().decode('unicode_escape')
lines = script.split('\n')
for i, line in enumerate(lines[-5:], len(lines)-4):
print(f"Line {i}: {repr(line)}")
EOFRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 60
🏁 Script executed:
# Let's decode the exact trailing line to confirm it's malformed
python3 << 'EOF'
# Read the pbxproj file
with open('ios/RocketChatRN.xcodeproj/project.pbxproj', 'r') as f:
content = f.read()
# Find the specific script
import re
# Look for the pattern with the echo $ at the end
match = re.search(r'echo "Completed for executable.*?\necho \$\n', content)
if match:
text = match.group(0)
print("Found trailing lines:")
print(repr(text))
print("\nDecoded:")
print(text)
else:
print("Pattern not found, searching differently...")
# Just look for the last few lines of that script
match = re.search(r'Completed for executable.*?done', content, re.DOTALL)
if match:
lines = match.group(0).split('\n')
for line in lines[-5:]:
print(repr(line))
EOFRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 197
🏁 Script executed:
# Verify if EXCLUDED_ARCHS is properly configured to handle architecture filtering
# This is the modern approach that should make the shell script unnecessary
rg "EXCLUDED_ARCHS.*iphonesimulator" ios/RocketChatRN.xcodeproj/project.pbxprojRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 231
🏁 Script executed:
# Check if this build phase is actually necessary by looking for framework handling in CocoaPods
cat ios/Podfile | grep -A 10 -B 10 "frameworks\|xcframework" || echo "No xcframework config found"Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 727
Remove this redundant and broken shell script build phase.
The script is malformed (trailing echo $ is incomplete), unnecessary, and potentially unsafe. The project already handles architecture filtering properly via EXCLUDED_ARCHS[sdk=iphonesimulator*] configuration, and modern CocoaPods with static framework linking handles framework embedding automatically. Direct lipo modifications on frameworks in the build directory can interfere with code signing and framework embedding, causing build failures especially with xcframeworks. Delete the entire 1E1EA8082326CCE300E22452 ShellScript build phase.
🤖 Prompt for AI Agents
In @ios/RocketChatRN.xcodeproj/project.pbxproj around lines 1567 - 1583, Remove
the entire PBXShellScriptBuildPhase entry with identifier
1E1EA8082326CCE300E22452 from the project file and delete any references to that
ID from target buildPhases arrays so the shell script is fully removed; ensure
you also remove its block from PBXBuildPhase lists (and any duplicate
occurrences) to avoid dangling references that would break the project.
|
The specified version ^6.0.0 targets Expo SDK 54, not SDK 53. For SDK 53 👀 |
yup, |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt (2)
60-101: Consider making debug logs conditional for production builds.The implementation correctly handles Expo quick actions with proper backward compatibility for the
PersistableBundleAPI and defensive fallback keys for shortcut ID extraction. However, theandroid.util.Log.dcalls throughout this method (and lines 105, 120) will appear in production builds.🔧 Optional: Wrap logs in BuildConfig.DEBUG check
+import chat.rocket.reactnative.BuildConfig + private fun embedQuickActionIntoLinking(intent: Intent) { val action = intent.action ?: return - android.util.Log.d("RocketChat.QuickAction", "Original action: $action") + if (BuildConfig.DEBUG) { + android.util.Log.d("RocketChat.QuickAction", "Original action: $action") + }Apply similar wrapping to the other log statements at lines 89, 95, 98, 105, and 120.
103-107: Clarify the comment to avoid confusion.The comment "skip for non-Expo quick actions (app launches)" is misleading since the code below actually handles non-Expo quick actions. The skip applies to actions that don't match the package prefix (i.e., standard app launches or other intents).
📝 Suggested comment clarification
- // skip for non-Expo quick actions (app launches) + // Skip actions that aren't package-prefixed quick actions (e.g., standard app launches) if (!action.startsWith("chat.rocket.reactnative.")) { android.util.Log.d("RocketChat.QuickAction", "Not a quick action, skipping") return }
📜 Review details
Configuration used: Organization 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)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt
🔇 Additional comments (4)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt (4)
9-15: LGTM!The new imports are appropriate for the quick action handling implementation:
Buildfor API level checks,PersistableBundlefor Expo shortcut data extraction, andtoUriextension for URI conversion.
32-43: LGTM!The order of operations is correct: normalizing the quick action intent before
super.onCreate()ensures the React Native Linking module receives the properly formattedrocketchat://quick-action/...URI when the app initializes.
45-54: LGTM!The intent handling flow is correct for when the activity is already running. The quick action normalization followed by notification handling ensures both pathways work properly.
109-121: Kebab-case transformation is correct and aligns with JS quick action handlers.The transformation logic (removing prefix, lowercasing, replacing underscores with dashes) correctly converts action strings like
chat.rocket.reactnative.ADD_SERVERtoadd-server, which matches the expected action names in the JavaScript saga (app/sagas/quickActions.ts). The resulting URI formatrocketchat://quick-action/{action}is properly handled by the deeplink router inapp/index.tsx.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt:
- Around line 45-54: Reorder onNewIntent so you normalize the intent via
embedQuickActionIntoLinking(intent) before calling super.onNewIntent(intent);
specifically, call embedQuickActionIntoLinking(intent) (and then
setIntent(intent) to update the activity intent) first so the ReactActivity
delegate receives the rewritten deep link, then call super.onNewIntent(intent),
and finally invoke NotificationIntentHandler.handleIntent(this, intent).
- Around line 60-136: The locale-sensitive call in embedQuickActionIntoLinking
uses .lowercase() which can produce incorrect mappings in some locales; change
the quickAction computation to use .lowercase(Locale.ROOT) (import
java.util.Locale if not present) so the transform is locale-stable; update the
expression that builds quickAction (the chain starting from
action.removePrefix("chat.rocket.reactnative.")...) to call
.lowercase(Locale.ROOT) instead of .lowercase().
📜 Review details
Configuration used: Organization 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)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt
🔇 Additional comments (2)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt (2)
8-16: Imports look fine;toUri()is a good call here.
No concerns with these additions.
32-43: Good: normalize the launch intent before React init.
Doing this beforesuper.onCreate(...)is the right direction for “killed state” quick actions so React Native reads the rewritten deep link.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt:
- Around line 128-137: The code builds a quickAction and unconditionally creates
a URI/intent even when the suffix is empty, producing
"rocketchat://quick-action/"; update the logic around action, quickAction, uri
and setIntent so that after computing quickAction =
action.removePrefix("chat.rocket.reactnative.").lowercase(...).replace(...), you
check quickAction.isNotBlank() (or non-empty after trimming) before constructing
the uri and calling intent.action/intent.data/setIntent; if quickAction is
blank, skip creating the intent (or provide a safe default path) to avoid
sending a trailing-slash quick-action URI to downstream handlers.
🧹 Nitpick comments (1)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt (1)
87-111: Add debug logging when shortcut ID cannot be extracted.When
shortcutDatais non-null but none of the expected keys (id,shortcutId,android.intent.extra.shortcut.ID) are present, the function silently fails without logging. This could make debugging difficult if Expo changes the bundle format.Suggested improvement
if (shortcutId != null) { val uri = "rocketchat://quick-action/$shortcutId".toUri() if (BuildConfig.DEBUG) { android.util.Log.d("RocketChat.QuickAction", "Converted to: $uri") } intent.action = Intent.ACTION_VIEW intent.data = uri setIntent(intent) if (BuildConfig.DEBUG) { android.util.Log.d( "RocketChat.QuickAction", "Intent set with data: ${getIntent().data}" ) } + } else { + if (BuildConfig.DEBUG) { + android.util.Log.d("RocketChat.QuickAction", "No shortcut ID found in bundle keys") + } }
📜 Review details
Configuration used: Organization 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)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt
🔇 Additional comments (1)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt (1)
66-142: Quick action embedding logic looks well-structured.The approach of normalizing quick action intents into deep link URIs (
rocketchat://quick-action/{id}) is clean and integrates well with the existing linking flow. The version-awarePersistableBundlehandling for API 33+ is correctly implemented, and the fallback keys for shortcut ID extraction provide good backward compatibility.
|
Hey Divyanshu, We think it would be better to show the last three visited rooms in the Quick Actions instead of showing Search and Add Workspace. This would help users quickly open a channel they recently visited. As an improvement, we should also display the room image instead of an icon. A room should appear in Quick Actions only when a user clicks on a room from the list and enters it. |
this is the expo-quick-actions limitation as image must be bundled with the app so we can't do that rn |
@Rohit3523 the original feature #1705 mentioned this 3 actions described waiting for feedback before proceeding |
Let me discuss about this with the team |
Yes, the original idea was to implement Add Workspace, Search, and Recent Chats. However, after reviewing several apps, we noticed that most of them already show recent chats in the options and we realized that Add Workspace and Search would be used very rarely by users. So, we decided to simply show the last three visited rooms. Additionally, please make sure that if more than one workspace is added and rooms from multiple workspaces are shown in the list, clicking on an item opens the chat in the correct workspace. |
okay thanks for feedback, |
|
converting to draft because of requested changes |
|
Sharing a quick demo for feedback updated.quick.actions.demo.mp4 |
|
Hello maintainers, I am facing a weird bug while switching servers after launching the app from quick actions. In this bug, when I launch the app using a quick action (e.g., recent room shortcut) to switch to a different server than the currently active one, the server names in the workspace list get mixed up. Both servers end up showing the same name instead of their actual distinct names. For example:
But after launching via quick action to Server 2, both servers display "Rocket.Chat Open" (or sometimes both show "div"). The server URLs/IDs remain correct, only the display names are duplicated. Steps to reproduce:
What I found:
The issue seems to be related to WatermelonDB caching or a race condition during the quick action flow. switch.server.3.mp4 |
|
When the app is launched using a quick action for a different server, both servers end up displaying the same name fetched from the database. This behavior is unexpected, difficult to reproduce consistently, and hard to trace back to a clear root cause. adding delays doesn't solves this Note happens mostly with local servers and works fine with deployed
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/reducers/index.js (1)
29-41: AddquickActionstoIApplicationStatetype definition.The
quickActionsreducer is wired into the rootcombineReducersinapp/reducers/index.js(line 41), but the type definition atapp/definitions/redux/index.tsis missing the corresponding property. AddquickActions: IQuickActionsState;to theIApplicationStateinterface (around line 79) to keep types in sync with the runtime state shape.
🧹 Nitpick comments (4)
app/reducers/rooms.ts (1)
61-89: WrapSTORE_LAST_VISITEDcase in a block for better scoping practice.
While theno-case-declarationsrule is currently disabled in the project's ESLint config, wrapping lexical declarations in a block scope prevents potential variable hoisting issues and follows best practices for switch statements.🛠️ Suggested fix
- case ROOMS.STORE_LAST_VISITED: + case ROOMS.STORE_LAST_VISITED: { const server = UserPreferences.getString(CURRENT_SERVER); const newRoom = { rid: action.lastVisitedRoomId, name: action.lastVisitedRoomName, server }; @@ return { ...state, lastVisitedRid: action.lastVisitedRoomId, lastVisitedName: action.lastVisitedRoomName, recentRooms: updatedRecentRooms }; + } case ROOMS.STORE_RECENT_ROOMS:app/lib/quickActions/index.ts (1)
80-85:appStateSubscriptionis not reset to null inunregisterQuickActions.The cleanup function resets
quickActionSubscriptionto null but notappStateSubscription, which could lead to inconsistent state if someone checks the subscription variables.Proposed fix
export function unregisterQuickActions() { quickActionSubscription?.remove(); appStateSubscription?.remove(); quickActionSubscription = null; + appStateSubscription = null; registered = false; }app/AppContainer.tsx (2)
43-49: Async function called without error handling in effect.
registerQuickActions()is async but its promise is not handled. If it throws, the error will be silently swallowed. Consider adding error handling or logging.Proposed fix
useEffect(() => { - registerQuickActions(); + registerQuickActions().catch(e => { + // Log or handle registration failure + console.warn('Failed to register quick actions:', e); + }); return () => { unregisterQuickActions(); }; }, []);
60-62: Same unhandled promise concern forupdateQuickActions.If
updateQuickActionsfails (e.g.,getServersList()throws), the error is silently ignored.Proposed fix
useEffect(() => { - updateQuickActions({ recentRooms }); + updateQuickActions({ recentRooms }).catch(e => { + console.warn('Failed to update quick actions:', e); + }); }, [recentRooms]);
3 participants
Proposed changes
This pull request introduces Quick Actions support to the app, including full support when the app is launched from a killed state on both iOS and Android.
It adds the complete Quick Actions flow end-to-end, enabling users to quickly access common actions such as Search, Add Server, Recent Rooms, and Contact us directly from the app icon. Platform-specific handling is implemented to ensure reliable behavior during cold starts.
Issue(s)
closes: #1705
closes: #6227
Approach for Killed state
iOS
Android
How to test or reproduce
Screenshots
Quick Actions
Search
Add Server
Recent rooms
Contact
Types of changes
Checklist
Further comments
the quick actions icons on android are temporary and should be changed later
Summary by CodeRabbit
New Features
Localization
Tests
✏️ Tip: You can customize this high-level summary in your review settings.