Conversation
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
|
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:
WalkthroughAdds new UI components (Icon, IconButton, InfoCard), broad UI kit type/signature changes for v0.39.0, interaction adapter utilities for mapping server↔frontend interactions, Markdown textStyle context support, icon mapping updates, many i18n bold call messages, tests, and small build/config updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant IconButton as IconButton(Component)
participant Parser as MessageParser
participant Adapter as interactionAdapters
participant Frontend as triggerAction
participant Server
participant Handler as handlePayloadUserInteraction
User->>IconButton: press
IconButton->>Parser: build ITriggerAction / invoke action handler
Parser->>Adapter: toUserInteraction(ITriggerAction)
Adapter->>Frontend: UserInteraction
Frontend->>Server: POST /api/apps/ui.interaction/... (interaction)
Server-->>Frontend: response (modal.open | modal.update | modal.close | errors)
Frontend->>Adapter: toServerModalInteractionType(response.type)
Frontend->>Handler: handlePayloadUserInteraction(parsedType, payload)
Handler-->>Frontend: update UI / emit events / navigate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes 🚥 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. 📝 Coding Plan
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 |
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
There was a problem hiding this comment.
Pull request overview
This PR upgrades the app’s UIKit integration to support newer block types (notably info_card) and adds styling/i18n improvements needed for voice/call-related message blocks.
Changes:
- Bump
@rocket.chat/ui-kitto^0.39.0and adjust Jest transforms for the package. - Add new UIKit rendering capabilities (InfoCard, Icon, IconButton) and adapt interaction payloads to the newer UI Kit types.
- Extend Markdown rendering to accept a
textStyleoverride propagated via context (with updated stories/snapshots).
Reviewed changes
Copilot reviewed 28 out of 34 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Locks new transitive deps from the @rocket.chat/ui-kit upgrade (incl. typia). |
| tsconfig.json | Formatting-only updates to inline comments. |
| package.json | Upgrades @rocket.chat/ui-kit dependency to ^0.39.0. |
| jest.config.js | Allows transforming @rocket.chat/ui-kit during Jest runs. |
| app/views/ModalBlockView.tsx | Updates modal update handler typing to new modal action type. |
| app/lib/methods/actions.ts | Refactors trigger interaction handling using adapters and updated UI Kit types. |
| app/i18n/locales/en.json | Adds i18n keys used by new call/voice UI blocks. |
| app/containers/markdown/index.tsx | Adds textStyle prop and provides it through Markdown context. |
| app/containers/markdown/contexts/MarkdownContext.ts | Adds textStyle to context shape/default state. |
| app/containers/markdown/components/mentions/Hashtag.tsx | Applies context textStyle to hashtag rendering. |
| app/containers/markdown/components/mentions/AtMention.tsx | Applies context textStyle to @mention rendering. |
| app/containers/markdown/components/inline/Link.tsx | Applies context textStyle to link rendering. |
| app/containers/markdown/components/Plain.tsx | Applies context textStyle to plain text rendering. |
| app/containers/markdown/snapshots/Markdown.test.tsx.snap | Updates snapshots for new textStyle story output. |
| app/containers/markdown/Markdown.stories.tsx | Adds a Storybook story to validate textStyle behavior. |
| app/containers/UIKit/interfaces.ts | Reworks UIKit types to align with @rocket.chat/ui-kit@0.39.x and adds InfoCard/Icon types. |
| app/containers/UIKit/interactionAdapters.ts | Introduces adapters to convert app trigger actions to UI Kit UserInteraction payloads. |
| app/containers/UIKit/index.tsx | Adds parser support for info_card, icon, icon_button, and i18n-aware mrkdwn rendering. |
| app/containers/UIKit/snapshots/UiKitModal.test.tsx.snap | Snapshot updates due to mrkdwn rendering/style changes. |
| app/containers/UIKit/snapshots/UiKitMessage.test.tsx.snap | Snapshot updates + new snapshots for InfoCard stories. |
| app/containers/UIKit/VideoConferenceBlock/components/VideoConferenceBaseContainer.tsx | Updates ended-call icon name to the new mapped icon. |
| app/containers/UIKit/UiKitMessage.stories.tsx | Adds InfoCard stories and wraps stories with responsive layout context. |
| app/containers/UIKit/Section.tsx | Updates accessory rendering call signature; adds React keys for fields rendering. |
| app/containers/UIKit/Input.tsx | Updates inputs rendering call signature. |
| app/containers/UIKit/InfoCard.tsx | New component to render info_card blocks. |
| app/containers/UIKit/Image.tsx | Refactors image rendering helper signature. |
| app/containers/UIKit/IconButton.tsx | New component to render icon_button elements. |
| app/containers/UIKit/Icon.tsx | New component to render UI Kit icons with variants/framing and aliasing. |
| app/containers/UIKit/Context.tsx | Adds keys while rendering context elements; updates render call signature. |
| app/containers/UIKit/Actions.tsx | Updates render call signature; keeps “show more” rendering stable. |
| app/containers/CustomIcon/mappedIcons.js | Updates icon mappings (adds phone-off, phone-question-mark, remaps phone-issue). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/lib/methods/actions.ts (1)
92-145:⚠️ Potential issue | 🟠 MajorAvoid async Promise executor.
The async function inside
new Promise()is flagged by static analysis. If an exception is thrown before the firstawait(e.g., duringgenerateTriggerIdor accessingsdk.current), it won't properly reject the promise—it will throw synchronously.Refactor to use async/await at the function level and handle rejection explicitly.
🔧 Proposed refactor
-export function triggerAction({ type, actionId, appId, rid, mid, viewId, container, ...rest }: ITriggerAction) { - return new Promise<TModalAction | undefined | void>(async (resolve, reject) => { - const triggerId = generateTriggerId(appId); - const payload = rest.payload || rest.value; - - try { - const { userId, authToken } = sdk.current.currentLogin; - const { host } = sdk.current.client; +export async function triggerAction({ type, actionId, appId, rid, mid, viewId, container, ...rest }: ITriggerAction): Promise<TModalAction | undefined | void> { + const triggerId = generateTriggerId(appId); + const payload = rest.payload || rest.value; + + try { + const { userId, authToken } = sdk.current.currentLogin; + const { host } = sdk.current.client; - const interaction = toUserInteraction({ + const interaction = toUserInteraction({ - type, - actionId, - appId, - rid, - mid, - viewId, - container, - payload, - blockId: rest.blockId, - value: rest.value, - view: rest.view, - isCleared: rest.isCleared, - triggerId - }); + type, + actionId, + appId, + rid, + mid, + viewId, + container, + payload, + blockId: rest.blockId, + value: rest.value, + view: rest.view, + isCleared: rest.isCleared, + triggerId + }); - // we need to use fetch because this.sdk.post add /v1 to url - const result = await fetch(`${host}/api/apps/ui.interaction/${appId}/`, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'X-Auth-Token': authToken, - 'X-User-Id': userId - }, - body: JSON.stringify(interaction) - }); + // we need to use fetch because this.sdk.post add /v1 to url + const result = await fetch(`${host}/api/apps/ui.interaction/${appId}/`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Auth-Token': authToken, + 'X-User-Id': userId + }, + body: JSON.stringify(interaction) + }); - try { - const { type: interactionType, ...data } = await result.json(); - const modalType = toServerModalInteractionType(interactionType); - if (!modalType || modalType === ModalActions.CLOSE) { - return resolve(ModalActions.CLOSE); - } - - return resolve(handlePayloadUserInteraction(modalType, data)); - } catch (e) { - // modal.close has no body, so result.json will fail - // but it returns ok status - if (result.ok) { - return resolve(); - } + try { + const { type: interactionType, ...data } = await result.json(); + const modalType = toServerModalInteractionType(interactionType); + if (!modalType || modalType === ModalActions.CLOSE) { + return ModalActions.CLOSE; } - } catch (e) { - // do nothing + + return handlePayloadUserInteraction(modalType, data); + } catch (e) { + // modal.close has no body, so result.json will fail + // but it returns ok status + if (result.ok) { + return undefined; + } + throw e; } - return reject(); - }); + } catch (e) { + // do nothing + } + throw new Error('Trigger action failed'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/actions.ts` around lines 92 - 145, The Promise executor is async (new Promise(async (resolve, reject) => { ... })) which can cause uncaught synchronous throws (e.g., in generateTriggerId or sdk.current). Refactor by removing the async Promise executor: make the enclosing function async (or return the result of an async helper) and perform the logic with top-level try/catch: call generateTriggerId, build interaction via toUserInteraction, await the fetch to `${host}/api/apps/ui.interaction/${appId}/`, parse the JSON and convert with toServerModalInteractionType, then return the resolved values (use return ModalActions.CLOSE, return handlePayloadUserInteraction(...), or return undefined when result.ok but no body). On errors throw (or return a rejected Promise) so callers receive rejections rather than relying on resolve/reject inside an async executor; keep references to sdk.current, fetch call, toUserInteraction, toServerModalInteractionType, and handlePayloadUserInteraction when moving code out of the async executor.
🧹 Nitpick comments (5)
app/containers/UIKit/interactionAdapters.ts (2)
83-92: Complex type assertion on viewClosed payload.The inline type assertion for
viewis verbose. If this pattern is used elsewhere, consider defining a named type alias for better maintainability.♻️ Consider extracting a type alias
type ViewClosedView = IView & { id: string; state: { [blockId: string]: { [key: string]: unknown } } }; // Then use: view: view as ViewClosedView,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/UIKit/interactionAdapters.ts` around lines 83 - 92, Extract the verbose inline assertion for the view in the viewClosed payload into a named type alias (e.g., declare type ViewClosedView = IView & { id: string; state: Record<string, Record<string, unknown>> }) and replace the inline cast "view as IView & { id: string; state: { [blockId: string]: { [key: string]: unknown } } }" with "view as ViewClosedView"; update any other occurrences of the same pattern in this module that use the same shape to use the new ViewClosedView alias for consistency.
74-81: Consider stronger typing for viewSubmit payload.The
payload as anycast loses type safety. If the UI Kit defines a specific payload structure forviewSubmit, consider using that type instead.♻️ Suggested improvement
If
@rocket.chat/ui-kitexports aViewSubmitPayloadtype or similar:if (type === ActionTypes.SUBMIT) { return { type: 'viewSubmit', viewId: assertViewId(viewId), - payload: payload as any, + payload: payload as ViewSubmitPayload, triggerId }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/UIKit/interactionAdapters.ts` around lines 74 - 81, The view submit branch is using a loose cast ("payload as any") which loses type safety; change this to the appropriate typed payload (e.g., import and use ViewSubmitPayload or the exact exported type from `@rocket.chat/ui-kit`) and use that type for the returned object and/or payload variable instead of any; update the return type of the viewSubmit branch (and any surrounding union types) so viewId uses assertViewId(viewId) and payload is typed as ViewSubmitPayload (or the correct exported name) to preserve compile-time checking; ensure you add the import for the payload type and adjust any related type definitions where viewSubmit is declared.app/containers/UIKit/Section.tsx (1)
34-34: Avoidanyin field key generation.
(field as any).typeweakens type safety. Prefer a narrow typed discriminator helper (or safe in-operator check) for key composition.♻️ Suggested typed key approach
- key={`${(field as any).type || 'field'}-${index}`} + key={`${'type' in field ? field.type : 'field'}-${index}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/UIKit/Section.tsx` at line 34, The key generation uses (field as any).type which throws away type safety; introduce a narrow type guard (e.g., isFieldWithType or hasTypeProperty) that checks "type" in field and narrows its type, then replace the cast with a safe expression like `${isFieldWithType(field) ? field.type : 'field'}-${index}` when setting the key in Section.tsx so you avoid using any and keep proper typing for field.app/containers/UIKit/index.tsx (1)
70-75: Compatibility workaround noted.The
as anycast and comment document the workaround for@rocket.chat/ui-kit@0.39.0. Consider opening an issue upstream to add'info_card'to the allowed layout block types, and add a TODO comment here to remove the workaround once fixed.✨ Suggested improvement
constructor() { super(); - // Compatibility for `@rocket.chat/ui-kit`@0.39.0 where info_card is exported - // but still missing from message allowed layout block types. + // TODO: Remove this workaround once `@rocket.chat/ui-kit` includes 'info_card' + // in message allowed layout block types. + // Ref: Compatibility for `@rocket.chat/ui-kit`@0.39.0 this.allowedLayoutBlockTypes.add('info_card' as any); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/UIKit/index.tsx` around lines 70 - 75, Add a TODO and upstream issue reference for the compatibility workaround: update the constructor in UIKit where allowedLayoutBlockTypes.add('info_card' as any) is used to include a clear TODO comment that references opening an upstream issue in `@rocket.chat/ui-kit` to add 'info_card' to allowed layout block types and mark this cast for removal once the upstream fix is released; keep the existing compatibility line (allowedLayoutBlockTypes.add('info_card' as any)) but document the ticket/issue number or repo link and the condition under which it should be removed.app/lib/methods/actions.ts (1)
48-48: Redundant type assertion.
THandledServerPayloadalready definesviewId?: string, so the assertionas { viewId?: string }is unnecessary.✨ Suggested fix
- let { viewId } = data as { viewId?: string }; + let { viewId } = data;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/actions.ts` at line 48, Remove the redundant type assertion on the destructuring of data: the variable declaration let { viewId } = data as { viewId?: string }; should simply destructure from data (which is already typed as THandledServerPayload) so delete the "as { viewId?: string }" cast; update the line that references viewId/data in actions.ts (the let { viewId } ... statement) to rely on the existing THandledServerPayload type and ensure no other casts are added.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/containers/UIKit/InfoCard.tsx`:
- Around line 58-74: renderRowAction currently returns null unconditionally
which discards row.action; remove the temporary early return and restore the
icon rendering path by invoking parser.icon_button with the merged action props
and BlockContext.ACTION. Specifically, in renderRowAction use the provided
action plus fallback appId/blockId (replace the unused _appId/_blockId with the
real appId/blockId from surrounding scope or accept them as parameters), pass
{...action, appId: action.appId || appId || '', blockId: action.blockId ||
blockId || ''} into parser.icon_button, and ensure BlockContext is
imported/available so the call uses BlockContext.ACTION as before.
---
Outside diff comments:
In `@app/lib/methods/actions.ts`:
- Around line 92-145: The Promise executor is async (new Promise(async (resolve,
reject) => { ... })) which can cause uncaught synchronous throws (e.g., in
generateTriggerId or sdk.current). Refactor by removing the async Promise
executor: make the enclosing function async (or return the result of an async
helper) and perform the logic with top-level try/catch: call generateTriggerId,
build interaction via toUserInteraction, await the fetch to
`${host}/api/apps/ui.interaction/${appId}/`, parse the JSON and convert with
toServerModalInteractionType, then return the resolved values (use return
ModalActions.CLOSE, return handlePayloadUserInteraction(...), or return
undefined when result.ok but no body). On errors throw (or return a rejected
Promise) so callers receive rejections rather than relying on resolve/reject
inside an async executor; keep references to sdk.current, fetch call,
toUserInteraction, toServerModalInteractionType, and
handlePayloadUserInteraction when moving code out of the async executor.
---
Nitpick comments:
In `@app/containers/UIKit/index.tsx`:
- Around line 70-75: Add a TODO and upstream issue reference for the
compatibility workaround: update the constructor in UIKit where
allowedLayoutBlockTypes.add('info_card' as any) is used to include a clear TODO
comment that references opening an upstream issue in `@rocket.chat/ui-kit` to add
'info_card' to allowed layout block types and mark this cast for removal once
the upstream fix is released; keep the existing compatibility line
(allowedLayoutBlockTypes.add('info_card' as any)) but document the ticket/issue
number or repo link and the condition under which it should be removed.
In `@app/containers/UIKit/interactionAdapters.ts`:
- Around line 83-92: Extract the verbose inline assertion for the view in the
viewClosed payload into a named type alias (e.g., declare type ViewClosedView =
IView & { id: string; state: Record<string, Record<string, unknown>> }) and
replace the inline cast "view as IView & { id: string; state: { [blockId:
string]: { [key: string]: unknown } } }" with "view as ViewClosedView"; update
any other occurrences of the same pattern in this module that use the same shape
to use the new ViewClosedView alias for consistency.
- Around line 74-81: The view submit branch is using a loose cast ("payload as
any") which loses type safety; change this to the appropriate typed payload
(e.g., import and use ViewSubmitPayload or the exact exported type from
`@rocket.chat/ui-kit`) and use that type for the returned object and/or payload
variable instead of any; update the return type of the viewSubmit branch (and
any surrounding union types) so viewId uses assertViewId(viewId) and payload is
typed as ViewSubmitPayload (or the correct exported name) to preserve
compile-time checking; ensure you add the import for the payload type and adjust
any related type definitions where viewSubmit is declared.
In `@app/containers/UIKit/Section.tsx`:
- Line 34: The key generation uses (field as any).type which throws away type
safety; introduce a narrow type guard (e.g., isFieldWithType or hasTypeProperty)
that checks "type" in field and narrows its type, then replace the cast with a
safe expression like `${isFieldWithType(field) ? field.type : 'field'}-${index}`
when setting the key in Section.tsx so you avoid using any and keep proper
typing for field.
In `@app/lib/methods/actions.ts`:
- Line 48: Remove the redundant type assertion on the destructuring of data: the
variable declaration let { viewId } = data as { viewId?: string }; should simply
destructure from data (which is already typed as THandledServerPayload) so
delete the "as { viewId?: string }" cast; update the line that references
viewId/data in actions.ts (the let { viewId } ... statement) to rely on the
existing THandledServerPayload type and ensure no other casts are added.
🪄 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: cc192c8a-b003-493e-be16-dabde538d1fd
⛔ Files ignored due to path filters (6)
android/app/src/main/assets/fonts/custom.ttfis excluded by!**/*.ttfapp/containers/UIKit/__snapshots__/UiKitMessage.test.tsx.snapis excluded by!**/*.snapapp/containers/UIKit/__snapshots__/UiKitModal.test.tsx.snapis excluded by!**/*.snapapp/containers/markdown/__snapshots__/Markdown.test.tsx.snapis excluded by!**/*.snapios/custom.ttfis excluded by!**/*.ttfyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (28)
app/containers/CustomIcon/mappedIcons.jsapp/containers/CustomIcon/selection.jsonapp/containers/UIKit/Actions.tsxapp/containers/UIKit/Context.tsxapp/containers/UIKit/Icon.tsxapp/containers/UIKit/IconButton.tsxapp/containers/UIKit/Image.tsxapp/containers/UIKit/InfoCard.tsxapp/containers/UIKit/Input.tsxapp/containers/UIKit/Section.tsxapp/containers/UIKit/UiKitMessage.stories.tsxapp/containers/UIKit/VideoConferenceBlock/components/VideoConferenceBaseContainer.tsxapp/containers/UIKit/index.tsxapp/containers/UIKit/interactionAdapters.tsapp/containers/UIKit/interfaces.tsapp/containers/markdown/Markdown.stories.tsxapp/containers/markdown/components/Plain.tsxapp/containers/markdown/components/inline/Link.tsxapp/containers/markdown/components/mentions/AtMention.tsxapp/containers/markdown/components/mentions/Hashtag.tsxapp/containers/markdown/contexts/MarkdownContext.tsapp/containers/markdown/index.tsxapp/i18n/locales/en.jsonapp/lib/methods/actions.tsapp/views/ModalBlockView.tsxjest.config.jspackage.jsontsconfig.json
📜 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: Agent
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-03-15T13:55:42.038Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6911
File: app/containers/markdown/Markdown.stories.tsx:104-104
Timestamp: 2026-03-15T13:55:42.038Z
Learning: In Rocket.Chat React Native, the markdown parser requires a space between the underscore wrapping italic text and a mention sigil (_ mention _ instead of _mention_). Ensure stories and tests that include italic-wrapped mentions follow this form to guarantee proper parsing. Specifically, for files like app/containers/markdown/Markdown.stories.tsx, and any test/content strings that exercise italic-mentions, use the pattern _ mention _ (with spaces) to prevent the mention from being treated as plain text. Validate any test strings or story content accordingly.
Applied to files:
app/containers/markdown/Markdown.stories.tsxapp/containers/UIKit/UiKitMessage.stories.tsx
📚 Learning: 2026-03-15T13:55:47.880Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6911
File: app/containers/markdown/Markdown.stories.tsx:104-104
Timestamp: 2026-03-15T13:55:47.880Z
Learning: In Rocket.Chat React Native's markdown parser, italic delimiters must have a space between the underscore and an `@` or `#` mention sigil for the mention to be parsed correctly. Using `_mention_` (no space) causes the mention to fall through as plain text; the correct form is `_ mention _`. This applies to story files like `app/containers/markdown/Markdown.stories.tsx` and any test/content strings that test italic-wrapped mentions.
Applied to files:
app/containers/markdown/components/mentions/AtMention.tsxapp/containers/markdown/components/Plain.tsxapp/containers/markdown/index.tsxapp/containers/markdown/components/mentions/Hashtag.tsx
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.
Applied to files:
app/containers/UIKit/UiKitMessage.stories.tsxapp/containers/markdown/components/Plain.tsxapp/containers/markdown/index.tsxapp/containers/markdown/contexts/MarkdownContext.tsapp/containers/UIKit/InfoCard.tsxapp/containers/markdown/components/mentions/Hashtag.tsxapp/views/ModalBlockView.tsx
📚 Learning: 2026-02-05T13:55:00.974Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:00.974Z
Learning: In this repository, the dependency on react-native-image-crop-picker should reference the RocketChat fork (RocketChat/react-native-image-crop-picker) with explicit commit pins, not the upstream ivpusic/react-native-image-crop-picker. Update package.json dependencies (and any lockfile) to point to the fork URL and a specific commit, ensuring edge-to-edge Android fixes are included. This pattern should apply to all package.json files in the repo that declare this dependency.
Applied to files:
package.json
🪛 Biome (2.4.7)
app/lib/methods/actions.ts
[error] 92-145: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
🔇 Additional comments (35)
tsconfig.json (1)
7-7: Formatting-only tsconfig update looks good.These edits are non-behavioral (comma/comment reflow only); compiler option values are unchanged.
Also applies to: 38-39
app/containers/markdown/components/inline/Link.tsx (1)
22-22: Good context-driven style propagation for links.The
textStylewiring is clean and keeps existing link interaction behavior unchanged.Also applies to: 51-54
app/containers/markdown/Markdown.stories.tsx (1)
172-188: Nice addition of a focusedtextStylestory.This gives practical coverage across emphasis, mentions, hashtags, quotes, and links for the new context styling path.
app/containers/markdown/components/Plain.tsx (1)
1-1: Plain text styling integration looks solid.Using
MarkdownContexthere keeps style propagation consistent with the rest of the markdown subtree.Also applies to: 7-7, 15-17
app/containers/markdown/index.tsx (1)
2-2:textStyleAPI threading is complete and consistent.The prop typing, component signature, and context provider updates are in sync.
Also applies to: 35-35, 48-50, 72-73
app/containers/markdown/contexts/MarkdownContext.ts (1)
2-2: Context shape update is clean.Adding
textStyleto both interface and default state keeps consumers type-safe and predictable.Also applies to: 14-14, 22-23
app/containers/markdown/components/mentions/AtMention.tsx (1)
1-2: At-mention styling refactor is well executed.The switch to context-driven
textStylekeeps behavior intact while simplifying the component contract.Also applies to: 11-11, 21-23, 31-31, 78-80, 87-89
app/containers/markdown/components/mentions/Hashtag.tsx (1)
1-2: Hashtag renderer now consistently follows markdown context styling.Good alignment with the broader
textStylepropagation across markdown inline components.Also applies to: 17-17, 25-27, 60-60, 70-72
app/containers/CustomIcon/mappedIcons.js (1)
164-166: LGTM! Icon mappings correctly support new voice call states.The icon changes align with the PR objectives:
phone-off(59805) replaces the removedphone-endfor call ended statephone-question-mark(59835) added for "not answered" statephone-issue(59879) reassigned for call failure stateThis matches the PR description screenshots showing distinct icons for ended, transferred, not answered, and failed call states.
jest.config.js (1)
4-4: LGTM!Adding
@rocket.chat/ui-kitto the transform allowlist is necessary for Jest to properly transform ES modules from this package during test execution.app/i18n/locales/en.json (1)
110-118: LGTM!The localization strings are well-structured with consistent naming (
_boldsuffix) and properly ordered. The markdown bold syntax (*text*) aligns with how these strings will be rendered in the info cards.app/containers/UIKit/Image.tsx (1)
36-47: LGTM!Clean refactor to a single props object pattern. The change improves consistency with React conventions and aligns with the broader UI Kit API updates.
app/containers/UIKit/interactionAdapters.ts (1)
1-105: Well-structured interaction adapter module.The implementation provides clean separation of concerns with proper validation, descriptive error messages, and type-safe conversions. The calling code in
actions.tscorrectly handles the nullable return fromtoServerModalInteractionType.app/containers/UIKit/UiKitMessage.stories.tsx (2)
23-54: LGTM! Well-structured decorator with proper context nesting.The decorator correctly provides both
ResponsiveLayoutContextandMessageContextwith sensible default values for story rendering. The nested provider pattern ensures components have access to all required contexts.
536-723: Comprehensive InfoCard stories covering all voice call states.The stories effectively demonstrate:
- All four call states (ended, transferred, not answered, failed)
- Icon framing variants and semantic colors (default, warning, danger)
- i18n integration with localization keys
- Edge case handling for long text content
This aligns well with the PR description screenshots and VMUX-24 objectives.
app/containers/UIKit/VideoConferenceBlock/components/VideoConferenceBaseContainer.tsx (1)
18-24: LGTM!The icon name change from
phone-endtophone-offaligns with the updated icon mappings. The visual appearance is preserved since both map to the same glyph code (59805).package.json (1)
53-53: The@rocket.chat/ui-kitversion 0.39.0 is valid and exists on npm.This is a significant version jump from 0.31.19 to 0.39.0. The caret specifier is appropriate for 0.x semver ranges, allowing patch and minor updates within 0.39.x.
app/containers/UIKit/Section.tsx (1)
28-28: Good alignment with the new renderer signature.This update is consistent with the parser-call refactor across UIKit and keeps the call site clean.
app/containers/UIKit/Input.tsx (1)
41-41: Looks good.The
renderInputscall is correctly aligned with the updated two-argument usage.app/containers/UIKit/Context.tsx (1)
16-22: Solid update for list identity and renderer call consistency.The keyed fragment wrapping plus two-argument
renderContextcall is clean and safe.app/containers/UIKit/Actions.tsx (1)
33-33: No concerns here.This call-site update is consistent with the parser signature changes and preserves behavior.
app/views/ModalBlockView.tsx (1)
167-168: Nice simplification.The direct comparison is clearer than a single-value
includescheck, and theTModalActiontyping improves readability.app/containers/UIKit/Icon.tsx (1)
22-59: Good defensive icon resolution and theming.Alias fallback and framed/unframed rendering paths look solid.
app/containers/UIKit/IconButton.tsx (1)
69-69: No change required. The type system guaranteeselement.iconis always present inIIconButton, making line 69 safe. The optional chaining at line 42 (element.icon?.icon) is defensive access to a nested property within the required icon object, not evidence that icon can be missing.> Likely an incorrect or invalid review comment.app/lib/methods/actions.ts (2)
35-89: LGTM!The function structure is well-organized with proper guard clauses for
triggerIdandviewIdvalidation, and clear branching logic for different modal action types.
26-33: LGTM!The type definitions are well-structured, using
Extractto derive specific interaction types fromServerInteractionand providing a flexible payload type with requiredtriggerIdand optional fields.app/containers/UIKit/index.tsx (4)
134-143: Consistent fallback pattern foractionId.The
actionId: element.actionId || ''pattern is applied consistently across multiple methods. PeruseBlockContext(app/containers/UIKit/utils.ts), an empty string will result invalues['']being undefined (falling back toinitialValue) and skip error lookup due to theactionId &&guard. This works but is subtle.
114-120: LGTM!The new
iconandicon_buttonmethods follow the established pattern for element rendering, cleanly delegating to their respective components.
91-97: LGTM!The i18n integration in
mrkdwnis clean—checks forelement.i18nand falls back toelement.textwhen not present.
166-168: LGTM!The
info_cardmethod follows the established pattern for block rendering, passing the parser for nested element rendering.app/containers/UIKit/interfaces.ts (5)
17-38: LGTM!The refactor from enums to const objects with derived union types is a good practice—it provides better tree-shaking, preserves the string literal values at runtime, and maintains full type safety.
266-273: LGTM!
IIconandIIconButtonare well-composed using TypeScript utility types (Omit) to properly extend the library types while replacing theiconfield with the correct type.
71-81: LGTM!The
ITexttype properly supports both plain text and i18n-enabled text with interpolation arguments.
275-281: LGTM!
IInfoCardandIInfoCardRoware well-defined, properly deriving fromInfoCardBlockwhile extending with app-specific fields (appId,blockId,parser).
153-173: LGTM!The
IParserinterface is comprehensive, supporting the newiconandicon_buttonmethods while maintaining flexibility with optional parameters for nested rendering.
diegolmello
force-pushed
the
feat.voip-blocks
branch
from
3859bce to
3bbf2a3
Compare
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
force-pushed
the
feat.voip-blocks
branch
from
6b23ec7 to
76cd954
Compare
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/i18n/locales/ja.json (1)
77-77: Polish the Japanese phrasing for “not answered.”
"*通話が応答されませんでした*"reads a bit unnatural in Japanese. A more natural phrasing would be*通話に応答がありませんでした*.Suggested wording tweak
- "Call_not_answered_bold": "*通話が応答されませんでした*", + "Call_not_answered_bold": "*通話に応答がありませんでした*",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/i18n/locales/ja.json` at line 77, Replace the value for the translation key "Call_not_answered_bold" so the Japanese reads more naturally; change "*通話が応答されませんでした*" to "*通話に応答がありませんでした*" by updating the string assigned to the "Call_not_answered_bold" key in the ja.json locale.app/containers/UIKit/InfoCard.tsx (1)
39-52: Consider avoidingas anycasts for better type safety.The
as anycasts bypass TypeScript checking for themrkdwnandplain_textelements. If theIInfoCardRow['elements']union type doesn't fully align withMarkdownandPlainText, consider narrowing the type properly or updating the interface to ensure type compatibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/UIKit/InfoCard.tsx` around lines 39 - 52, The code is using unsafe casts (as any) for element when calling parser.mrkdwn and parser.plain_text; replace these with proper type narrowing or updated interfaces so the element is typed as Markdown or PlainText before calling parser.mrkdwn/parser.plain_text. Add type guards (e.g., isMarkdownElement(element) / isPlainTextElement) or refine IInfoCardRow['elements'] to include discriminated unions that match the parser input, then call parser.mrkdwn(element, BlockContext.NONE) and parser.plain_text(element, BlockContext.NONE) without using as any.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/i18n/locales/it.json`:
- Line 83: The localization key "Call_not_answered_bold" uses the phrase "non
risposta" which is unnatural in Italian UI copy; update its value to use
idiomatic Italian by replacing "*Chiamata vocale non risposta*" with "*Chiamata
vocale senza risposta*" so the string reads naturally and consistently in the
UI.
---
Nitpick comments:
In `@app/containers/UIKit/InfoCard.tsx`:
- Around line 39-52: The code is using unsafe casts (as any) for element when
calling parser.mrkdwn and parser.plain_text; replace these with proper type
narrowing or updated interfaces so the element is typed as Markdown or PlainText
before calling parser.mrkdwn/parser.plain_text. Add type guards (e.g.,
isMarkdownElement(element) / isPlainTextElement) or refine
IInfoCardRow['elements'] to include discriminated unions that match the parser
input, then call parser.mrkdwn(element, BlockContext.NONE) and
parser.plain_text(element, BlockContext.NONE) without using as any.
In `@app/i18n/locales/ja.json`:
- Line 77: Replace the value for the translation key "Call_not_answered_bold" so
the Japanese reads more naturally; change "*通話が応答されませんでした*" to
"*通話に応答がありませんでした*" by updating the string assigned to the
"Call_not_answered_bold" key in the ja.json locale.
🪄 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: d01e7642-c360-4ffa-a39a-bd40360d5bcb
⛔ Files ignored due to path filters (1)
app/containers/UIKit/__snapshots__/UiKitMessage.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (27)
app/containers/UIKit/InfoCard.tsxapp/containers/UIKit/UiKitMessage.stories.tsxapp/i18n/locales/ar.jsonapp/i18n/locales/bn-IN.jsonapp/i18n/locales/cs.jsonapp/i18n/locales/de.jsonapp/i18n/locales/es.jsonapp/i18n/locales/fi.jsonapp/i18n/locales/fr.jsonapp/i18n/locales/hi-IN.jsonapp/i18n/locales/hu.jsonapp/i18n/locales/it.jsonapp/i18n/locales/ja.jsonapp/i18n/locales/nl.jsonapp/i18n/locales/nn.jsonapp/i18n/locales/no.jsonapp/i18n/locales/pt-BR.jsonapp/i18n/locales/pt-PT.jsonapp/i18n/locales/ru.jsonapp/i18n/locales/sl-SI.jsonapp/i18n/locales/sv.jsonapp/i18n/locales/ta-IN.jsonapp/i18n/locales/te-IN.jsonapp/i18n/locales/tr.jsonapp/i18n/locales/zh-CN.jsonapp/i18n/locales/zh-TW.jsontsconfig.json
✅ Files skipped from review due to trivial changes (1)
- app/i18n/locales/no.json
🚧 Files skipped from review as they are similar to previous changes (1)
- app/containers/UIKit/UiKitMessage.stories.tsx
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.
Applied to files:
app/containers/UIKit/InfoCard.tsx
🔇 Additional comments (26)
tsconfig.json (1)
7-7: LGTM!These are minor formatting changes standardizing comma placement after inline comments. No semantic or behavioral changes to the TypeScript configuration.
Also applies to: 38-39
app/i18n/locales/nn.json (1)
56-58: Localization additions are consistent and good to merge.Line 56, Line 57, Line 58, and Line 61 correctly add the new
Call_<status>_boldkeys with the expected bold markdown format and consistent key naming.Also applies to: 61-61
app/i18n/locales/hi-IN.json (1)
104-105: Looks good — bold voice-call status translations are consistent.These new Hindi
*_boldkeys align with the expected naming and markdown emphasis pattern for voice message blocks.Also applies to: 107-107, 112-112
app/i18n/locales/ru.json (1)
91-95: Looks good: new bold call-status translations are consistent and ready to ship.The added Russian strings are clear, correctly formatted with markdown emphasis, and aligned with the voice message block statuses.
app/i18n/locales/cs.json (1)
110-111: LGTM!The new Czech translations for voice call message blocks are well-implemented:
- Keys follow the
_boldsuffix convention- Alphabetical ordering is maintained
- Bold markdown markers (
*...*) are consistent- "Hlasové volání" (Voice call) appropriately distinguishes these voice-specific message blocks from the existing general "Hovor" (Call) translations
Also applies to: 113-113, 118-118
app/i18n/locales/pt-PT.json (1)
74-78: LGTM!The new translation keys for voice call status messages are correctly added:
- Translations are grammatically correct in Portuguese.
- Keys maintain alphabetical ordering within the
Call_*group.- Bold markdown formatting (
*text*) is consistent with the_boldkey suffix convention.app/i18n/locales/hu.json (1)
104-105: LGTM!The four new Hungarian localization strings for voice call status blocks are correctly added:
- Keys follow the established
_boldnaming convention- Markdown bold syntax (
*...*) is consistently applied- Translations are grammatically correct in Hungarian
- Alphabetical ordering within the JSON is maintained
- The use of "Hanghívás" (voice call) appropriately differentiates these VoIP-specific messages from the generic "Hívás" (call) used elsewhere
Also applies to: 107-107, 112-112
app/i18n/locales/sv.json (1)
93-97: Looks good: bold Swedish call-status messages are consistent and correctly formatted.Line 93, Line 94, Line 95, and Line 97 follow the new
*_boldi18n key pattern and keep valid Markdown emphasis formatting for voice-call block labels.app/i18n/locales/de.json (1)
101-103: LGTM!The new German localization keys for voice call status messages are correctly translated and consistently formatted. The translations accurately convey the voice call events (ended, failed, not answered, transferred), and the Markdown bold syntax with asterisks aligns with the expected rendering for message blocks.
Also applies to: 107-107
app/i18n/locales/sl-SI.json (1)
85-89: LGTM!The four new voice call status translation keys are correctly added:
- Alphabetical ordering is maintained within the
Call_*key group.- Bold markdown formatting (
*text*) is consistent across all entries.- Slovenian translations align with the intended semantics (ended, failed, not answered, transferred).
app/i18n/locales/zh-TW.json (1)
75-77: Bold call-status zh-TW translations look correct and consistent.The new
*_boldentries are clear, idiomatic, and aligned with the voice message block status intent.Also applies to: 79-79
app/i18n/locales/nl.json (1)
81-85: LGTM!The new Dutch translation keys for voice call status messages are well-implemented:
- Translations are accurate ("Spraakgesprek" appropriately conveys "voice call")
- Markdown bold formatting (
*text*) is correct for use with themrkdwnhandler- Keys are properly ordered alphabetically within the
Call_prefix group- Key naming with
_boldsuffix follows the convention for markdown-formatted stringsapp/i18n/locales/tr.json (1)
75-79: LGTM!The new Turkish locale keys for voice call statuses are correctly added with proper alphabetical ordering, valid JSON syntax, and accurate translations. The bold Markdown formatting (
*text*) is consistent with the_boldkey suffix convention used across other locale files in this PR.app/i18n/locales/zh-CN.json (1)
75-77: Looks good — the new call-status bold translations are consistent and correctly added.The new keys are well-formed, semantically correct for voice call message blocks, and the JSON changes are valid.
Also applies to: 79-79
app/i18n/locales/fr.json (1)
81-85: LGTM!The new French translation keys for voice call status messages are accurate and well-integrated:
- Translations are grammatically correct and idiomatic
- Bold formatting with markdown asterisks is consistent
- Alphabetical ordering within the JSON file is maintained
app/i18n/locales/te-IN.json (1)
104-105: LGTM!The new Telugu locale entries for voice call message blocks are well-formed:
- Keys follow the established
Call_<status>_boldnaming convention- Markdown bold syntax (
*...*) is consistently applied- Alphabetical ordering is maintained within the JSON structure
- Telugu translations appropriately use "వాయిస్ కాల్" (transliteration of "Voice call") which is a reasonable approach for technical terms
Also applies to: 107-107, 112-112
app/i18n/locales/ta-IN.json (1)
104-105: Looks good — new bold call-status locale keys are correctly added.These additions are well-scoped, consistent with the existing i18n key pattern, and valid JSON for the new voice message block statuses.
Also applies to: 107-107, 112-112
app/i18n/locales/ja.json (1)
75-76: Good addition of bold call-status localization keys.These new keys align with the message-block status use case and keep JSON structure valid.
Also applies to: 79-79
app/i18n/locales/fi.json (1)
93-95: Bold call-status locale entries look good.These additions are consistent with the existing
Call_*localization namespace and correctly formatted for emphasized rendering in message blocks.Also applies to: 97-97
app/i18n/locales/bn-IN.json (1)
104-105: LGTM!The new bold call status translations are correctly added:
- Proper Markdown bold formatting with asterisks
- Keys maintain alphabetical ordering within the file
- Bengali translations are appropriate for voice call contexts
- Pattern aligns with the cross-locale additions mentioned in the PR summary
Also applies to: 107-107, 112-112
app/i18n/locales/pt-BR.json (1)
108-109: Good locale extension for voice-call status blocks.Line 108, Line 109, Line 111, and Line 116 add the expected
_boldcall-status keys with consistent naming and proper pt-BR wording for the new message-block states.Also applies to: 111-111, 116-116
app/i18n/locales/ar.json (1)
76-78: Arabic bold call-status keys are correctly added and consistent.Line 76 to Line 78 and Line 80 follow the same key pattern as other locales and provide appropriate Arabic strings for the new voice message block statuses.
Also applies to: 80-80
app/i18n/locales/es.json (1)
71-73: Spanish voice-call bold status translations look good.Line 71 to Line 73 and Line 75 correctly introduce the new
_boldvariants with clear, natural Spanish phrasing and consistent formatting.Also applies to: 75-75
app/i18n/locales/it.json (1)
81-82: Nice addition of bold call-status keys.These new entries are consistent with the message-block objective and formatted correctly for markdown emphasis.
Also applies to: 85-85
app/containers/UIKit/InfoCard.tsx (2)
58-74: Acknowledged:renderRowActionis stubbed pending call history implementation.The TODO comment clearly documents the intentional temporary disabling. This was previously flagged and appears to be a deliberate deferral.
76-98: LGTM!The component structure is clean, properly uses theme colors, and correctly maps row elements through the parser. The key generation with
blockIdprefix and index is acceptable for this use case where rows represent static data that won't be reordered.
diegolmello
force-pushed
the
feat.voip-blocks
branch
from
449b701 to
92e1f0e
Compare
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/lib/methods/actions.ts`:
- Around line 134-137: The branch that returns ModalActions.CLOSE after calling
toServerModalInteractionType currently skips handlePayloadUserInteraction and
leaves a stale entry in triggersId; before returning resolve(ModalActions.CLOSE)
ensure you invalidate/remove the trigger entry (the same deletion logic used in
handlePayloadUserInteraction) for the current triggerId so triggersId is cleaned
up on unmapped or modal.close paths; locate the modalType check around
toServerModalInteractionType and ModalActions.CLOSE and perform the triggersId
deletion there prior to resolving.
- Around line 98-99: The function triggerAction currently wraps its logic in new
Promise(async (resolve, reject) => ...) which triggers
lint/suspicious/noAsyncPromiseExecutor and discards error context via bare
reject(); change triggerAction to be an async function (remove the Promise
constructor), inline the body directly, replace all resolve(value) calls with
return value, replace reject(error) (and the bare reject()) with throw error (or
throw new Error(...)) to preserve stack/error info, and keep usages of
generateTriggerId and any other helper calls unchanged so behavior remains the
same.
- Line 100: Change the payload assignment to use nullish coalescing so explicit
falsy values are preserved: replace the use of the logical OR between
rest.payload and rest.value with the nullish coalescing operator (i.e., use
rest.payload ?? rest.value) in the payload assignment; this aligns behavior with
toUserInteraction and ensures values like '', 0, and false are not overwritten.
🪄 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: 04892f22-30b4-4e56-825a-cfe238098509
📒 Files selected for processing (3)
app/i18n/locales/it.jsonapp/lib/methods/actions.tstsconfig.json
🚧 Files skipped from review as they are similar to previous changes (1)
- app/i18n/locales/it.json
📜 Review details
🧰 Additional context used
🪛 Biome (2.4.7)
app/lib/methods/actions.ts
[error] 98-151: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
🔇 Additional comments (2)
tsconfig.json (2)
7-7: No functional impact ontargetconfig.Line 7 is a formatting-only adjustment; compiler behavior remains unchanged.
38-39:noUnused*checks are preserved correctly.Lines 38-39 keep the same enforcement settings; this is style-only cleanup.
diegolmello
force-pushed
the
feat.voip-blocks
branch
from
8276231 to
b0a6443
Compare
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/containers/markdown/Markdown.textStyle.test.tsx (2)
14-43: Optional: Consider splitting into two focused test cases.The current test validates two distinct behaviors: style propagation and link interaction. Splitting these into separate test cases would improve test isolation and make failures easier to diagnose.
🧪 Suggested test split
describe('Markdown textStyle integration', () => { const renderMarkdown = (props = {}) => render( <Markdown msg='hello [my link](https://rocket.chat) `@rocket.cat` `#general`' textStyle={{ fontSize: 17 }} mentions={[{ _id: 'u1', username: 'rocket.cat', name: 'Rocket Cat', type: 'user' }]} username='another.user' channels={[{ _id: 'r1', name: 'general' }]} {...props} /> ); it('propagates textStyle to plain text, link, mention, and hashtag', () => { const { getByLabelText, getByText } = renderMarkdown(); expect(getByLabelText('hello ').props.style).toEqual(expect.arrayContaining([expect.objectContaining({ fontSize: 17 })])); expect(getByText('my link').props.style).toEqual(expect.arrayContaining([expect.objectContaining({ fontSize: 17 })])); expect(getByText('@rocket.cat').props.style).toEqual(expect.arrayContaining([expect.objectContaining({ fontSize: 17 })])); expect(getByText('#general').props.style).toEqual(expect.arrayContaining([expect.objectContaining({ fontSize: 17 })])); }); it('preserves link interaction when textStyle is applied', () => { const onLinkPress = jest.fn(); const { getByText } = renderMarkdown({ onLinkPress }); fireEvent.press(getByText('my link')); expect(onLinkPress).toHaveBeenCalledWith('https://rocket.chat'); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/markdown/Markdown.textStyle.test.tsx` around lines 14 - 43, The test mixes style-propagation assertions and link-interaction assertions in one spec; split them into two focused tests: (1) a "propagates textStyle to plain text, link, mention, and hashtag" test that renders <Markdown> with textStyle and asserts styles for nodes (use getByLabelText/getByText as currently used), and (2) a "preserves link interaction when textStyle is applied" test that renders <Markdown> with an onLinkPress mock and only asserts that pressing the link calls onLinkPress with 'https://rocket.chat'. Factor out a small renderMarkdown helper to DRY the shared <Markdown> props (msg, textStyle, mentions, username, channels) used by both tests.
6-12: Consider documenting mock behavior in a comment.The
useUserPreferencesmock returning[true]enables both@and#prefixes (affectingmentionsWithAtSymbolandroomsWithHashTagSymbol). A brief inline comment would clarify this intentional behavior for future maintainers.📝 Suggested documentation
-jest.mock('../../lib/methods/userPreferences', () => ({ - useUserPreferences: jest.fn(() => [true]) -})); +// Returns [true] to enable both @ and # prefixes for mentions and hashtags +jest.mock('../../lib/methods/userPreferences', () => ({ + useUserPreferences: jest.fn(() => [true]) +}));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/markdown/Markdown.textStyle.test.tsx` around lines 6 - 12, Add a short inline comment above the jest.mock for useUserPreferences explaining that the mock returns [true], which intentionally enables both mentionsWithAtSymbol and roomsWithHashTagSymbol (i.e., both `@` and `#` prefixes are active) so future maintainers understand the test setup; reference the mock function name useUserPreferences and the affected flags mentionsWithAtSymbol and roomsWithHashTagSymbol in the comment.app/lib/methods/triggerActions.test.ts (1)
61-75: Add theerrorskeep-open case here.
triggerSubmitView()should also avoidNavigation.back()whentriggerAction()returnsModalActions.ERRORS; that’s the validation-error path for the modal.🧪 Suggested test
it('does not go back for modal.open', async () => { mockedTriggerAction.mockResolvedValueOnce(ModalActions.OPEN); await triggerSubmitView(submitInput as any); expect(mockedBack).not.toHaveBeenCalled(); }); + + it('does not go back for errors', async () => { + mockedTriggerAction.mockResolvedValueOnce(ModalActions.ERRORS); + + await triggerSubmitView(submitInput as any); + + expect(mockedBack).not.toHaveBeenCalled(); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/triggerActions.test.ts` around lines 61 - 75, Add a test case to ensure triggerSubmitView does not call Navigation.back when triggerAction returns the validation-errors variant: mock mockedTriggerAction.mockResolvedValueOnce(ModalActions.ERRORS), call await triggerSubmitView(submitInput as any) and assert expect(mockedBack).not.toHaveBeenCalled(); mirror the existing modal.update/modal.open tests (use ModalActions.ERRORS, mockedTriggerAction, mockedBack, and triggerSubmitView).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/lib/methods/actions.ts`:
- Around line 145-155: Replace the current blind result.json() usage with a
two-step check: read the raw response text first (e.g., await result.text()),
return ModalActions.CLOSE only when the body is actually empty/blank, otherwise
parse the text to JSON and then call toServerModalInteractionType on the parsed
type; if JSON parsing fails or toServerModalInteractionType returns null/unknown
(i.e., not one of the validated types), throw or propagate an error instead of
returning CLOSE so handlePayloadUserInteraction is not called for
malformed/legacy responses—update the block around
toServerModalInteractionType/handlePayloadUserInteraction to implement this flow
and ensure parse errors and unknown types fail fast.
---
Nitpick comments:
In `@app/containers/markdown/Markdown.textStyle.test.tsx`:
- Around line 14-43: The test mixes style-propagation assertions and
link-interaction assertions in one spec; split them into two focused tests: (1)
a "propagates textStyle to plain text, link, mention, and hashtag" test that
renders <Markdown> with textStyle and asserts styles for nodes (use
getByLabelText/getByText as currently used), and (2) a "preserves link
interaction when textStyle is applied" test that renders <Markdown> with an
onLinkPress mock and only asserts that pressing the link calls onLinkPress with
'https://rocket.chat'. Factor out a small renderMarkdown helper to DRY the
shared <Markdown> props (msg, textStyle, mentions, username, channels) used by
both tests.
- Around line 6-12: Add a short inline comment above the jest.mock for
useUserPreferences explaining that the mock returns [true], which intentionally
enables both mentionsWithAtSymbol and roomsWithHashTagSymbol (i.e., both `@` and
`#` prefixes are active) so future maintainers understand the test setup;
reference the mock function name useUserPreferences and the affected flags
mentionsWithAtSymbol and roomsWithHashTagSymbol in the comment.
In `@app/lib/methods/triggerActions.test.ts`:
- Around line 61-75: Add a test case to ensure triggerSubmitView does not call
Navigation.back when triggerAction returns the validation-errors variant: mock
mockedTriggerAction.mockResolvedValueOnce(ModalActions.ERRORS), call await
triggerSubmitView(submitInput as any) and assert
expect(mockedBack).not.toHaveBeenCalled(); mirror the existing
modal.update/modal.open tests (use ModalActions.ERRORS, mockedTriggerAction,
mockedBack, and triggerSubmitView).
🪄 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: 7107b26b-405e-4e0a-bbdf-61c975fe714f
📒 Files selected for processing (8)
app/containers/UIKit/Icon.test.tsxapp/containers/UIKit/InfoCard.test.tsxapp/containers/UIKit/interactionAdapters.test.tsapp/containers/markdown/Markdown.textStyle.test.tsxapp/lib/methods/actions.test.tsapp/lib/methods/actions.tsapp/lib/methods/triggerActions.test.tstsconfig.json
🚧 Files skipped from review as they are similar to previous changes (1)
- tsconfig.json
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-03-15T13:55:47.880Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6911
File: app/containers/markdown/Markdown.stories.tsx:104-104
Timestamp: 2026-03-15T13:55:47.880Z
Learning: In Rocket.Chat React Native's markdown parser, italic delimiters must have a space between the underscore and an `@` or `#` mention sigil for the mention to be parsed correctly. Using `_mention_` (no space) causes the mention to fall through as plain text; the correct form is `_ mention _`. This applies to story files like `app/containers/markdown/Markdown.stories.tsx` and any test/content strings that test italic-wrapped mentions.
Applied to files:
app/containers/markdown/Markdown.textStyle.test.tsx
📚 Learning: 2026-03-05T14:28:17.265Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6997
File: .maestro/tests/room/message-markdown-click.yaml:28-39
Timestamp: 2026-03-05T14:28:17.265Z
Learning: In the Rocket.Chat React Native repository's Maestro test files (.maestro/tests/), regex selectors for partial/contains text matching should use the `'.*keyword.*'` pattern (both leading and trailing `.*`). The pattern `'.*keyword*.'` is incorrect — it parses as zero-or-more of the last character of the keyword followed by one required character, and will fail to match elements where the keyword appears at the end of the element's text. This applies to all Maestro YAML selector fields (text, id) in the codebase.
Applied to files:
app/containers/markdown/Markdown.textStyle.test.tsx
🔇 Additional comments (3)
app/containers/markdown/Markdown.textStyle.test.tsx (2)
1-44: Well-structured integration test for textStyle propagation.The test effectively covers textStyle propagation to all relevant Markdown child components (plain text, links, mentions, hashtags) and verifies that link press functionality remains intact. The mocks are appropriate, and the assertions using
expect.arrayContainingcorrectly verify the presence of the style property without being brittle about style array ordering.
19-28: ThemeProvider is not required for this test.The
useTheme()hook usesReact.useContext(ThemeContext), andThemeContextis initialized with a default value ({ theme: 'light', colors: colors.light }). When no Provider is in the component tree, the hook returns the default value. The test will work without any additional setup.> Likely an incorrect or invalid review comment.app/lib/methods/actions.test.ts (1)
81-217: Nice branch coverage ontriggerAction().This locks in OPEN/UPDATE/ERRORS/CLOSE handling plus trigger-id invalidation, which are the risky paths in this refactor.
diegolmello
force-pushed
the
feat.voip-blocks
branch
from
dd0c2c9 to
adae8b9
Compare
diegolmello
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/lib/methods/actions.test.ts (1)
186-195: Add a whitespace-only body test to pin CLOSE semantics.Runtime handles blank text via
trim(). Adding' 'here would lock that behavior and prevent regressions.🧪 Optional test addition
+ it('returns modal.close for whitespace-only response body with ok status', async () => { + mockedFetch.mockResolvedValueOnce({ + ok: true, + text: () => Promise.resolve(' ') + } as Response); + + const result = await triggerAction(actionInput); + + expect(result).toBe(ModalActions.CLOSE); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/actions.test.ts` around lines 186 - 195, Add a new test case alongside the existing one that verifies a whitespace-only response body still yields ModalActions.CLOSE: update the mockedFetch.resolve to return ok: true and text: () => Promise.resolve(' ') (three spaces) and assert that await triggerAction(actionInput) === ModalActions.CLOSE; this ensures the trim()-based blank-body handling in triggerAction is pinned (referencing mockedFetch, triggerAction, and ModalActions.CLOSE).app/lib/methods/actions.ts (1)
60-79: Deduplicate UPDATE/ERRORS event emission payload assembly.Both branches build and emit the same shape; extracting a small helper would reduce drift risk.
♻️ Optional refactor
+ const emitModalEvent = (eventType: ModalActions.UPDATE | ModalActions.ERRORS): TModalAction => { + EventEmitter.emit(viewId, { + ...data, + appId: payloadAppId, + type: eventType, + triggerId, + viewId + } as any); + return eventType; + }; + - if (type === ModalActions.ERRORS) { - EventEmitter.emit(viewId, { - ...data, - appId: payloadAppId, - type, - triggerId, - viewId - } as any); - return ModalActions.ERRORS; - } - - if (type === ModalActions.UPDATE) { - EventEmitter.emit(viewId, { - ...data, - appId: payloadAppId, - type, - triggerId, - viewId - } as any); - return ModalActions.UPDATE; - } + if (type === ModalActions.ERRORS || type === ModalActions.UPDATE) { + return emitModalEvent(type); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/actions.ts` around lines 60 - 79, The code duplicates building and emitting the same payload in the ModalActions.ERRORS and ModalActions.UPDATE branches; extract a small helper (e.g., emitModalEvent or buildAndEmitPayload) that accepts (type, viewId, data, payloadAppId, triggerId) and constructs the object { ...data, appId: payloadAppId, type, triggerId, viewId } then calls EventEmitter.emit(viewId, payload) and returns the type; replace both branches to call this helper to remove duplication and keep behavior identical for ModalActions.ERRORS and ModalActions.UPDATE.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/lib/methods/actions.test.ts`:
- Around line 186-195: Add a new test case alongside the existing one that
verifies a whitespace-only response body still yields ModalActions.CLOSE: update
the mockedFetch.resolve to return ok: true and text: () => Promise.resolve('
') (three spaces) and assert that await triggerAction(actionInput) ===
ModalActions.CLOSE; this ensures the trim()-based blank-body handling in
triggerAction is pinned (referencing mockedFetch, triggerAction, and
ModalActions.CLOSE).
In `@app/lib/methods/actions.ts`:
- Around line 60-79: The code duplicates building and emitting the same payload
in the ModalActions.ERRORS and ModalActions.UPDATE branches; extract a small
helper (e.g., emitModalEvent or buildAndEmitPayload) that accepts (type, viewId,
data, payloadAppId, triggerId) and constructs the object { ...data, appId:
payloadAppId, type, triggerId, viewId } then calls EventEmitter.emit(viewId,
payload) and returns the type; replace both branches to call this helper to
remove duplication and keep behavior identical for ModalActions.ERRORS and
ModalActions.UPDATE.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06a47120-e6c2-4dfa-b474-d7515477653a
📒 Files selected for processing (3)
app/lib/methods/actions.test.tsapp/lib/methods/actions.tstsconfig.json
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-17T19:15:30.463Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:30.463Z
Learning: In `.maestro/tests/room/share-message.yaml` (Rocket.Chat React Native), the `tapOn: point: 5%,10%` step is intentional: it taps the empty area above the bottom sheet and keyboard to dismiss both simultaneously. Using `action-sheet-handle` instead would only close the sheet but not the keyboard. This pattern is acceptable when both need to be dismissed together in a single step.
Applied to files:
app/lib/methods/actions.ts
🔇 Additional comments (4)
tsconfig.json (2)
7-7: Safe formatting-only change ontargetline.Line 7 updates inline comment formatting only; no compile behavior change.
38-39: Safe formatting-only normalization for unused checks.Lines 38-39 keep the same option values and only adjust comment style.
app/lib/methods/actions.ts (1)
97-171: Strong hardening on request/response and trigger cleanup.Great update: async flow is cleaner, empty-body close is explicit, malformed/unknown response types now fail fast, and
finallyguarantees trigger invalidation.app/lib/methods/actions.test.ts (1)
69-252: Comprehensive behavioral coverage fortriggerAction.This test set validates success paths, side effects (navigation/events), and key failure modes introduced by the new flow.
|
Android Build Available Rocket.Chat Experimental 4.71.0.108396 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNQA5__QIA_JPfiqBj_62NCV05oO_9rytja2At2pf-e4vR4GMhB3G30Ow5m0ThiKAyLroqcAaWbo7om9r2Vt |
|
iOS Build Available Rocket.Chat Experimental 4.71.0.108395 |
Proposed changes
Issue(s)
https://rocketchat.atlassian.net/browse/VMUX-24
How to test or reproduce
https://github.com/RocketChat/Rocket.Chat.ReactNative/actions/runs/23261226760?pr=7057
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Enhancements
Localization
Tests
Chores