fix: notification preference options are not visible#7162
Merged
Conversation
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Contributor
WalkthroughThese changes address notification preference visibility issues by adding optional style props to list components and applying fixed-height styling in the ListPicker component. Test coverage is expanded to verify notification preferences across in-app, push, and email channels with their respective options. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/containers/List/ListItem.tsx (1)
259-271:⚠️ Potential issue | 🟡 MinorApply
stylein the non-press branch too.
IListItemexposesstyle, but whenonPressis absent (Line 269), the style is dropped. That makes behavior inconsistent across the two render paths.🐛 Proposed fix
return ( - <View style={{ backgroundColor: props.backgroundColor || colors.surfaceRoom }}> + <View style={[{ backgroundColor: props.backgroundColor || colors.surfaceRoom }, props.style]}> <Content {...props} /> </View> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/List/ListItem.tsx` around lines 259 - 271, The non-press render path in ListItem drops the passed-in style from IListItem; when props.onPress is falsy the returned View uses only backgroundColor and ignores props.style, causing inconsistent styling compared to the Button branch. Fix by merging/applying props.style on the non-press View (e.g., combine props.style with the backgroundColor-derived style) so Content receives the same visual styling as the Button path; update ListItem to use props.style alongside backgroundColor (symbols to edit: ListItem, IListItem props.style, backgroundColor, colors.surfaceRoom, Button, Content).
🧹 Nitpick comments (2)
app/containers/List/ListRadio.tsx (1)
2-2: Remove duplicatedstyledeclaration fromIListRadio.
IListRadioalready inheritsstylefromIListItem, so redeclaring it adds drift risk for no gain.♻️ Proposed cleanup
-import type { ViewStyle } from 'react-native'; @@ interface IListRadio extends IListItem { value: any; isSelected: boolean; - style?: ViewStyle; }Also applies to: 9-13
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/List/ListRadio.tsx` at line 2, IListRadio declares a duplicate style prop that it already inherits from IListItem; remove the redundant "style" declaration from the IListRadio interface in ListRadio.tsx (and any duplicated declarations around lines 9–13) so the interface relies on the inherited style from IListItem, keeping the rest of IListRadio unchanged.app/containers/List/ListItem.tsx (1)
216-219: UseStyleProp<ViewStyle>for liststyleprops.Line 218 and Line 256 currently type
styleasViewStyle, which is narrower than RN style prop usage and can block array/registered styles.♻️ Proposed typing update
import { I18nManager, type StyleProp, @@ interface IListButtonPress extends IListItemButton { onPress: Function; - style?: ViewStyle; + style?: StyleProp<ViewStyle>; } @@ export interface IListItem extends Omit<IListItemContent, 'theme'>, Omit<IListItemButton, 'theme'> { backgroundColor?: string; onPress?: Function; - style?: ViewStyle; + style?: StyleProp<ViewStyle>; }Also applies to: 253-257
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/List/ListItem.tsx` around lines 216 - 219, The style prop types are too narrow: change the type of style on IListButtonPress and on the other list item interface that currently declares style (IListItemButton) from ViewStyle to StyleProp<ViewStyle>, and add/import StyleProp (and ViewStyle if not already) from 'react-native' so array/registered styles are accepted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.maestro/tests/assorted/user-preferences.yaml:
- Around line 59-60: Replace the flaky text-based assertion with a stable testID
check: instead of "assertVisible: Vibrate for new messages" use the element id
user-notification-preference-view-in-app-vibration (or the test selector that
maps to it) to assert presence/visibility; update the test step that references
the visible string to use the testID selector for the element
(user-notification-preference-view-in-app-vibration) so the E2E check targets
the stable identifier rather than localized text.
In `@app/views/UserNotificationPreferencesView/ListPicker.tsx`:
- Line 59: The testID currently uses the option label which is fragile: replace
the test selector template `notification-preferences-${preference}-${i.label}`
with `notification-preferences-${preference}-${i.value}` so testIDs are tied to
stable option values rather than translatable/mutable labels; update the JSX
prop where testID is set (the template string in ListPicker's option rendering)
to use i.value.
---
Outside diff comments:
In `@app/containers/List/ListItem.tsx`:
- Around line 259-271: The non-press render path in ListItem drops the passed-in
style from IListItem; when props.onPress is falsy the returned View uses only
backgroundColor and ignores props.style, causing inconsistent styling compared
to the Button branch. Fix by merging/applying props.style on the non-press View
(e.g., combine props.style with the backgroundColor-derived style) so Content
receives the same visual styling as the Button path; update ListItem to use
props.style alongside backgroundColor (symbols to edit: ListItem, IListItem
props.style, backgroundColor, colors.surfaceRoom, Button, Content).
---
Nitpick comments:
In `@app/containers/List/ListItem.tsx`:
- Around line 216-219: The style prop types are too narrow: change the type of
style on IListButtonPress and on the other list item interface that currently
declares style (IListItemButton) from ViewStyle to StyleProp<ViewStyle>, and
add/import StyleProp (and ViewStyle if not already) from 'react-native' so
array/registered styles are accepted.
In `@app/containers/List/ListRadio.tsx`:
- Line 2: IListRadio declares a duplicate style prop that it already inherits
from IListItem; remove the redundant "style" declaration from the IListRadio
interface in ListRadio.tsx (and any duplicated declarations around lines 9–13)
so the interface relies on the inherited style from IListItem, keeping the rest
of IListRadio unchanged.
🪄 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: 1b55d390-36da-4d8a-90fc-ece33fc0fabf
📒 Files selected for processing (4)
.maestro/tests/assorted/user-preferences.yamlapp/containers/List/ListItem.tsxapp/containers/List/ListRadio.tsxapp/views/UserNotificationPreferencesView/ListPicker.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Configure Prettier with tabs, single quotes, 130 character width, no trailing commas, arrow parens avoid, and bracket same line
Files:
app/containers/List/ListRadio.tsxapp/views/UserNotificationPreferencesView/ListPicker.tsxapp/containers/List/ListItem.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ESLint with
@rocket.chat/eslint-configbase configuration including React, React Native, TypeScript, and Jest plugins
Files:
app/containers/List/ListRadio.tsxapp/views/UserNotificationPreferencesView/ListPicker.tsxapp/containers/List/ListItem.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript with strict mode enabled and configure baseUrl to app/ for import resolution
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/containers/List/ListRadio.tsxapp/views/UserNotificationPreferencesView/ListPicker.tsxapp/containers/List/ListItem.tsx
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable UI components in app/containers/ directory
Files:
app/containers/List/ListRadio.tsxapp/containers/List/ListItem.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/containers/List/ListRadio.tsxapp/views/UserNotificationPreferencesView/ListPicker.tsxapp/containers/List/ListItem.tsx
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in app/views/ directory
Files:
app/views/UserNotificationPreferencesView/ListPicker.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
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.
📚 Learning: 2026-03-05T14:28:10.004Z
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:10.004Z
Learning: In Maestro YAML selector fields (text, id) within the Rocket.Chat React Native repository, use the contains pattern '.*keyword.*' (leading and trailing '.*') for matching text. The pattern '.*keyword*.' is incorrect and will fail to match cases where the keyword appears at the end of the element's text. This guideline applies to all Maestro YAML selector fields across the codebase.
Applied to files:
.maestro/tests/assorted/user-preferences.yaml
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
…at/Rocket.Chat.ReactNative into notification-toggle-fix
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Failure
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Failure
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Failure
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Failure
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
If we go to the notification preference settings under the profile and click on any alert option, it shows a bottom sheet with empty content.
Regression introduced in #6970
Issue(s)
https://rocketchat.atlassian.net/browse/CORE-2120
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
Tests
Refactor
Style