feat: add channels tab for inbox navigation #996
Conversation
Add long-press translate option for messages on iOS/Android: - Add translateMessage API call to ConversationService - Add translateMessage async action to conversationActions - Add Translate menu option in message long-press context menu - Update TextBubble to display translations with toggle - Add translations field to MessageContentAttributes type - Add i18n strings for translate feature 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add try-catch with .unwrap() for proper error handling - Show error toast when translation fails - Add loading state to prevent duplicate translation requests - Hide translate option for already-translated messages - Add null safety check for displayContent - Use Object.values() for cleaner translation extraction - Add TRANSLATE_MESSAGE_ERROR i18n string 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d024fb7d39
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| translateMessage: createAsyncThunk<void, TranslateMessagePayload>( | ||
| 'conversations/translateMessage', | ||
| async (payload, { rejectWithValue }) => { | ||
| try { | ||
| await ConversationService.translateMessage(payload); |
There was a problem hiding this comment.
When a user taps Translate on a message, the thunk at this location just posts to /translate and resolves without dispatching any reducer update or returning the translated content. The UI renders translations only from message.contentAttributes.translations in TextBubble.tsx, so the successful toast shown in handleTranslateMessage never displays a translated body and the translate option remains available to re-run indefinitely. The new translate action therefore does nothing visible for users.
Useful? React with 👍 / 👎.
|
Summary
Tests: yarn run v1.22.22 |
There was a problem hiding this comment.
Pull request overview
This PR adds a new "Channels" tab to the bottom navigation bar and implements a message translation feature. The Channels tab displays all inbox channels and allows users to filter conversations by selecting a channel. Additionally, the PR introduces functionality to translate messages into the user's preferred locale with a toggle to view original or translated content.
Key changes:
- New Channels tab with mailbox icon and screen listing all inbox channels
- Message translation feature with API integration and UI controls
- New
setFiltersStateaction to replace all conversation filters at once
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/navigation/tabs/AppTabs.tsx | Adds Channels tab to navigation with proper permission checks |
| src/navigation/tabs/BottomTabBar.tsx | Updates tab bar icon rendering and adjusts padding for 4-tab layout |
| src/navigation/stack/ChannelsStack.tsx | New stack navigator for Channels screen |
| src/navigation/stack/index.ts | Exports new ChannelsStack |
| src/screens/channels/ChannelsScreen.tsx | New screen component displaying inbox channels list with filtering |
| src/svg-icons/tabs/ChannelsIcon.tsx | New mailbox icon component with outline and filled variants |
| src/svg-icons/tabs/index.ts | Exports new ChannelsIcon components |
| src/store/conversation/conversationFilterSlice.ts | Adds setFiltersState action to replace entire filter state |
| src/store/conversation/specs/conversationFilterSlice.spec.ts | Tests for new setFiltersState action |
| src/types/Message.ts | Adds optional translations field to message content attributes |
| src/store/conversation/conversationTypes.ts | Adds types for translate message payload and API response |
| src/store/conversation/conversationService.ts | Implements translateMessage API service method |
| src/store/conversation/conversationActions.ts | Adds translateMessage async thunk action |
| src/store/conversation/specs/conversationActions.spec.ts | Tests for translateMessage action |
| src/screens/chat-screen/components/message-item/MessageItemContainer.tsx | Adds translate menu option and handling logic |
| src/screens/chat-screen/components/message-components/TextBubble.tsx | Displays translated content with toggle between original and translation |
| src/screens/chat-screen/components/message-list/stories/Basic.stories.tsx | Adds story for translated message display |
| src/i18n/en.json | Adds translation-related i18n strings |
| CHANNELS_TAB_PLAN.md | Planning document (should be removed) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const hasTranslations = | ||
| message.contentAttributes?.translations && | ||
| Object.keys(message.contentAttributes.translations).length > 0; | ||
| const isTranslating = translatingMessageId === message.id; |
There was a problem hiding this comment.
The translate action only checks if translations exist, not if the specific target language translation already exists. This means users cannot re-translate a message to the same language if they want to, and the button is hidden even if the current locale translation is missing. Consider checking for the specific locale translation instead of just checking if any translations exist.
| static async translateMessage( | ||
| payload: TranslateMessagePayload, | ||
| ): Promise<TranslateMessageAPIResponse> { | ||
| const { conversationId, messageId, targetLanguage } = payload; | ||
| const response = await apiService.post<TranslateMessageAPIResponse>( | ||
| `conversations/${conversationId}/messages/${messageId}/translate`, | ||
| { | ||
| target_language: targetLanguage, | ||
| }, | ||
| ); | ||
| return response.data; | ||
| } |
There was a problem hiding this comment.
The new translateMessage service method lacks test coverage. Consider adding a test case in conversationService.spec.ts to verify that the API is called with the correct endpoint and payload, similar to the existing tests for other service methods.
| # Channels Tab Plan | ||
|
|
||
| ## Goal | ||
| Add a new bottom tab (left of Inbox) with the desktop channel icon (`i-lucide-mailbox`), labeled **Channels**. The new tab shows a list of all inbox channels. Selecting a channel resets conversation filters and navigates to the Conversations list filtered by that inbox. | ||
|
|
||
| ## Scope | ||
| - Mobile app only (`chatwoot-mobile-app`). | ||
| - New tab + new screen for channel list. | ||
| - Reuse existing conversation filters and inbox data. | ||
| - No "Unread vs All" toggle work. | ||
|
|
||
| ## UX Decisions (confirmed) | ||
| - Icon: matches desktop `i-lucide-mailbox` (we will add an equivalent SVG in mobile). | ||
| - Title: "Channels". | ||
| - List content: all inboxes/channels (no "All inboxes" entry). | ||
| - Selection behavior: reset conversation filters (assignee/status/sort/inbox) and then apply the selected inbox filter. | ||
|
|
||
| ## Implementation Steps | ||
| 1. **Assets & Icons** | ||
| - Create a new SVG icon component for the mailbox icon (outline + filled if needed) under `src/svg-icons/tabs/`. | ||
| - Export in `src/svg-icons/tabs/index.ts` and `src/svg-icons/index.ts`. | ||
|
|
||
| 2. **Navigation & Tabs** | ||
| - Add a new tab route (e.g., `Channels`) in `src/navigation/tabs/AppTabs.tsx`. | ||
| - Create a new stack for channels (e.g., `ChannelsStack`) in `src/navigation/stack/`. | ||
| - Update `BottomTabBar` to include the new icon in `TabBarIcons`. | ||
| - Adjust tab bar padding (`pl/pr`) if needed for 4 tabs. | ||
|
|
||
| 3. **Channels Screen (List of Inboxes)** | ||
| - New screen file (e.g., `src/screens/channels/ChannelsScreen.tsx`). | ||
| - Use `selectAllInboxes` to show channels with `getChannelIcon`. | ||
| - Match visual style used in conversation inbox picker (list row layout). | ||
| - Header title "Channels". | ||
|
|
||
| 4. **Selection Behavior** | ||
| - On tap: dispatch `resetFilters()` from `conversationFilterSlice`, then `setFilters({ key: 'inbox_id', value: inbox.id.toString() })`. | ||
| - Navigate to Conversations tab (likely `navigation.navigate('Conversations')`). | ||
|
|
||
| 5. **Testing** | ||
| - Run `pnpm test` and report results. | ||
| - If tests are too heavy, at least run a targeted lint/test if available. | ||
|
|
||
| 6. **Review Package (no PR submission)** | ||
| - Provide diff summary and the list of changed files. | ||
| - Wait for your review before any PR creation or push. | ||
|
|
||
| ## Risks & Notes | ||
| - Verify the correct tab navigation target name (Tabs uses `Conversations`). | ||
| - Ensure filter reset does not break persisted filters. | ||
| - Ensure list uses available inbox data and renders safely when empty. | ||
|
|
||
| ## Done When | ||
| - New Channels tab appears left of Inbox with mailbox icon. | ||
| - Channels list shows all inboxes and navigates to Conversations with filters reset and inbox filter applied. | ||
| - Tests run locally and results shared. | ||
| - No PR submitted or pushed. |
There was a problem hiding this comment.
The CHANNELS_TAB_PLAN.md file appears to be a planning document and should not be included in the production codebase. Consider removing this file from the PR as it's not meant for production deployment.
| if (typeof translatedContent === 'string') { | ||
| const state = getState() as { | ||
| conversations: { | ||
| entities: Record<number, { messages: Message[] } | undefined>; | ||
| }; | ||
| }; | ||
| const conversation = state.conversations.entities[payload.conversationId]; | ||
| const existingMessage = conversation?.messages?.find( | ||
| message => message.id === payload.messageId, | ||
| ); | ||
|
|
||
| if (existingMessage) { | ||
| const contentAttributes = existingMessage.contentAttributes ?? {}; | ||
| const translations = contentAttributes.translations ?? {}; | ||
| dispatch({ | ||
| type: 'conversation/addOrUpdateMessage', | ||
| payload: { | ||
| ...existingMessage, | ||
| contentAttributes: { | ||
| ...contentAttributes, | ||
| translations: { | ||
| ...translations, | ||
| [payload.targetLanguage]: translatedContent, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The condition checks if the translatedContent is a string, but the API response type suggests content can be null. When content is null, the translation shouldn't be added to the message. However, the current logic silently ignores null responses without providing feedback to the user. Consider handling the null case explicitly, perhaps by throwing an error that will be caught by the catch block to show the error toast.
| if (typeof translatedContent === 'string') { | |
| const state = getState() as { | |
| conversations: { | |
| entities: Record<number, { messages: Message[] } | undefined>; | |
| }; | |
| }; | |
| const conversation = state.conversations.entities[payload.conversationId]; | |
| const existingMessage = conversation?.messages?.find( | |
| message => message.id === payload.messageId, | |
| ); | |
| if (existingMessage) { | |
| const contentAttributes = existingMessage.contentAttributes ?? {}; | |
| const translations = contentAttributes.translations ?? {}; | |
| dispatch({ | |
| type: 'conversation/addOrUpdateMessage', | |
| payload: { | |
| ...existingMessage, | |
| contentAttributes: { | |
| ...contentAttributes, | |
| translations: { | |
| ...translations, | |
| [payload.targetLanguage]: translatedContent, | |
| }, | |
| }, | |
| }, | |
| }); | |
| } | |
| if (typeof translatedContent !== 'string') { | |
| const error = { | |
| response: { | |
| data: { | |
| // Fallback error payload when translation content is missing or invalid | |
| } as ApiErrorResponse, | |
| }, | |
| } as AxiosError<ApiErrorResponse>; | |
| throw error; | |
| } | |
| const state = getState() as { | |
| conversations: { | |
| entities: Record<number, { messages: Message[] } | undefined>; | |
| }; | |
| }; | |
| const conversation = state.conversations.entities[payload.conversationId]; | |
| const existingMessage = conversation?.messages?.find( | |
| message => message.id === payload.messageId, | |
| ); | |
| if (existingMessage) { | |
| const contentAttributes = existingMessage.contentAttributes ?? {}; | |
| const translations = contentAttributes.translations ?? {}; | |
| dispatch({ | |
| type: 'conversation/addOrUpdateMessage', | |
| payload: { | |
| ...existingMessage, | |
| contentAttributes: { | |
| ...contentAttributes, | |
| translations: { | |
| ...translations, | |
| [payload.targetLanguage]: translatedContent, | |
| }, | |
| }, | |
| }, | |
| }); |
| inbox_id: inboxId.toString(), | ||
| }), | ||
| ); | ||
| navigation.navigate('Conversations'); |
There was a problem hiding this comment.
The navigation call should be type-safe. Consider typing the navigation hook with the proper navigation type. You can use useNavigation<NavigationProp<TabParamList>>() from '@react-navigation/native' to ensure type safety and avoid runtime errors if the route name changes.
| const translations = contentAttributes?.translations; | ||
| const hasTranslations = translations && Object.keys(translations).length > 0; | ||
| const translationContent = hasTranslations | ||
| ? Object.values(translations)[0] | ||
| : null; | ||
|
|
||
| const displayContent = | ||
| hasTranslations && !showOriginal && translationContent | ||
| ? translationContent | ||
| : content; | ||
|
|
||
| const renderTranslationToggle = () => { | ||
| if (!hasTranslations) return null; | ||
|
|
||
| return ( | ||
| <Pressable onPress={() => setShowOriginal(!showOriginal)}> | ||
| <Animated.Text | ||
| style={tailwind.style( | ||
| 'text-xs text-gray-500 mt-1 font-inter-420-20 tracking-[0.32px]', | ||
| )}> | ||
| {showOriginal | ||
| ? i18n.t('CONVERSATION.VIEW_TRANSLATED') | ||
| : i18n.t('CONVERSATION.VIEW_ORIGINAL')} |
There was a problem hiding this comment.
The translation toggle always displays the first translation value regardless of which language it is. Consider displaying the translation that matches the current locale, or if not available, show a label indicating which language the translation is in so users know what they're viewing.
| const translations = contentAttributes?.translations; | |
| const hasTranslations = translations && Object.keys(translations).length > 0; | |
| const translationContent = hasTranslations | |
| ? Object.values(translations)[0] | |
| : null; | |
| const displayContent = | |
| hasTranslations && !showOriginal && translationContent | |
| ? translationContent | |
| : content; | |
| const renderTranslationToggle = () => { | |
| if (!hasTranslations) return null; | |
| return ( | |
| <Pressable onPress={() => setShowOriginal(!showOriginal)}> | |
| <Animated.Text | |
| style={tailwind.style( | |
| 'text-xs text-gray-500 mt-1 font-inter-420-20 tracking-[0.32px]', | |
| )}> | |
| {showOriginal | |
| ? i18n.t('CONVERSATION.VIEW_TRANSLATED') | |
| : i18n.t('CONVERSATION.VIEW_ORIGINAL')} | |
| const translations = contentAttributes?.translations as | |
| | Record<string, string> | |
| | undefined; | |
| const hasTranslations = | |
| translations !== undefined && Object.keys(translations).length > 0; | |
| // Determine the current locale from i18n, trying common properties/methods. | |
| const currentLocale: string | undefined = | |
| (i18n as any).language || | |
| (i18n as any).locale || | |
| (typeof (i18n as any).currentLocale === 'function' | |
| ? (i18n as any).currentLocale() | |
| : undefined); | |
| let selectedTranslationKey: string | undefined; | |
| let selectedTranslation: string | null = null; | |
| if (hasTranslations && translations) { | |
| const translationKeys = Object.keys(translations); | |
| // Try to find an exact match for the current locale (e.g., "en-US"). | |
| if (currentLocale && translations[currentLocale]) { | |
| selectedTranslationKey = currentLocale; | |
| selectedTranslation = translations[currentLocale]; | |
| } else if (currentLocale) { | |
| // Fallback: match by base language code (e.g., "en" matches "en-US"). | |
| const baseLocale = currentLocale.split('-')[0]; | |
| const matchedKey = translationKeys.find( | |
| key => key === baseLocale || key.split('-')[0] === baseLocale, | |
| ); | |
| if (matchedKey) { | |
| selectedTranslationKey = matchedKey; | |
| selectedTranslation = translations[matchedKey]; | |
| } | |
| } | |
| // Final fallback: use the first available translation. | |
| if (!selectedTranslation) { | |
| selectedTranslationKey = translationKeys[0]; | |
| selectedTranslation = translations[selectedTranslationKey]; | |
| } | |
| } | |
| const displayContent = | |
| hasTranslations && !showOriginal && selectedTranslation | |
| ? selectedTranslation | |
| : content; | |
| const renderTranslationToggle = () => { | |
| if (!hasTranslations) return null; | |
| const translationLanguageLabel = selectedTranslationKey | |
| ? selectedTranslationKey.toUpperCase() | |
| : null; | |
| const toggleText = showOriginal | |
| ? i18n.t('CONVERSATION.VIEW_TRANSLATED') | |
| : i18n.t('CONVERSATION.VIEW_ORIGINAL'); | |
| return ( | |
| <Pressable onPress={() => setShowOriginal(!showOriginal)}> | |
| <Animated.Text | |
| style={tailwind.style( | |
| 'text-xs text-gray-500 mt-1 font-inter-420-20 tracking-[0.32px]', | |
| )}> | |
| {toggleText} | |
| {/* When showing the translated content, indicate which language it is. */} | |
| {!showOriginal && translationLanguageLabel | |
| ? ` (${translationLanguageLabel})` | |
| : null} |
|
@muhsin-k — this adds a channels tab for inbox navigation, making it easier to switch between different channels. Would really appreciate a review. Thanks for your time! |
1 participant
Summary
Testing