feat: apply font size scaling feature across all components #6906
Conversation
- Apply scaleFontSize to all Text components throughout the application - Update Directory pages (DirectoryItem, DirectoryOptions) - Update Reactions components (Reactions, ReactionsList, AllTab) - Update ListPicker components across multiple views - Update RoomInfoView components (ItemLabel, RoomInfoTag, Direct, Livechat) - Update various view components (JoinCode, LoadMore, AuthLoadingView, etc.) - Fix TypeScript errors by using object spread for style props - Ensure all user-facing text respects font size preference setting
WalkthroughAdds app-wide font-size support: extends useResponsiveLayout with persistent font-size preferences and scaleFontSize, updates many UI components to consume scaleFontSize, adds Font Size picker UI and navigation routes, and adjusts RoomItem styles to handle scaled fonts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as FontSizePickerView
participant Storage as UserPreferencesStorage
participant Hook as useResponsiveLayout
participant App as App Components
User->>UI: open Font Size screen & adjust slider
UI->>Storage: persist FONT_SIZE_PREFERENCES_KEY
Storage-->>Hook: storage change event
Hook->>App: update context (fontScale, scaleFontSize, row heights)
App->>App: components re-render using new scaleFontSize values
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/views/PickerView.tsx (1)
17-24: Remove redundant fontSize from StyleSheet.The
fontSize: 16in thenoResultstyle (line 19) is now redundant since line 113 appliesfontSize: scaleFontSize(16)inline, which overrides it. Remove the static fontSize to avoid confusion and maintain a single source of truth.♻️ Proposed fix
const styles = StyleSheet.create({ noResult: { - fontSize: 16, paddingVertical: 56, ...sharedStyles.textSemibold, ...sharedStyles.textAlignCenter } });
🤖 Fix all issues with AI agents
In @app/containers/Button/index.tsx:
- Around line 84-92: The dynamic font size object in textStyle is placed before
the static styles so styles.text or styles.smallText can override fontSize;
update textStyle to apply static styles first (styles.smallText or styles.text,
plus styleText) and place the dynamic object ({ color: ..., fontSize: ...,
lineHeight: ... }) last so the computed fontSize from scaleFontSize(fontSize) or
small logic takes precedence (refer to textStyle, styles.text, styles.smallText,
styleText, scaleFontSize, and fontSize).
In @app/containers/MessageComposer/components/RecordAudio/Duration.tsx:
- Around line 8-9: The duration label style spreads sharedStyles.textRegular
before applying the scaled fontSize, so sharedStyles.textRegular can override
the computed size; in the Duration component locate the Text/style objects that
use sharedStyles.textRegular (e.g., in the render and the other instance around
lines 34-42) and change the spread order so the computed/scaled fontSize is
applied after the spread (i.e., spread sharedStyles.textRegular first, then set
fontSize: scaledFontSize) so the scaled value takes precedence.
In @app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx:
- Around line 44-59: The polling setInterval in useResponsiveLayout should be
removed and replaced with MMKV's addOnValueChangedListener to avoid unnecessary
re-renders; register userPreferences.addOnValueChangedListener((key) => { if
(key === FONT_SIZE_PREFERENCES_KEY) setUpdateTrigger(t => t + 1) }) in the
useEffect that currently sets the interval, store the returned subscription and
call its remove() in the cleanup, and remove the empty if (updateTrigger) block
and its comment; continue to read customFontSize from
userPreferences.getString(FONT_SIZE_PREFERENCES_KEY) so the component re-reads
on updates triggered by the listener.
🧹 Nitpick comments (21)
app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx (1)
42-42: Consider removing duplicatefontSizefrom StyleSheet.The static
fontSize: 16instyles.text(line 29) is now overridden by the inlinefontSize: scaleFontSize(16). You can remove the static value to avoid confusion.♻️ Proposed cleanup
const styles = StyleSheet.create({ // ... text: { flex: 1, - fontSize: 16, ...sharedStyles.textRegular } });app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx (1)
96-96: Font scaling applied correctly.The
scaleFontSizefunction is properly applied to bothfontSizeandlineHeightfor both title text components, which ensures proper text rendering at different scale factors.♻️ Optional: Clean up redundant static styles
The static
fontSizeandlineHeightvalues instyles.leftTitle(lines 21-22) andstyles.rightTitle(lines 29-30) are now always overridden by the dynamic inline styles and could be removed to avoid confusion:leftTitle: { ...sharedStyles.textMedium, - fontSize: 16, - lineHeight: 24 },rightTitle: { ...sharedStyles.textRegular, - fontSize: 16, - lineHeight: 24 },Also applies to: 101-101
app/views/SelectedUsersView/Header.tsx (1)
14-20: Remove the redundant hardcodedfontSize: 12fromstyles.selectedText.Right now
styles.selectedText.fontSizeis always overridden by the inline{ fontSize: scaleFontSize(12) }, which can cause confusion during future edits.Proposed cleanup
const styles = StyleSheet.create({ selectedText: { marginLeft: 16, marginBottom: 12, - fontSize: 12, ...sharedStyles.textRegular },Also applies to: 50-52
app/views/UserPreferencesView/ListPicker.tsx (1)
76-76: Dynamic font sizing correctly applied.The
scaleFontSize(16)is correctly applied to the label text, ensuring it respects the user's font size preference. The style array spreading correctly overrides the static fontSize fromstyles.title.♻️ Optional: Remove redundant static fontSize
Since the fontSize is now dynamically set at line 76, the static
fontSize: 16instyles.title(line 13) is always overridden and could be removed for clarity:const styles = StyleSheet.create({ - title: { ...sharedStyles.textRegular, fontSize: 16 } + title: { ...sharedStyles.textRegular } });This is a minor cleanup and not required, as the current implementation works correctly.
app/views/RoomView/LoadMore/index.tsx (1)
75-75: Dynamic font sizing correctly applied to load more text.The
scaleFontSize(16)is correctly applied within the style array, ensuring the "Load More" text respects the user's font size preference.♻️ Optional: Remove redundant static fontSize
Since fontSize is now dynamically set at line 75, the static
fontSize: 16instyles.text(line 23) is always overridden and could be removed:const styles = StyleSheet.create({ button: { paddingVertical: 16, alignItems: 'center', justifyContent: 'center' }, text: { - fontSize: 16, ...sharedStyles.textMedium } });This is optional cleanup; the current implementation functions correctly.
app/views/AuthLoadingView.tsx (1)
33-33: Dynamic font sizing correctly applied to loading text.The
scaleFontSize(16)is correctly applied to the loading message text, ensuring it respects the user's font size preference. This improves accessibility across the authentication flow.♻️ Optional: Remove redundant static fontSize
Since fontSize is now dynamically set at line 33, the static
fontSize: 16instyles.text(line 17) is always overridden and could be removed:const styles = StyleSheet.create({ container: { flex: 1, justifyContent: 'center', alignItems: 'center' }, text: { - fontSize: 16, paddingTop: 10, ...sharedStyles.textRegular, ...sharedStyles.textAlignCenter } });This is optional cleanup; the current implementation works as intended.
app/containers/message/Components/Attachments/Reply.tsx (2)
103-114: Reply header “time” text may still be unscaled (inconsistent font preference).
author_name/titlenow usescaleFontSize(16), but the adjacent timeTextrelies onmessageStyles.time(which may be fixed-size). Consider explicitly scaling thatTexttoo for consistency.
185-198:key={field.title}can collide; prefer a stable unique key.
If multiple fields share the same title (or title is empty), React reconciliation can misbehave. Consider using a composite key (e.g., title + index) or a real field id if available.app/containers/UserGeneratedContentRules.tsx (1)
9-52: Typography scaling looks correct; consider link accessibility + lineHeight sanity.
- For the pressable “Terms_of_Service” / “Privacy_Policy” Text, consider adding
accessibilityRole="link"(and possiblyaccessibilityHint) to better convey semantics to screen readers.- Please sanity-check that
scaleFontSize(22)yields a sensible lineHeight relative toscaleFontSize(14)across all font scale settings.app/containers/message/Time.tsx (1)
7-23: Good: time now respects font scaling; consider guardingtsif “now” is undesirable.
Iftscan be missing in some paths, you may want to returnnull(or'') rather than formatting the current time implicitly.app/containers/RoomItem/Wrapper.tsx (1)
2-29: Consider hoistingborderWidthout of render + verify Android rendering of 0.5 borders (may look uneven).If this is intended as a consistent “1px-ish” divider, computing it once avoids recreating the number each render.
Proposed tweak
import React from 'react'; import { StyleSheet, View } from 'react-native'; @@ import { useResponsiveLayout } from '../../lib/hooks/useResponsiveLayout/useResponsiveLayout'; +const BORDER_WIDTH = Math.max(StyleSheet.hairlineWidth || 0, 0.5); + const Wrapper = ({ accessibilityLabel, children, displayMode, ...props }: IWrapperProps): React.ReactElement => { const { colors } = useTheme(); const { rowHeight, rowHeightCondensed } = useResponsiveLayout(); - const borderWidth = Math.max(StyleSheet.hairlineWidth || 0, 0.5); return ( @@ { borderColor: colors.strokeLight, - borderBottomWidth: borderWidth + borderBottomWidth: BORDER_WIDTH } ]}>app/views/RoomInfoView/Livechat.tsx (1)
16-22: Consider removing redundant fontSize from StyleSheet.The
fontSize: 16defined instyles.title(line 18) is now overridden by the inlinefontSize: scaleFontSize(16)on line 27, making the static value in the StyleSheet redundant.♻️ Proposed cleanup
const styles = StyleSheet.create({ title: { - fontSize: 16, paddingHorizontal: 20, ...sharedStyles.textMedium } });Also applies to: 26-27
app/views/UserNotificationPreferencesView/ListPicker.tsx (1)
13-18: Consider removing redundant fontSize from StyleSheet.The
fontSize: 16defined instyles.pickerText(line 16) is now overridden by the inlinefontSize: scaleFontSize(16)on line 71, making the static value redundant.♻️ Proposed cleanup
const styles = StyleSheet.create({ pickerText: { - ...sharedStyles.textRegular, - fontSize: 16 + ...sharedStyles.textRegular } });Also applies to: 40-40, 71-71
app/views/RegisterView/index.tsx (1)
52-52: LGTM! Responsive typography correctly applied.The scaleFontSize hook is properly used for both the Sign Up title and the login prompt text, ensuring these user-facing elements respect font size preferences. The inline styling approach is consistent with existing color styling in this component.
Optional: Extract responsive font styles to styles object
For improved maintainability, you could extract the font sizing to the styles object, though the current inline approach is acceptable:
const styles = StyleSheet.create({ // ... existing styles titleText: { fontSize: 24, lineHeight: 36 }, bottomText: { fontSize: 14, lineHeight: 22 } }); // Then in useStyle or component: const responsiveStyles = { titleText: { ...styles.titleText, fontSize: scaleFontSize(styles.titleText.fontSize), lineHeight: scaleFontSize(styles.titleText.lineHeight) } // ... };This keeps the base values in one place while still applying responsive scaling.
Also applies to: 182-182, 299-299
app/containers/RoomItem/Title.tsx (1)
13-21: Consider memoizing the inline style object.The inline style object
{ color: colors.fontTitlesLabels, fontSize: scaleFontSize(17) }is recreated on every render. For better performance, consider usinguseMemo:const titleStyle = useMemo( () => ({ color: colors.fontTitlesLabels, fontSize: scaleFontSize(17) }), [colors.fontTitlesLabels, scaleFontSize] );However, if this pattern is consistent across the PR and performance testing shows no issues, the current approach is acceptable.
app/containers/ReactionsList/AllTab.tsx (1)
11-12: Defaultreactionsto[]to avoidFlatListreceivingundefined.
This keeps behavior consistent and avoids edge cases across RN versions.Proposed diff
-const AllTab = ({ reactions, getCustomEmoji }: IAllTabProps): React.ReactElement => ( +const AllTab = ({ reactions = [], getCustomEmoji }: IAllTabProps): React.ReactElement => ( <View style={styles.allTabContainer} testID='reactionsListAllTab'> <FlatList data={reactions} contentContainerStyle={styles.listContainer} renderItem={({ item }) => <AllReactionsListItem item={item} getCustomEmoji={getCustomEmoji} />} keyExtractor={item => item.emoji} /> </View> );Also applies to: 25-26, 51-60, 65-72
app/views/DiscussionsView/DiscussionDetails.tsx (1)
26-30: Remove unusedfontSizefrom static styles.The
detailTextstyle definesfontSize: 10, but this is now overridden inline withscaleFontSize(10)on lines 51 and 58. The staticfontSizeis dead code and creates confusion about which value is actually used.♻️ Proposed fix to remove unused fontSize
detailText: { - fontSize: 10, marginLeft: 2, ...sharedStyles.textSemibold }app/containers/InAppNotification/NotifierComponent.tsx (1)
117-122: Consider calculating lineHeight as a ratio of fontSize.Currently, both
fontSizeandlineHeightare scaled independently usingscaleFontSize. While this works, it means the ratio between them changes ifscaleFontSizeapplies non-linear scaling. A more typical approach would be to calculatelineHeightas a ratio of the scaledfontSize:const roomNameFontSize = scaleFontSize(17); const roomNameLineHeight = roomNameFontSize * 1.18; // maintains ratio const messageFontSize = scaleFontSize(14); const messageLineHeight = messageFontSize * 1.21; // maintains ratioThis ensures consistent line spacing regardless of font scaling. However, if the current approach is intentional per design system requirements, it can remain as-is.
app/views/MediaAutoDownloadView/ListPicker.tsx (1)
19-23: Remove redundant fontSize and lineHeight from static styles.The static styles define
fontSize: 16andlineHeight: 24(lines 21-22, 29-30), but these values are always overridden inline with dynamic values (lines 98, 103). Remove the static declarations to eliminate duplication and potential confusion.♻️ Proposed cleanup
const styles = StyleSheet.create({ leftTitleContainer: { flex: 1, justifyContent: 'center', alignItems: 'flex-start' }, leftTitle: { - ...sharedStyles.textMedium, - fontSize: 16, - lineHeight: 24 + ...sharedStyles.textMedium }, rightContainer: { flex: 1 }, rightTitle: { - ...sharedStyles.textRegular, - fontSize: 16, - lineHeight: 24 + ...sharedStyles.textRegular }, rightTitleContainer: { flex: 1, justifyContent: 'center', alignItems: 'flex-end' } });Also applies to: 27-31, 98-98, 103-103
app/views/UserPreferencesView/FontSizePicker.tsx (1)
14-16: Remove redundantfontSizefrom static styles.The static
fontSize: 16is overridden byscaleFontSize(16)at line 67. Remove it to avoid confusion.Suggested fix
const styles = StyleSheet.create({ - title: { ...sharedStyles.textRegular, fontSize: 16 } + title: { ...sharedStyles.textRegular } });app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
61-63: Minor: Redundant ternary check.
customFontSizeis already guaranteed to have a value from line 55's default assignment. The ternary on line 62 is unnecessary.Suggested fix
- const fontSizeMultiplier = customFontSize ? parseFloat(customFontSize) : 1.0; + const fontSizeMultiplier = parseFloat(customFontSize);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (96)
app/containers/ActionSheet/ActionSheetContentWithInputAndSubmit/index.tsxapp/containers/ActionSheet/BottomSheetContent.tsxapp/containers/ActionSheet/Item.tsxapp/containers/ActionSheet/styles.tsapp/containers/AppVersion.tsxapp/containers/BackgroundContainer/index.tsxapp/containers/Button/index.tsxapp/containers/CallHeader.tsxapp/containers/ChangePasswordRequired.tsxapp/containers/Chip/index.tsxapp/containers/CollapsibleText/index.tsxapp/containers/DirectoryItem/index.tsxapp/containers/Header/components/HeaderButton/HeaderButtonItem.tsxapp/containers/Header/components/HeaderTitle/index.tsxapp/containers/Header/components/HeaderTitle/styles.tsapp/containers/InAppNotification/NotifierComponent.tsxapp/containers/List/ListHeader.tsxapp/containers/List/ListInfo.tsxapp/containers/List/ListItem.tsxapp/containers/MessageActions/Header.tsxapp/containers/MessageComposer/components/ComposerInput.tsxapp/containers/MessageComposer/components/EmojiSearchbar.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/components/RecordAudio/Duration.tsxapp/containers/MessageComposer/components/RecordAudio/RecordAudio.tsxapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/containers/MessageSeparator.tsxapp/containers/OrSeparator.tsxapp/containers/ReactionsList/AllTab.tsxapp/containers/ReactionsList/UsersList.tsxapp/containers/ReactionsList/index.tsxapp/containers/RoomItem/LastMessage.tsxapp/containers/RoomItem/RoomItem.stories.tsxapp/containers/RoomItem/RoomItem.tsxapp/containers/RoomItem/Tag.tsxapp/containers/RoomItem/Title.tsxapp/containers/RoomItem/UpdatedAt.tsxapp/containers/RoomItem/Wrapper.tsxapp/containers/RoomItem/styles.tsapp/containers/SearchHeader.tsxapp/containers/TextInput/FormTextInput.tsxapp/containers/ThreadDetails.tsxapp/containers/Toast.tsxapp/containers/UnreadBadge/index.tsxapp/containers/UserGeneratedContentRules.tsxapp/containers/UserItem.tsxapp/containers/markdown/components/Heading.tsxapp/containers/markdown/components/InlineCode.tsxapp/containers/markdown/components/Paragraph.tsxapp/containers/markdown/components/Plain.tsxapp/containers/markdown/components/Preview.tsxapp/containers/markdown/components/code/CodeLine.tsxapp/containers/markdown/components/inline/Link.tsxapp/containers/markdown/components/mentions/AtMention.tsxapp/containers/markdown/components/mentions/Hashtag.tsxapp/containers/markdown/styles.tsapp/containers/message/Components/Attachments/CollapsibleQuote/index.tsxapp/containers/message/Components/Attachments/Reply.tsxapp/containers/message/Components/Attachments/Video.tsxapp/containers/message/Message.stories.tsxapp/containers/message/Reactions.tsxapp/containers/message/Time.tsxapp/containers/message/Urls.tsxapp/containers/message/User.tsxapp/i18n/locales/en.jsonapp/lib/constants/keys.tsapp/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsxapp/views/AccessibilityAndAppearanceView/components/ListPicker.tsxapp/views/AuthLoadingView.tsxapp/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsxapp/views/CreateChannelView/RoomSettings/SwitchItem.tsxapp/views/DirectoryView/Options.tsxapp/views/DiscussionsView/DiscussionDetails.tsxapp/views/ForgotPasswordView.tsxapp/views/LoginView/UserForm.tsxapp/views/MediaAutoDownloadView/ListPicker.tsxapp/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.stories.tsxapp/views/NotificationPreferencesView/index.tsxapp/views/PickerView.tsxapp/views/ProfileView/components/DeleteAccountActionSheetContent/index.tsxapp/views/RegisterView/index.tsxapp/views/RoomInfoView/Direct.tsxapp/views/RoomInfoView/Livechat.tsxapp/views/RoomInfoView/components/ItemLabel.tsxapp/views/RoomInfoView/components/RoomInfoTag.tsxapp/views/RoomInfoView/components/RoomInfoViewTitle.tsxapp/views/RoomView/JoinCode.tsxapp/views/RoomView/LoadMore/index.tsxapp/views/RoomsListView/components/Header.tsxapp/views/SelectedUsersView/Header.tsxapp/views/SendEmailConfirmationView.tsxapp/views/SetUsernameView.tsxapp/views/UserNotificationPreferencesView/ListPicker.tsxapp/views/UserPreferencesView/FontSizePicker.tsxapp/views/UserPreferencesView/ListPicker.tsxapp/views/UserPreferencesView/index.tsx
💤 Files with no reviewable changes (2)
- app/containers/ActionSheet/styles.ts
- app/containers/Header/components/HeaderTitle/styles.ts
🧰 Additional context used
🧬 Code graph analysis (78)
app/views/ProfileView/components/DeleteAccountActionSheetContent/index.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/PickerView.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/RoomInfoView/components/RoomInfoTag.tsx (4)
app/containers/CustomIcon/index.tsx (2)
TIconsName(21-21)CustomIcon(47-47)app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
app/views/UserNotificationPreferencesView/ListPicker.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/ReactionsList/UsersList.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/SelectedUsersView/Header.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/views/RoomInfoView/components/RoomInfoViewTitle.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/OrSeparator.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/MessageComposer/components/Quotes/Quote.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/RoomItem/UpdatedAt.tsx (3)
app/containers/RoomItem/interfaces.ts (1)
IUpdatedAtProps(36-40)app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/UnreadBadge/index.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/markdown/components/InlineCode.tsx (3)
app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/views/RoomInfoView/Direct.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/ReactionsList/index.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/markdown/components/Heading.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/containers/markdown/components/mentions/AtMention.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/containers/markdown/components/mentions/Hashtag.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/containers/message/Urls.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/RoomInfoView/Livechat.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/ThreadDetails.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/views/RoomsListView/components/Header.tsx (3)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/containers/Header/components/HeaderTitle/styles.ts (1)
styles(5-21)app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/markdown/components/code/CodeLine.tsx (2)
app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/MediaAutoDownloadView/ListPicker.tsx (3)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/containers/Header/components/HeaderTitle/styles.ts (1)
styles(5-21)app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/RoomItem/Wrapper.tsx (4)
app/containers/RoomItem/interfaces.ts (1)
IWrapperProps(42-57)app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/MessageComposer/components/SendThreadToChannel.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/UserItem.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/List/ListInfo.tsx (4)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/containers/Header/components/HeaderTitle/styles.ts (1)
styles(5-21)app/containers/List/styles.ts (1)
styles(3-7)app/lib/constants/colors.ts (1)
themes(304-304)
app/containers/BackgroundContainer/index.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/containers/message/Reactions.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/containers/ReactionsList/AllTab.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/DirectoryItem/index.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/message/Components/Attachments/Video.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/message/Components/Attachments/CollapsibleQuote/index.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/MessageSeparator.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/containers/message/Time.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/MessageComposer/components/EmojiSearchbar.tsx (3)
app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/MessageComposer/components/RecordAudio/RecordAudio.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/UserGeneratedContentRules.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/MessageComposer/components/RecordAudio/Duration.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/CollapsibleText/index.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/MessageActions/Header.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/ActionSheet/Item.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/Header/components/HeaderTitle/index.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
app/views/DirectoryView/Options.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/RoomItem/LastMessage.tsx (1)
app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/ChangePasswordRequired.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/UserPreferencesView/ListPicker.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/SearchHeader.tsx (4)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/containers/Header/components/HeaderTitle/styles.ts (1)
styles(5-21)app/containers/TextInput/TextInput.tsx (1)
TextInput(18-29)app/lib/constants/colors.ts (1)
themes(304-304)
app/views/SetUsernameView.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/Chip/index.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/RoomInfoView/components/ItemLabel.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/InAppNotification/NotifierComponent.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/containers/message/Components/Attachments/Reply.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/containers/message/User.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/List/ListHeader.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/containers/ActionSheet/BottomSheetContent.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/RoomItem/RoomItem.tsx (1)
app/containers/RoomItem/RoomItem.stories.tsx (1)
LastMessage(106-133)
app/containers/MessageComposer/components/ComposerInput.tsx (3)
app/containers/MessageComposer/interfaces.ts (3)
IInputSelection(15-18)IComposerInput(24-31)IComposerInputProps(33-35)app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/RoomView/JoinCode.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/containers/Header/components/HeaderTitle/styles.ts (1)
styles(5-21)
app/containers/RoomItem/Tag.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/containers/CallHeader.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/NotificationPreferencesView/index.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/markdown/components/Plain.tsx (2)
app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/LoginView/UserForm.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/RoomItem/Title.tsx (4)
app/containers/RoomItem/interfaces.ts (1)
ITitleProps(30-34)app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/containers/Header/components/HeaderTitle/styles.ts (1)
styles(5-21)
app/containers/Button/index.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/markdown/components/Paragraph.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
app/containers/ActionSheet/ActionSheetContentWithInputAndSubmit/index.tsx (3)
app/containers/Header/components/HeaderTitle/styles.ts (1)
styles(5-21)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/Toast.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/markdown/components/inline/Link.tsx (4)
app/containers/markdown/components/inline/index.ts (1)
Link(6-6)app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)
app/containers/List/ListItem.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
app/views/CreateChannelView/RoomSettings/SwitchItem.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
app/views/DiscussionsView/DiscussionDetails.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/views/RoomView/LoadMore/index.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/AppVersion.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
diegolmello
left a comment
diegolmello
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
Left a few comments.
You should also solve issues reported by CodeRabbit and add screenshots/videos on your PR description.
|
thanks for the review |
…UI alignment - Fix channel header text size scaling in RoomHeader component - Add font size scaling to RoomActionsView (Actions page) title and description - Fix vertical centering of room title and description in Actions page - Add font size scaling to AudioPlayer components (Seek, PlaybackSpeed) - Add font size scaling to SelectListView info text - Fix List.Item rightContainer layout to display text and arrow horizontally - Replace MarkdownPreview with direct Text component in RoomActionsView for better alignment
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @app/containers/AudioPlayer/PlaybackSpeed.tsx:
- Around line 26-30: The accessibilityLabel for the NativeButton currently
formats playbackSpeed as "${playbackSpeed} x" (with a space) while the visible
Text shows "{playbackSpeed}x" (no space); update the accessibilityLabel call in
PlaybackSpeed (the NativeButton accessibilityLabel prop / i18n.t invocation) to
use the same formatting as the UI (e.g., `${playbackSpeed}x` or the
corresponding i18n key value) so the spoken/accessible string exactly matches
the displayed Text content.
In @app/views/FontSizePickerView/index.tsx:
- Around line 31-33: The interface IFontSizePickerViewProps references the route
name 'FontSizePickerView' which isn't declared on ProfileStackParamList; add a
matching entry to ProfileStackParamList (e.g., FontSizePickerView: undefined or
the appropriate params object) so the navigation prop type resolves, then
rebuild/type-check to confirm IFontSizePickerViewProps compiles correctly.
In @app/views/RoomActionsView/index.tsx:
- Around line 82-89: RoomTitle currently types the theme prop as string, which
allows invalid keys when indexing themes; change the prop type to the actual
theme key type (e.g., use keyof typeof themes or a shared ThemeName/ThemeType)
so the component signature becomes RoomTitle({ children, theme }: { children:
React.ReactNode; theme: keyof typeof themes }) (or use the project-wide Theme
type), then update any related usages to satisfy the stricter type; keep the
lookup themes[theme] and adjust imports if you introduce or reuse a theme type.
- Around line 91-105: RoomDescription improperly types props and uses an
unnecessary null coalescing; change the prop types so msg is string | undefined
and theme uses the same theme type as RoomTitle (replace theme: string with the
shared Theme type used by RoomTitle), remove the fallback in
usePreviewFormatText (usePreviewFormatText(msg) instead of
usePreviewFormatText(msg ?? '')), and keep the early return if !msg; update any
imports/types as needed to reference the Theme type and adjust the
RoomDescription signature accordingly.
🧹 Nitpick comments (6)
app/containers/AudioPlayer/PlaybackSpeed.tsx (1)
10-16: Avoid potential crash ifResponsiveLayoutContextis missing (unsafe destructure).If
useResponsiveLayout()can ever returnundefined(e.g., tests/storybook or a future render path outside the provider),const { scaleFontSize } = useResponsiveLayout();will throw.Proposed safer pattern (local fallback)
import NativeButton from '../NativeButton'; import { useResponsiveLayout } from '../../lib/hooks/useResponsiveLayout/useResponsiveLayout'; const PlaybackSpeed = () => { const [playbackSpeed, setPlaybackSpeed] = useUserPreferences<number>(AUDIO_PLAYBACK_SPEED, AVAILABLE_SPEEDS[1]); const { colors } = useTheme(); - const { scaleFontSize } = useResponsiveLayout(); + const responsiveLayout = useResponsiveLayout(); + const scaleFontSize = responsiveLayout?.scaleFontSize ?? ((size: number) => size);app/views/SelectListView.tsx (1)
23-30: Well-implemented responsive font sizing!The
InfoTextcomponent correctly applies responsive typography usingscaleFontSize. The implementation follows React best practices with hooks and proper style merging.♻️ Optional: Move inline style to styles object
For consistency and potential performance benefits, consider moving the inline
backgroundColorstyle to thestylesobject:-const InfoText = ({ text, theme }: { text: string; theme: TSupportedThemes }) => { - const { scaleFontSize } = useResponsiveLayout(); - return ( - <View style={{ backgroundColor: themes[theme].surfaceRoom }}> - <Text style={[styles.buttonText, { color: themes[theme].fontDefault, fontSize: scaleFontSize(16) }]}>{I18n.t(text)}</Text> - </View> - ); -}; +const InfoText = ({ text, theme }: { text: string; theme: TSupportedThemes }) => { + const { scaleFontSize } = useResponsiveLayout(); + return ( + <View style={[styles.infoContainer, { backgroundColor: themes[theme].surfaceRoom }]}> + <Text style={[styles.buttonText, { color: themes[theme].fontDefault, fontSize: scaleFontSize(16) }]}>{I18n.t(text)}</Text> + </View> + ); +};And add to the styles object:
const styles = StyleSheet.create({ + infoContainer: { + // Add any static container styles here if needed + }, buttonText: { margin: 16, ...sharedStyles.textRegular } });app/views/FontSizePickerView/index.tsx (4)
47-48: Extract hardcoded font scale threshold to a constant.The
1.3threshold is repeated twice and should be extracted to a named constant for maintainability and consistency with the codebase.♻️ Refactor to use a constant
At the top of the file, after the existing constants:
const FONT_SIZE_LABELS = { [FONT_SIZE_OPTIONS.SMALL]: 'Small', [FONT_SIZE_OPTIONS.NORMAL]: 'Normal', [FONT_SIZE_OPTIONS.LARGE]: 'Large', [FONT_SIZE_OPTIONS.EXTRA_LARGE]: 'Extra_Large' }; + +const LARGE_FONT_SCALE_THRESHOLD = 1.3;Then update lines 47-48:
const previewContextValue = { fontScale: previewFontScale, width: 0, height: 0, - isLargeFontScale: previewFontScale > 1.3, - fontScaleLimited: previewFontScale > 1.3 ? 1.3 : previewFontScale, + isLargeFontScale: previewFontScale > LARGE_FONT_SCALE_THRESHOLD, + fontScaleLimited: previewFontScale > LARGE_FONT_SCALE_THRESHOLD ? LARGE_FONT_SCALE_THRESHOLD : previewFontScale, rowHeight: 75 * previewFontScale, rowHeightCondensed: 60 * previewFontScale, scaleFontSize: previewScaleFontSize };
76-76: Remove redundant fallback after I18n.t().The
|| 'This is an example message...'fallback is redundant sinceI18n.t()already has adefaultValueparameter that returns the same string. If the translation is missing, thedefaultValuewill be used.♻️ Simplify the expression
-{I18n.t('Font_Size_Example_Message', { defaultValue: 'This is an example message to preview how the text size will look.' }) || 'This is an example message to preview how the text size will look.'} +{I18n.t('Font_Size_Example_Message', { defaultValue: 'This is an example message to preview how the text size will look.' })}
127-127: Remove redundant fallback after I18n.t().Same issue as line 76: the
|| 'Adjust the font size...'fallback is redundant since thedefaultValueparameter already provides this fallback.♻️ Simplify the expression
-{I18n.t('Font_Size_Description', { defaultValue: 'Adjust the font size for all text in the application.' }) || 'Adjust the font size for all text in the application.'} +{I18n.t('Font_Size_Description', { defaultValue: 'Adjust the font size for all text in the application.' })}
168-173: Simplify the current size label logic.Two minor improvements:
- Line 169: The trailing space after "Current size" is used for spacing, but using
marginRightor the existinggap: 8would be more semantic.- Line 172: The logic
I18n.t(currentLabel || 'Normal', {...}) ?? currentLabelis convoluted. SinceI18n.t()has adefaultValue, the?? currentLabelfallback is unnecessary.♻️ Proposed improvements
<View style={styles.currentSizeContainer}> <Text style={[styles.currentSizeLabel, { color: colors.fontHint, fontSize: scaleFontSize(12) }]}> - {I18n.t('Current_Size', { defaultValue: 'Current size' })}: + {I18n.t('Current_Size', { defaultValue: 'Current size' })}: </Text> <Text style={[styles.currentSizeValue, { color: colors.fontTitlesLabels, fontSize: scaleFontSize(14) }]}> - {I18n.t(currentLabel || 'Normal', { defaultValue: currentLabel || 'Normal' }) ?? currentLabel} + {I18n.t(currentLabel, { defaultValue: currentLabel })} </Text> </View>Note:
currentLabelalready has a fallback to'Normal'on line 114, so the extra|| 'Normal'checks are redundant.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
app/containers/AudioPlayer/PlaybackSpeed.tsxapp/containers/AudioPlayer/Seek.tsxapp/containers/List/ListItem.tsxapp/containers/RoomHeader/RoomHeader.tsxapp/views/FontSizePickerView/index.tsxapp/views/RoomActionsView/index.tsxapp/views/RoomActionsView/styles.tsapp/views/SelectListView.tsxapp/views/UserPreferencesView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/views/UserPreferencesView/index.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
app/views/FontSizePickerView/index.tsx (6)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (3)
FONT_SIZE_OPTIONS(32-37)useResponsiveLayout(85-89)ResponsiveLayoutContext(22-22)app/stacks/types.ts (1)
ProfileStackParamList(190-202)app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useAppSelector.ts (1)
useAppSelector(6-6)app/lib/methods/userPreferences.ts (1)
useUserPreferences(63-90)app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
app/containers/AudioPlayer/PlaybackSpeed.tsx (3)
app/lib/methods/userPreferences.ts (1)
useUserPreferences(63-90)app/containers/AudioPlayer/constants.ts (2)
AUDIO_PLAYBACK_SPEED(9-9)AVAILABLE_SPEEDS(1-1)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)
app/containers/AudioPlayer/Seek.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/RoomHeader/RoomHeader.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/containers/Header/components/HeaderTitle/styles.ts (1)
styles(5-21)
app/views/RoomActionsView/index.tsx (3)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
themes(304-304)app/lib/methods/helpers/helpers.ts (1)
getRoomTitle(43-60)
app/containers/List/ListItem.tsx (3)
app/containers/List/constants.ts (1)
PADDING_HORIZONTAL(1-1)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(85-89)app/lib/constants/colors.ts (1)
colors(280-302)
🔇 Additional comments (15)
app/containers/RoomHeader/RoomHeader.tsx (3)
86-117: LGTM! Font scaling correctly applied to SubTitle.The implementation properly integrates
scaleFontSizefromuseResponsiveLayoutand applies it consistently across all subtitle rendering paths (typing indicator, custom render function, and markdown preview).
119-133: Excellent accessibility improvement withisLargeFontScale.The font scaling is correctly applied, and the conditional
numberOfLineson Line 126 is a thoughtful touch—allowing 2 lines whenisLargeFontScaleis true prevents excessive truncation for users with large font preferences.
178-196: LGTM! Consistent font scaling in tmid subtitle path.The addition of
scaleFontSizeat Line 178 and its application at Line 191 ensures that the subtitle in the thread/discussion context (tmid branch) receives the same responsive font treatment as the main subtitle path, maintaining consistency across all rendering scenarios.app/views/SelectListView.tsx (2)
21-21: LGTM!The import of
useResponsiveLayoutis correctly added to support responsive font sizing in the newInfoTextcomponent.
108-111: LGTM!The
renderInfoTextmethod correctly delegates to the newInfoTextcomponent, passing the requiredtextandthemeprops.app/containers/AudioPlayer/Seek.tsx (2)
17-17: LGTM! Responsive font sizing correctly integrated.The import and hook usage follow the established pattern for implementing responsive typography across the app. The
scaleFontSizefunction is properly extracted fromuseResponsiveLayout()and ready for use in the component.Also applies to: 48-48
116-116: LGTM! Duration label now respects font size preference.The styled font size application is correct. Using
scaleFontSize(14)ensures the audio duration label text will scale appropriately based on the user's font size preference setting, fulfilling the PR's accessibility objective.app/containers/List/ListItem.tsx (4)
20-20: LGTM! Proper hook usage for responsive font sizing.The
useResponsiveLayouthook is correctly imported andscaleFontSizeis properly destructured alongside the existingfontScale. This aligns well with the PR objective of applying font size scaling across all components.Also applies to: 136-136
186-186: Font size preference takes precedence over custom styles.The dynamic
fontSize: scaleFontSize(16)is applied afterstyleTitle, which means any customfontSizepassed via thestyleTitleprop will be overridden. This ensures the user's font size preference is always respected, but it may affect consumers who were passing custom font sizes.This behavior is likely intentional for consistency with the font size preference feature.
196-196: LGTM! Subtitle styling correctly applies responsive font size.The subtitle properly combines base styles from
styles.subtitle(preservingsharedStyles.textRegular) with themed color and dynamic font size. The implementation is consistent with the title's approach.
36-39: The project uses React Native 0.79.4, which is well above the minimum version (0.71) required for thegapproperty. The code is compatible and correct.Likely an incorrect or invalid review comment.
app/views/RoomActionsView/styles.ts (1)
16-29: LGTM!The style updates appropriately support the new typography components with vertical centering and font padding control.
app/views/RoomActionsView/index.tsx (1)
815-829: Component usage looks correct.The new
RoomTitleandRoomDescriptioncomponents are integrated properly. The type safety improvements suggested in previous comments will handle potentialundefinedvalues fortopicandmember.statusText.app/views/FontSizePickerView/index.tsx (2)
182-281: LGTM! Styles are well-organized.The styles follow React Native best practices with appropriate use of flexbox, responsive padding/margins, and the newer
gapproperty for spacing. The visual hierarchy is clear with proper use of shadows and elevation for depth.
11-11: No issue found. The import statement at line 11 correctly importsFONT_SIZE_OPTIONS,ResponsiveLayoutContext, anduseResponsiveLayoutfromapp/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx. All three exports are properly defined and exported from that file, and the import path is valid.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @app/containers/Button/index.tsx:
- Around line 87-91: The disabled text color currently always uses
colors.buttonPrimaryDisabled; update the color expression in the Button render
to choose the token by button type: when isDisabled is true use isPrimary ?
colors.buttonPrimaryDisabled : colors.buttonSecondaryDisabled, otherwise keep
resolvedTextColor; modify the object that sets color (the snippet that
references isDisabled, resolvedTextColor and colors) so it mirrors the existing
background logic and uses the type-specific disabled token.
In @app/stacks/MasterDetailStack/index.tsx:
- Around line 49-50: Remove the extra ModalStack registration and import for
FontSizePickerView in MasterDetailStack: delete the import of FontSizePickerView
and the ModalStack.Screen entry that registers it (the duplicated registration
around the ModalStack/Screen block), since FontSizePickerView is already
registered as ProfileStack.Screen in InsideStack.tsx and is navigated to from
UserPreferencesView; leave all ProfileStack types and InsideStack.tsx
registration unchanged to preserve correct param types.
In @app/views/UserPreferencesView/FontSizePicker.tsx:
- Around line 56-63: The avatar can overflow because styles.avatarContainer has
a fixed width (36) while Avatar.size uses previewScaleFontSize(36); update the
container to use a dynamic width that matches the scaled avatar by changing the
View using styles.avatarContainer to combine the static style with an inline
width: { width: previewScaleFontSize(36) } (e.g., <View
style={[styles.avatarContainer, { width: previewScaleFontSize(36) }]}>), or
remove the fixed width from styles.avatarContainer; apply the same change to the
other occurrence where Avatar and styles.avatarContainer are used.
- Around line 33-50: The preview scaling is inconsistent: ExampleMessage builds
previewFontScale (systemFontScale * previewFontSize) but
previewContextValue.scaleFontSize uses previewFontSize only; update
scaleFontSize to use the computed preview scale (and respect the limited value
if desired) so sizes match previewContextValue (e.g., set scaleFontSize: (size:
number) => size * previewFontScale or size *
previewContextValue.fontScaleLimited), leaving previewFontScale,
previewContextValue and previewScaleFontSize identifiers intact.
🧹 Nitpick comments (9)
app/views/RoomActionsView/index.tsx (1)
90-104: Optimize hook invocation to skip unnecessary work.When
msgis falsy,usePreviewFormatTextis still called with an empty string, and then the component returns null, discarding the computedformattedText. Consider refactoring to avoid this wasted computation.♻️ Proposed optimization
const RoomDescription = ({ msg, theme }: { msg: string | undefined; theme: TSupportedThemes }) => { const { scaleFontSize } = useResponsiveLayout(); - const formattedText = usePreviewFormatText(msg || ''); if (!msg) { return null; } + const formattedText = usePreviewFormatText(msg); return ( <Text style={[styles.roomDescription, { color: themes[theme].fontSecondaryInfo, fontSize: scaleFontSize(13), lineHeight: scaleFontSize(18) }]} numberOfLines={1} ellipsizeMode='tail'> {formattedText} </Text> ); };Note: This moves the hook call after the null check. However, this violates the Rules of Hooks if
msgcan change between renders in a way that affects whether the hook is called. IfusePreviewFormatTextmust be called unconditionally (per Rules of Hooks), then the current implementation is correct but you might consider memoizing or optimizing the hook itself to handle empty strings efficiently.Alternatively, if the hook cannot be called conditionally, the current approach is acceptable and the hook should handle empty strings efficiently internally.
app/containers/Button/index.tsx (1)
8-9:styleTextis effectively prevented from overriding fontSize/color
Because the computed object is last intextStyle, anystyleTextpassed by callers can’t overridecolor/fontSize/lineHeight. IfstyleTextis meant to be an override escape hatch, swap the order (or selectively merge).Proposed adjustment (keep disabled styles, but allow caller overrides)
- const textStyle = [ - small ? styles.smallText : styles.text, - styleText, - { - color: isDisabled ? colors.buttonPrimaryDisabled : resolvedTextColor, - fontSize: small ? scaleFontSize(12) : scaleFontSize(fontSize), - lineHeight: small ? scaleFontSize(18) : undefined - } - ]; + const computedTextStyle = { + color: isDisabled ? colors.buttonPrimaryDisabled : resolvedTextColor, + fontSize: small ? scaleFontSize(12) : scaleFontSize(fontSize), + lineHeight: small ? scaleFontSize(18) : undefined + }; + const textStyle = [small ? styles.smallText : styles.text, computedTextStyle, styleText];Also applies to: 84-92
app/views/UserPreferencesView/index.tsx (1)
38-49: Avoid float-key mapping + re-creating labels map on every render
FONT_SIZE_LABELSis re-created each render and indexes byparseFloat(...). Consider moving the map outside the component and keying by the stored string (e.g.,'0.9' | '1' | '1.1' | '1.2') to avoid float representation edge cases (like'1.0'vs'1').app/views/UserPreferencesView/FontSizePicker.tsx (2)
89-91: Persisted value matching is brittle ('1'vs'1.0')
findIndex(opt => opt.toString() === fontSize)will fail if a previously stored value uses a different string representation. Safer: compare numbers (parseFloat(fontSize)), or normalize once.
126-138: Slider accessibility metadata is missing
Consider settingaccessibilityLabel(and potentiallyaccessibilityValue) so screen readers announce what the slider controls and the current selection.app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (4)
27-28: Clarify constant names: BASE_ROW_HEIGHT_SMALL_FONT is larger, not smaller.
BASE_ROW_HEIGHT_SMALL_FONT(82) is larger thanBASE_ROW_HEIGHT(75), which contradicts the "SMALL_FONT" label. These constants provide extra vertical space when fonts are small to prevent overly compressed layouts.Consider renaming to reflect their purpose more clearly:
BASE_ROW_HEIGHT_SMALL_FONT→BASE_ROW_HEIGHT_FOR_SMALL_FONTSorBASE_ROW_HEIGHT_MINIMUM- Or add an inline comment explaining the intentional inversion
61-65: Add comment explaining row height compensation for small fonts.The logic calculates larger base heights when
fontScale < 0.9, then multiplies by the small fontScale. This maintains minimum usable row heights but is not immediately obvious.📝 Suggested inline documentation
+ // When fontScale is very small (< 0.9), use larger base heights to prevent + // overly compressed layouts. The base is still scaled by fontScale to maintain + // proportionality, but the larger starting point ensures minimum usability. const isSmallFont = fontScale < SMALL_FONT_THRESHOLD; const baseRowHeight = isSmallFont ? BASE_ROW_HEIGHT_SMALL_FONT : BASE_ROW_HEIGHT; const baseRowHeightCondensed = isSmallFont ? BASE_ROW_HEIGHT_CONDENSED_SMALL_FONT : BASE_ROW_HEIGHT_CONDENSED; const rowHeight = baseRowHeight * fontScale; const rowHeightCondensed = baseRowHeightCondensed * fontScale;
29-29: Consider exporting SMALL_FONT_THRESHOLD for consistency.
FONT_SIZE_OPTIONSis exported (line 31) butSMALL_FONT_THRESHOLD(line 29) is not, despite both being related font size constants. If other components need to apply small-font-specific logic, exporting the threshold would maintain consistency.Also applies to: 31-36
45-54: API pattern is correct, but consider adding explicit error handling in the listener callback.The
addOnValueChangedListenerAPI returns an object with a.remove()method, and the cleanup pattern correctly removes the listener on unmount. However, whileuserPreferences.getString()has internal error handling, wrapping the listener callback logic in a try-catch would provide additional robustness:useEffect(() => { const listener = initializeStorage.addOnValueChangedListener((changedKey: string) => { try { if (changedKey === FONT_SIZE_PREFERENCES_KEY) { const newFontSize = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY) || FONT_SIZE_OPTIONS.NORMAL.toString(); setCustomFontSize(newFontSize); } } catch (error) { console.warn('Error updating font size preference:', error); } }); return () => listener.remove(); }, []);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
app/containers/AudioPlayer/PlaybackSpeed.tsxapp/containers/Button/index.tsxapp/containers/MessageComposer/components/RecordAudio/Duration.tsxapp/containers/RoomItem/Wrapper.tsxapp/containers/RoomItem/styles.tsapp/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsxapp/stacks/InsideStack.tsxapp/stacks/MasterDetailStack/index.tsxapp/stacks/MasterDetailStack/types.tsapp/stacks/types.tsapp/views/RoomActionsView/index.tsxapp/views/RoomsListView/index.tsxapp/views/SelectedUsersView/Header.tsxapp/views/UserPreferencesView/FontSizePicker.tsxapp/views/UserPreferencesView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/containers/MessageComposer/components/RecordAudio/Duration.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
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.
Applied to files:
app/views/UserPreferencesView/FontSizePicker.tsxapp/containers/AudioPlayer/PlaybackSpeed.tsx
🧬 Code graph analysis (8)
app/views/RoomActionsView/index.tsx (4)
app/theme.tsx (1)
TSupportedThemes(8-8)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(86-103)app/lib/constants/colors.ts (1)
themes(304-304)app/lib/methods/helpers/helpers.ts (1)
getRoomTitle(43-60)
app/views/UserPreferencesView/FontSizePicker.tsx (4)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (3)
FONT_SIZE_OPTIONS(31-36)useResponsiveLayout(86-103)ResponsiveLayoutContext(22-22)app/theme.tsx (1)
useTheme(29-29)app/lib/methods/userPreferences.ts (1)
useUserPreferences(63-90)app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
app/containers/Button/index.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(86-103)app/lib/constants/colors.ts (1)
colors(280-302)
app/views/UserPreferencesView/index.tsx (4)
app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (2)
useResponsiveLayout(86-103)FONT_SIZE_OPTIONS(31-36)app/lib/methods/userPreferences.ts (1)
useUserPreferences(63-90)app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
app/views/RoomsListView/index.tsx (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(86-103)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
app/containers/AudioPlayer/PlaybackSpeed.tsx (2)
app/containers/AudioPlayer/constants.ts (2)
AUDIO_PLAYBACK_SPEED(9-9)AVAILABLE_SPEEDS(1-1)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(86-103)
app/views/SelectedUsersView/Header.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(86-103)app/lib/constants/colors.ts (1)
themes(304-304)
🔇 Additional comments (18)
app/views/RoomActionsView/index.tsx (3)
15-15: LGTM: Imports are correct.The new imports support the responsive font sizing feature properly.
Also applies to: 33-33, 77-77
81-88: LGTM: RoomTitle component correctly implements dynamic font sizing.The component properly integrates the responsive layout hook and applies theme colors.
814-828: LGTM: Components are integrated correctly.The conditional rendering logic properly handles different room types, and the theme prop is correctly passed to both
RoomTitleandRoomDescription. The defensive check on line 828 ensuresmember.statusTextis only rendered when it exists.app/containers/RoomItem/styles.ts (4)
10-72: Excellent truncation-safe layout patterns.The addition of
minWidth: 0across flex containers (flex, centerContainer, title, titleText, row, titleContainer, lastMessageContainer), combined withoverflow: 'hidden'and strategicflexShrinkvalues, implements best practices for text truncation in React Native flex layouts. These constraints ensure that long text content will truncate properly rather than overflow or break the layout.Key improvements:
minWidth: 0enables nested flex children to shrink below their content sizeflexShrink: 0ondateandtagContainerprevents these UI elements from collapsing- New
titleTextstyle withflexShrink: 1allows the title to shrink when space is constrained
42-44: Verify thealignItems: 'flex-start'change doesn't introduce visual regressions.The change from implicit center alignment to
alignItems: 'flex-start'in therowstyle will affect the vertical positioning of child elements within the RoomItem rows. This may be intentional to improve alignment with dynamically scaled text, but please verify that the layout appears correct across different font size settings and doesn't introduce unexpected spacing or misalignment issues.
26-30: Hardcoded font sizes properly replaced withscaleFontSize.The removal of
fontSize: 17from thetitlestyle andfontSizefromtagTextis correctly accompanied by dynamic font scaling in the corresponding components:Title.tsxusesfontSize: scaleFontSize(17)andTag.tsxusesfontSize: scaleFontSize(13). The changes align with the PR objective and are properly implemented.
67-72: No action required. The style key rename frommarkdownTexttolastMessageContaineris not a breaking change—neither style name is referenced anywhere in the codebase. The style definition exists but remains unused.app/views/RoomsListView/index.tsx (3)
54-61: LGTM! Responsive layout integration is well-implemented.The memoized
extraDatacorrectly includes responsive metrics (rowHeight,rowHeightCondensed,fontScale) to ensure the FlatList re-renders when font scaling changes. The dependencies are complete and the conditional logic properly handles search vs. normal mode.
73-73: LGTM! useEffect dependencies are now complete.Including
navigationandstopSearchin the dependencies array correctly follows React Hook rules, as both are used within the effect callback.
143-143: LGTM! Key includes fontScale to trigger re-renders.Including
fontScalein thekeyExtractorensures all list items re-render when font size changes, which is necessary for the font scaling feature to apply immediately.app/stacks/types.ts (1)
202-202: LGTM! Navigation route type is correctly defined.The
FontSizePickerViewroute is properly added toProfileStackParamListwith no required parameters, consistent with other profile stack routes.app/stacks/MasterDetailStack/types.ts (1)
190-190: LGTM! Modal route type is correctly defined.The
FontSizePickerViewroute is properly added toModalStackParamList, ensuring the font size picker is accessible in master-detail navigation mode.app/containers/AudioPlayer/PlaybackSpeed.tsx (1)
10-10: LGTM! Responsive font sizing correctly applied.The integration of
scaleFontSizefollows the established pattern. The accessibility label improvement (removing the space before "x") makes it more natural to read.Also applies to: 15-15, 26-26, 29-29
app/stacks/InsideStack.tsx (1)
40-40: LGTM! Navigation screen is properly registered.The
FontSizePickerViewscreen is correctly added to theProfileStackNavigator, completing the navigation setup for the font size preference feature.Also applies to: 177-177
app/views/SelectedUsersView/Header.tsx (1)
12-13: Looks good: scaleFontSize applied without altering layout logic
ThescaleFontSize(12)override being last in the style array is the right precedence (keeps sharedStyles but forces responsive sizing).Also applies to: 37-50
app/views/UserPreferencesView/index.tsx (1)
138-153: > Likely an incorrect or invalid review comment.app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (2)
1-1: Verify React 19 compatibility for hooks and context usage.The code uses React hooks (useState, useEffect, useCallback, useMemo, useContext) and context patterns. Since the library context indicates React 19.0.0 is in use, confirm that:
- The hook dependency arrays are complete and won't trigger warnings in React 19's stricter Strict Mode
- The cleanup pattern in useEffect (line 53) is compatible with React 19's double-mount behavior in development
The current implementation appears correct, but given React 19's changes to Strict Mode behavior, verify this in testing.
Also applies to: 40-40, 41-54, 57-57, 67-67, 69-81
67-67: Confirmed:scaleFontSizecorrectly excludes systemfontScalefor Text component usage.The implementation on line 67 intentionally multiplies by
fontSizeMultiplier(user preference) only and excludessystemFontScale. This is correct because React Native's Text component automatically applies system font scaling tofontSizestyles. All 50+ usages across the codebase are in Text/TextInput component styles where this automatic scaling is applied, so no double-scaling occurs.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @app/containers/Button/index.tsx:
- Around line 85-93: The dynamic style object currently overrides any
fontSize/lineHeight in styleText because it comes after styleText in the
textStyle array; change the ordering so styleText is last (textStyle = [small ?
styles.smallText : styles.text, { color: ..., fontSize: ..., lineHeight: ... },
styleText]) so consumer overrides win, and update the fontSize/lineHeight logic
in that dynamic object to respect the fontSize prop and provide sensible
defaults (e.g., fontSize: scaleFontSize(small ? (fontSize ?? 12) : (fontSize ??
16)) and set lineHeight for both small and normal when not provided) by
checking/using fontSize and lineHeight values rather than hardcoding
12/undefined; modify the textStyle construction in the Button component
accordingly (symbols: textStyle, styleText, small, fontSize, lineHeight,
scaleFontSize).
In @app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx:
- Around line 68-77: fontScaleLimited is computed but never used, so rowHeight
and rowHeightCondensed still use the unbounded fontScale; update the
calculations in useResponsiveLayout to use fontScaleLimited instead of fontScale
when computing rowHeight and rowHeightCondensed (references: fontSizeMultiplier,
fontScale, fontScaleLimited, rowHeight, rowHeightCondensed, FONT_SCALE_LIMIT) so
the FONT_SCALE_LIMIT is actually enforced, or if you prefer unbounded scaling
remove fontScaleLimited and all references to FONT_SCALE_LIMIT consistently.
🧹 Nitpick comments (3)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (3)
29-29: Consider exportingSMALL_FONT_THRESHOLDfor consistency.While currently only used internally, this threshold might be useful for other components that need to detect small font scenarios. Consider exporting it alongside the other constants for API completeness.
♻️ Proposed refactor
-const SMALL_FONT_THRESHOLD = 0.9; +export const SMALL_FONT_THRESHOLD = 0.9;
41-49: Extract duplicate parsing logic into a helper function.The font size parsing logic (getNumber fallback to getString with Number parsing) is duplicated in lines 41-49 and lines 54-61. Extract this into a reusable helper function to improve maintainability.
♻️ Proposed refactor
Add a helper function before the component:
+const parseFontSizePreference = (): number => { + const storedNumber = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); + if (typeof storedNumber === 'number' && !Number.isNaN(storedNumber)) { + return storedNumber; + } + const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); + const parsed = storedString !== null ? Number(storedString) : undefined; + return !Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL; +}; + const ResponsiveLayoutProvider = ({ children }: IResponsiveFontScaleProviderProps) => { const { fontScale: systemFontScale, width, height } = useWindowDimensions(); - const [customFontSize, setCustomFontSize] = useState(() => { - const storedNumber = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); - if (typeof storedNumber === 'number' && !Number.isNaN(storedNumber)) { - return storedNumber; - } - const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); - const parsed = storedString !== null ? Number(storedString) : undefined; - return !Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL; - }); + const [customFontSize, setCustomFontSize] = useState(parseFontSizePreference);Then update the listener:
useEffect(() => { const listener = initializeStorage.addOnValueChangedListener((changedKey: string) => { if (changedKey === FONT_SIZE_PREFERENCES_KEY) { - const newValue = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); - if (typeof newValue === 'number' && !Number.isNaN(newValue)) { - setCustomFontSize(newValue); - return; - } - const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); - const parsed = storedString !== null ? Number(storedString) : undefined; - setCustomFontSize(!Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL); + setCustomFontSize(parseFontSizePreference()); } });
101-112: Consider adding a development warning for missing provider.The fallback logic prevents crashes when the hook is used outside the provider, which is good defensive programming. However, silently returning defaults might hide integration issues during development.
♻️ Proposed enhancement
if (!context || Object.keys(context).length === 0) { + if (__DEV__) { + console.warn('useResponsiveLayout must be used within ResponsiveLayoutProvider. Using default values.'); + } return { fontScale: 1.0,
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
app/containers/Button/index.tsxapp/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsxapp/views/FontSizePickerView/index.tsxapp/views/UserPreferencesView/FontSizePicker.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- app/views/UserPreferencesView/FontSizePicker.tsx
- app/views/FontSizePickerView/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
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.
Applied to files:
app/containers/Button/index.tsx
🧬 Code graph analysis (2)
app/containers/Button/index.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(98-115)app/lib/constants/colors.ts (1)
colors(280-302)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
🔇 Additional comments (6)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (3)
51-66: LGTM: Effect properly manages storage listener lifecycle.The effect correctly sets up the storage listener on mount and cleans it up on unmount. The empty dependency array is appropriate since the listener should persist for the component's lifetime.
79-79: LGTM: Proper useCallback implementation.The callback correctly memoizes the font scaling function with the appropriate dependency, ensuring stable references and optimal re-render behavior.
81-93: LGTM: Context value properly memoized.The context value is correctly memoized with all relevant dependencies, ensuring consumers only re-render when necessary.
app/containers/Button/index.tsx (3)
43-45: LGTM: Font properties moved to dynamic styling.Removing static fontSize and lineHeight from the style object allows the component to apply font size scaling dynamically, which is the correct approach for this feature.
66-69: LGTM: Clean hook integration.The hook is properly integrated and the disabled text color computation correctly varies by button type.
90-91: Clarify fontSize prop behavior for small buttons and lineHeight for normal buttons.Two potentially unexpected behaviors:
Line 90: When
small={true}, thefontSizeprop is ignored and always defaults to 12 (scaled). This might surprise developers who pass a custom fontSize expecting it to apply.Line 91:
lineHeightis only set for small buttons. Normal buttons getundefined, which relies on the default line height computation. This asymmetry might cause inconsistent text rendering.Please verify:
- Is ignoring the
fontSizeprop for small buttons intentional? Consider documenting this in the interface or applying:small ? scaleFontSize(12) : scaleFontSize(fontSize)- Should normal buttons also receive an explicit
lineHeightfor consistency?
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx:
- Around line 68-82: scaleFontSize currently multiplies by fontSizeMultiplier
only, ignoring system accessibility scaling; update the useCallback for
scaleFontSize to use fontScale (which includes systemFontScale *
fontSizeMultiplier) and update its dependency array to [fontScale] so scaled
text respects both system and custom font settings; locate the scaleFontSize
definition and replace the multiplier to use fontScale instead of
fontSizeMultiplier and adjust dependencies accordingly.
🧹 Nitpick comments (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
41-49: Extract storage retrieval logic to eliminate duplication.The font size retrieval logic (getNumber with fallback to getString parsing) is duplicated in lines 54-61. This creates maintenance burden and increases risk of inconsistency.
♻️ Proposed refactor to extract common logic
+const retrieveFontSizeFromStorage = (): number => { + const storedNumber = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); + if (typeof storedNumber === 'number' && !Number.isNaN(storedNumber)) { + return storedNumber; + } + const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); + const parsed = storedString !== null ? Number(storedString) : undefined; + return !Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL; +}; + const ResponsiveLayoutProvider = ({ children }: IResponsiveFontScaleProviderProps) => { const { fontScale: systemFontScale, width, height } = useWindowDimensions(); - const [customFontSize, setCustomFontSize] = useState(() => { - const storedNumber = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); - if (typeof storedNumber === 'number' && !Number.isNaN(storedNumber)) { - return storedNumber; - } - const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); - const parsed = storedString !== null ? Number(storedString) : undefined; - return !Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL; - }); + const [customFontSize, setCustomFontSize] = useState(retrieveFontSizeFromStorage); useEffect(() => { const listener = initializeStorage.addOnValueChangedListener((changedKey: string) => { if (changedKey === FONT_SIZE_PREFERENCES_KEY) { - const newValue = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); - if (typeof newValue === 'number' && !Number.isNaN(newValue)) { - setCustomFontSize(newValue); - return; - } - const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); - const parsed = storedString !== null ? Number(storedString) : undefined; - setCustomFontSize(!Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL); + setCustomFontSize(retrieveFontSizeFromStorage()); } });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/containers/Button/index.tsxapp/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
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.
Applied to files:
app/containers/Button/index.tsx
🧬 Code graph analysis (1)
app/containers/Button/index.tsx (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(101-118)app/lib/constants/colors.ts (1)
colors(280-302)
🔇 Additional comments (4)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (2)
15-15: LGTM! Clean API extension for font scaling.The additions are well-structured:
scaleFontSizefunction properly typed in the interfaceFONT_SIZE_OPTIONSprovides clear preset values- Small font threshold aligns with the SMALL option (0.9)
- Alternative base row heights maintain layout proportions when scaling
Also applies to: 27-36
84-96: LGTM! Context memoization and fallback are correct.
- Dependencies array properly includes
scaleFontSizeto trigger updates when the multiplier changes- Fallback provides safe defaults, including identity function for
scaleFontSize- All interface properties are satisfied
Also applies to: 104-115
app/containers/Button/index.tsx (2)
8-8: LGTM! Clean integration of responsive font sizing.The changes correctly:
- Import the layout hook for font scaling
- Remove static font sizing from
smallTextstyle (now handled dynamically)- Extract
scaleFontSizefor use in text styling- Compute disabled text color based on button type
Also applies to: 44-44, 66-66, 69-69
85-93: The edge case you identified is real (there's actual usage in ChangePassword.tsx with both props), but it's not problematic. Thesmallprop is a preset variant by design—it always uses fixed sizing (fontSize: scaleFontSize(12),lineHeight: scaleFontSize(18)) and intentionally ignores any customfontSizeprop. This is working as intended.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx:
- Around line 41-49: The font-size retrieval logic duplicated between the
useState initializer (customFontSize) and the storage listener should be
extracted into a single helper function (e.g., getStoredFontSize) that
encapsulates reading from userPreferences using FONT_SIZE_PREFERENCES_KEY,
parsing numeric/string values, validating with Number.isNaN, and falling back to
FONT_SIZE_OPTIONS.NORMAL; replace the inline logic in the useState initializer
and the storage listener callback with calls to this helper and call
setCustomFontSize(getStoredFontSize()) where appropriate so both initialization
and updates share the same, tested logic.
🧹 Nitpick comments (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (2)
68-69: Consider documenting the multiplicative font scaling behavior.The code multiplies
systemFontScalebyfontSizeMultiplier(line 69), meaning these scales compound. For example, if a user has system accessibility set to 1.5× and selects "Large" (1.1), the result is 1.65× total scaling.While this aligns with the commit message "respect system accessibility settings," this behavior might be unexpected. Consider adding a comment explaining this design decision.
📝 Suggested comment
+ // Compound system accessibility scale with user preference to respect both settings + // e.g., system 1.5× × custom 1.1× = 1.65× total const fontSizeMultiplier = typeof customFontSize === 'number' && !Number.isNaN(customFontSize) ? customFontSize : 1.0; const fontScale = systemFontScale * fontSizeMultiplier;
27-29: This row height logic is intentional and correct—no changes needed.When
fontScaledrops below theSMALL_FONT_THRESHOLD(0.9), the code intentionally uses a larger base row height (82 vs 75) to ensure rows don't become cramped. For example, at fontScale 0.8, the small font base yields 65.6 units versus 60 with the normal base—a deliberate spacing buffer for accessibility. This design prevents UI crowding when users have reduced font scaling.That said, a brief comment explaining this behavior would improve code clarity, since the naming alone could suggest the opposite intent.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
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.
Applied to files:
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧬 Code graph analysis (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
🔇 Additional comments (4)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (4)
1-16: LGTM: Interface and imports are well-structured.The additions to the interface and imports properly support the new font scaling functionality.
84-99: LGTM: Context value is properly memoized.The context value is correctly memoized with all necessary dependencies, and the provider implementation is sound.
104-115: LGTM: Fallback values provide safe defaults.The hook correctly handles missing context by returning sensible default values, preventing runtime errors when used outside the provider.
51-66: Storage initialization is synchronous and ready before component mount—no race condition exists.The
userPreferencesandinitializeStorageobjects are created synchronously at module load time (lines 61 and 165 in userPreferences.ts), not asynchronously. The MMKV instance is fully initialized when the module is imported, so both are available and safe to use when this component mounts. All methods (getString,getNumber,addOnValueChangedListener) are synchronous with proper error handling.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx:
- Around line 38-46: getFontSizeFromStorage currently handles both string and
number formats; standardize font-size storage to a number across the app by
updating the writers (UserPreferencesView/index.tsx,
UserPreferencesView/FontSizePicker.tsx, and FontSizePickerView/index.tsx) to
persist FONT_SIZE_PREFERENCES_KEY as a numeric value via
userPreferences.setNumber, then simplify getFontSizeFromStorage to read only
userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY) and fall back to
FONT_SIZE_OPTIONS.NORMAL.
🧹 Nitpick comments (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (2)
38-46: Consider adding validation for font size values.The
getFontSizeFromStoragefunction reads from storage but doesn't validate that the value is within the expected range (0.9-1.2 based on FONT_SIZE_OPTIONS). Consider adding validation to constrain values:const value = /* ...existing logic... */; return Math.min(Math.max(value, 0.5), 2.0); // Reasonable min/max boundsThis prevents extreme scaling if invalid values are stored.
63-64: Simplify the font size multiplier fallback.The validation on line 63 is redundant—
getFontSizeFromStorage()already returns a valid number or 1.0. You can simplify:-const fontSizeMultiplier = typeof customFontSize === 'number' && !Number.isNaN(customFontSize) ? customFontSize : 1.0; +const fontSizeMultiplier = customFontSize;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
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.
Applied to files:
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧬 Code graph analysis (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
🔇 Additional comments (6)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (6)
27-36: LGTM! Constants are well-defined.The new base row height constants for small fonts and the FONT_SIZE_OPTIONS preset values are clearly defined and appropriately exported for use across the codebase.
77-77: LGTM! Correct useCallback implementation.The
scaleFontSizecallback correctly depends onfontScaleand will update whenever the font scale changes, ensuring consumers always get the current scaling factor.
79-91: LGTM! Proper memoization with complete dependencies.The context value is correctly memoized with all dependencies listed, preventing unnecessary re-renders while ensuring updates propagate when values change.
99-110: LGTM! Proper fallback when context is unavailable.The hook correctly detects when it's used outside a provider (empty context object) and returns sensible default values, including a no-op
scaleFontSizefunction. This prevents crashes when the hook is used without the provider.
63-72: [rewritten review comment]
[classification tag]
53-61: Storage listener cleanup is correctly implemented.The code properly handles listener cleanup:
initializeStorage.addOnValueChangedListener()returns an object with a.remove()method (confirmed by the mock implementation and type annotations used elsewhere in the codebase likeapp/lib/methods/helpers/theme.ts). The cleanup function correctly unsubscribes by callinglistener.remove(), preventing memory leaks and ensuring the listener doesn't fire after component unmount.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx:
- Around line 72-84: The computed fontScale (fontScale = systemFontScale *
fontSizeMultiplier) is unbounded and used by scaleFontSize, allowing text to
exceed FONT_SCALE_LIMIT; change the logic so that the capped value is used for
text sizing: compute fontScaleLimited = Math.min(systemFontScale *
fontSizeMultiplier, FONT_SCALE_LIMIT) (or apply fontScaleLimited before any use)
and update scaleFontSize to reference fontScaleLimited instead of raw fontScale,
while keeping rowHeight and rowHeightCondensed calculations using the same
capped value.
🧹 Nitpick comments (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
38-55: Consider optimizing storage writes during migration.The type normalization logic is correct and handles the migration from string to numeric storage gracefully. However, lines 48 and 53 write to storage on every read when the value isn't already numeric. During migration, this could cause a storage write on every app launch or preference read.
Consider tracking whether normalization has occurred to avoid repeated writes, or accept this as acceptable migration overhead.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsxapp/views/UserPreferencesView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/views/UserPreferencesView/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
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.
Applied to files:
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧬 Code graph analysis (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
🔇 Additional comments (4)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (4)
1-16: LGTM! Interface and imports properly extended.The new imports support storage-backed preferences and the
scaleFontSizefunction signature correctly extends the context interface.
86-102: LGTM! Proper memoization prevents unnecessary re-renders.The
scaleFontSizecallback and context value are correctly memoized with appropriate dependencies.
108-119: LGTM! Fallback defaults improve initialization robustness.The default values prevent crashes when the hook is used before the provider mounts, with sensible identity/base values.
27-29: This behavior is intentional and appropriate for accessibility.When
fontScale < 0.9, using larger base row heights (82 vs 75, and 68 vs 60) prevents rows from becoming too compressed when users apply small font scales. For example, withfontScale = 0.8, this yields65.6vs60pixels—maintaining minimum visual space and touch targets. This is a valid accessibility pattern, not an error.Consider adding test coverage for the small font scenario (fontScale < 0.9) to document this behavior.
- Use native number storage in useUserPreferences for numeric values - Add findClosestOption to handle floating point precision in label lookup - Apply fontScaleLimited to scaleFontSize to prevent unbounded scaling - Fix current size label showing correctly in FontSizePickerView and UserPreferencesView
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx:
- Around line 38-55: getFontSizeFromStorage currently accepts any numeric value
from storage; add bounds validation to reject or clamp invalid sizes (zero,
negative, or excessively large). After parsing the numeric value in
getFontSizeFromStorage, check it against allowed min/max (e.g.,
FONT_SIZE_OPTIONS.MIN and FONT_SIZE_OPTIONS.MAX or use
FONT_SIZE_OPTIONS.SMALL/NORMAL/LARGE constants) and if out of range, set and
return FONT_SIZE_OPTIONS.NORMAL (or clamp to the nearest allowed value); ensure
you still call userPreferences.setNumber(FONT_SIZE_PREFERENCES_KEY,
<validatedNumber>) to normalize stored type. Update only getFontSizeFromStorage
and any constants/enums for min/max values if needed; preserve existing behavior
of falling back to FONT_SIZE_OPTIONS.NORMAL when no valid value exists.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsxapp/lib/methods/userPreferences.tsapp/views/FontSizePickerView/index.tsxapp/views/UserPreferencesView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- app/views/UserPreferencesView/index.tsx
- app/views/FontSizePickerView/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
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.
Applied to files:
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧬 Code graph analysis (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
🔇 Additional comments (8)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (6)
27-29: Verify the small-font base height logic.When
fontScale < 0.9(small font), the code applies larger base row heights (BASE_ROW_HEIGHT_SMALL_FONT = 82vs.BASE_ROW_HEIGHT = 75). This appears counterintuitive—why do smaller fonts require more vertical space?If this ensures minimum touch targets per iOS (44pt) and Android (48dp) guidelines even with reduced text size, that's excellent accessible design. However, please confirm this is intentional and document the reasoning in a comment.
📝 Suggested documentation
+// When font scale is small, use larger base heights to maintain minimum +// touch target sizes (iOS: 44pt, Android: 48dp) for accessibility. const SMALL_FONT_THRESHOLD = 0.9;Also applies to: 74-76
59-73: Verify accessibility implications of compounding font scales with absolute cap.Line 73 compounds
systemFontScale * fontSizeMultiplier, then caps the result atFONT_SCALE_LIMIT = 1.3. This means a user with system accessibility settings at 1.5× who also selects EXTRA_LARGE (1.2×) would expect 1.8× but only receives 1.3×, potentially undermining their accessibility needs.If the cap is necessary to prevent layout breakage, consider:
- Documenting this tradeoff
- Using dynamic layouts that accommodate larger scales
- Respecting system accessibility settings by applying the cap only to the custom multiplier
Based on learnings, accessibility is important to this codebase (iOS VoiceOver support). Confirm whether limiting users' chosen accessibility settings aligns with your accessibility goals.
62-70: LGTM: Storage listener implementation.The storage change listener correctly updates
customFontSizestate and properly cleans up on unmount.
86-87: LGTM:scaleFontSizeimplementation.Using
fontScaleLimitedprevents unbounded scaling and avoids compound scaling issues. TheuseCallbackensures stable function identity.
89-101: LGTM: Context value memoization.All dependencies are correctly listed in the
useMemodependency array, preventing unnecessary re-renders.
109-120: LGTM: Defensive fallback context.Provides sensible defaults when the context is unavailable, including a no-op
scaleFontSize. Good defensive programming.app/lib/methods/userPreferences.ts (2)
64-102: LGTM: Numeric storage with migration.The implementation correctly:
- Prefers native numeric storage for number defaults (lines 71-88)
- Migrates legacy string-stored numbers to numeric storage (lines 77-88)
- Falls back to string storage for non-numeric defaults (lines 89-102)
- Handles JSON parsing for complex types
The type-based branching assumes keys are consistently used with the same default type, which is a reasonable constraint for user preference keys.
104-121: LGTM: setValue handles both storage types correctly.The logic appropriately:
- Clears both storages for numeric types (supporting migration cleanup, lines 106-109)
- Clears only string storage for non-numeric types (line 109)
- Stores numbers in native numeric storage and cleans up legacy string storage (lines 110-115)
- Stores strings and JSON-serialized objects in string storage (lines 116-120)
The asymmetry in clearing is correct: numeric types might have old string representations from pre-migration storage, while non-numeric types never use numeric storage.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (2)
41-49: Extract storage reading logic to reduce duplication.The logic for reading and normalizing the font size from storage (lines 41-49) is duplicated in the storage listener (lines 54-62). Extract this into a helper function to improve maintainability.
♻️ Proposed refactor
+const readFontSizeFromStorage = (): number => { + const storedNumber = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); + if (typeof storedNumber === 'number' && !Number.isNaN(storedNumber)) { + return storedNumber; + } + const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); + const parsed = storedString !== null ? Number(storedString) : undefined; + return !Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL; +}; + const ResponsiveLayoutProvider = ({ children }: IResponsiveFontScaleProviderProps) => { const { fontScale: systemFontScale, width, height } = useWindowDimensions(); - const [customFontSize, setCustomFontSize] = useState(() => { - const storedNumber = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); - if (typeof storedNumber === 'number' && !Number.isNaN(storedNumber)) { - return storedNumber; - } - const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); - const parsed = storedString !== null ? Number(storedString) : undefined; - return !Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL; - }); + const [customFontSize, setCustomFontSize] = useState(readFontSizeFromStorage); useEffect(() => { const listener = initializeStorage.addOnValueChangedListener((changedKey: string) => { if (changedKey === FONT_SIZE_PREFERENCES_KEY) { - const newValue = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); - if (typeof newValue === 'number' && !Number.isNaN(newValue)) { - setCustomFontSize(newValue); - return; - } - const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); - const parsed = storedString !== null ? Number(storedString) : undefined; - setCustomFontSize(!Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL); + setCustomFontSize(readFontSizeFromStorage()); } }); return () => listener.remove(); }, []);
51-66: Add error handling for storage operations.The storage listener and initialization code don't handle potential errors from
userPreferencesoperations. If storage access fails, it could crash the provider during initialization or when preferences change.🛡️ Proposed error handling
const [customFontSize, setCustomFontSize] = useState(() => { + try { const storedNumber = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); if (typeof storedNumber === 'number' && !Number.isNaN(storedNumber)) { return storedNumber; } const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); const parsed = storedString !== null ? Number(storedString) : undefined; return !Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL; + } catch (error) { + console.warn('Failed to read font size preference:', error); + return FONT_SIZE_OPTIONS.NORMAL; + } }); useEffect(() => { const listener = initializeStorage.addOnValueChangedListener((changedKey: string) => { if (changedKey === FONT_SIZE_PREFERENCES_KEY) { + try { const newValue = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); if (typeof newValue === 'number' && !Number.isNaN(newValue)) { setCustomFontSize(newValue); return; } const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); const parsed = storedString !== null ? Number(storedString) : undefined; setCustomFontSize(!Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL); + } catch (error) { + console.warn('Failed to read font size preference:', error); + setCustomFontSize(FONT_SIZE_OPTIONS.NORMAL); + } } }); return () => listener.remove(); }, []);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsxapp/views/FontSizePickerView/index.tsxapp/views/UserPreferencesView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- app/views/UserPreferencesView/index.tsx
- app/views/FontSizePickerView/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
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.
Applied to files:
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧬 Code graph analysis (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
🔇 Additional comments (6)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (6)
15-15: LGTM: scaleFontSize addition to context.The
scaleFontSizefunction is a well-designed addition to the responsive layout context, enabling components to scale font sizes consistently based on user preferences.
27-29: Verify small font row heights are larger than normal.The base row heights for small fonts (82, 68) are larger than normal row heights (75, 60). This seems counterintuitive—typically smaller fonts would allow for more compact layouts. Please confirm this is intentional (perhaps to maintain touch target sizes) and consider adding a comment explaining the rationale.
68-69: Verify font scale multiplication behavior with users.The font scale calculation multiplies the system font scale by the user's font size preference (line 69). This means the two scales compound rather than the user preference overriding the system scale. For example, if a user has system font scale of 1.5 and selects "Large" (1.1), the result is 1.65x scaling.
Confirm this is the intended UX—users might expect their preference to replace rather than multiply the system setting.
82-96: LGTM: Well-structured memoization.The
scaleFontSizecallback is properly memoized withuseCallback, and the context value is correctly memoized with all dependencies. This ensures optimal performance and prevents unnecessary re-renders.
104-116: Defensive fallback is good practice.The empty context check provides a safe fallback when the hook is used outside the provider. While the provider should always wrap consuming components, this defensive programming prevents potential crashes during development or refactoring.
29-29: SMALL_FONT_THRESHOLD is only used internally within this file and does not need to be exported.The constant is defined and used only once more on line 70 within the same file. The value
0.9that is used for font size decisions elsewhere in the codebase is already exported as part ofFONT_SIZE_OPTIONS.SMALL. No external imports ofSMALL_FONT_THRESHOLDexist.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx:
- Around line 41-66: The font-size parsing logic duplicated in the useState
initializer and in the initializeStorage listener should be moved into a small
helper function (e.g., parseStoredFontSize) so both places call it; implement a
function that reads userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY) and
falls back to parsing userPreferences.getString(FONT_SIZE_PREFERENCES_KEY)
returning a valid number or FONT_SIZE_OPTIONS.NORMAL, then replace the inline
logic in the useState initialiser and inside the
initializeStorage.addOnValueChangedListener callback to call parseStoredFontSize
and pass its result to setCustomFontSize.
🧹 Nitpick comments (4)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (4)
27-36: Consider exporting SMALL_FONT_THRESHOLD for consistency.Lines 27-28 export the new base row height constants, but line 29's
SMALL_FONT_THRESHOLDis not exported. For consistency and potential reuse, consider exporting it:-const SMALL_FONT_THRESHOLD = 0.9; +export const SMALL_FONT_THRESHOLD = 0.9;
68-68: Redundant validation of customFontSize.Since
customFontSizeis validated in both the initial state (lines 41-49) and the listener (lines 51-66), the validation at line 68 is redundant:- const fontSizeMultiplier = typeof customFontSize === 'number' && !Number.isNaN(customFontSize) ? customFontSize : 1.0; + const fontSizeMultiplier = customFontSize;Alternatively, if you prefer defensive programming, this is acceptable but consider whether the state guarantees are sufficient.
102-113: Consider using FONT_SIZE_OPTIONS.NORMAL for consistency.The fallback object uses hardcoded
1.0forfontScaleandfontScaleLimited. For consistency with the rest of the codebase, consider usingFONT_SIZE_OPTIONS.NORMAL:if (!context || Object.keys(context).length === 0) { return { - fontScale: 1.0, + fontScale: FONT_SIZE_OPTIONS.NORMAL, width: 0, height: 0, isLargeFontScale: false, - fontScaleLimited: 1.0, + fontScaleLimited: FONT_SIZE_OPTIONS.NORMAL, rowHeight: BASE_ROW_HEIGHT, rowHeightCondensed: BASE_ROW_HEIGHT_CONDENSED, scaleFontSize: (size: number) => size }; }
41-66: Add error handling for storage operations.The storage operations (lines 42-48, 54-61) lack error handling. If
userPreferencesmethods throw orinitializeStorage.addOnValueChangedListenerfails, the component will crash.Consider wrapping storage operations in try-catch blocks:
🛡️ Add defensive error handling
const [customFontSize, setCustomFontSize] = useState(() => { + try { const storedNumber = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); if (typeof storedNumber === 'number' && !Number.isNaN(storedNumber)) { return storedNumber; } const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); const parsed = storedString !== null ? Number(storedString) : undefined; return !Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL; + } catch (error) { + console.warn('Failed to read font size from storage:', error); + return FONT_SIZE_OPTIONS.NORMAL; + } }); useEffect(() => { + try { const listener = initializeStorage.addOnValueChangedListener((changedKey: string) => { if (changedKey === FONT_SIZE_PREFERENCES_KEY) { + try { const newValue = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); if (typeof newValue === 'number' && !Number.isNaN(newValue)) { setCustomFontSize(newValue); return; } const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); const parsed = storedString !== null ? Number(storedString) : undefined; setCustomFontSize(!Number.isNaN(parsed ?? NaN) ? (parsed as number) : FONT_SIZE_OPTIONS.NORMAL); + } catch (error) { + console.warn('Failed to update font size from storage:', error); + } } }); return () => listener.remove(); + } catch (error) { + console.warn('Failed to initialize font size listener:', error); + } }, []);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
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.
Applied to files:
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧬 Code graph analysis (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
🔇 Additional comments (6)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (6)
1-5: LGTM!The additional React hooks and storage utilities are appropriately imported for the font size customization feature.
15-15: LGTM!The scaleFontSize method signature correctly describes the font scaling utility.
70-78: LGTM!The small-font detection and row height calculations correctly adjust base heights for better layout at small font scales and apply appropriate clamping.
80-80: LGTM!The
scaleFontSizecallback is correctly memoized with the appropriate dependency.
82-94: LGTM!The context value is properly memoized with all dependencies included.
69-69: The multiplicative font scaling behavior (combiningsystemFontScalefrom device accessibility withfontSizeMultiplierfrom app preferences) is intentional. The design keeps these sources separate—device setting vs. user preference—and combines them to allow both to apply. For example, if the user has system accessibility set to 1.5× and selects app preference LARGE (1.1×), the result of 1.65× correctly reflects both settings. The implementation includes appropriate safeguards: a cap at 1.3× to prevent excessive scaling and consistent application across all layout calculations.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (4)
29-29: Consider exporting SMALL_FONT_THRESHOLD for consistency.The constant is used internally to determine when small-font layout adjustments apply. Exporting it would allow other components to reference the same threshold if they need font-scale-aware behavior.
📤 Proposed export
-const SMALL_FONT_THRESHOLD = 0.9; +export const SMALL_FONT_THRESHOLD = 0.9;
63-63: Remove redundant validation.Since
getFontSizeFromStorage()already ensures a valid number is returned (with FONT_SIZE_OPTIONS.NORMAL as fallback), the NaN and type checks here are redundant.♻️ Proposed simplification
- const fontSizeMultiplier = typeof customFontSize === 'number' && !Number.isNaN(customFontSize) ? customFontSize : 1.0; + const fontSizeMultiplier = customFontSize;
65-71: Add comments to explain counterintuitive row height logic.The code uses larger base row heights when
fontScale < 0.9, which seems counterintuitive. This appears intentional—to prevent rows from becoming too cramped when fonts are very small—but lacks explanation. Future maintainers might misinterpret this as a bug.💬 Proposed clarifying comments
+ // When fontScale is very small (< 0.9), use larger base heights to maintain minimum + // row spacing and prevent overly cramped layouts. The larger base partially compensates + // for the small scale factor (e.g., 82 * 0.85 ≈ 70, closer to normal 75 * 1.0). const isSmallFont = fontScale < SMALL_FONT_THRESHOLD; const baseRowHeight = isSmallFont ? BASE_ROW_HEIGHT_SMALL_FONT : BASE_ROW_HEIGHT; const baseRowHeightCondensed = isSmallFont ? BASE_ROW_HEIGHT_CONDENSED_SMALL_FONT : BASE_ROW_HEIGHT_CONDENSED; const scaledRowHeight = baseRowHeight * fontScale; const scaledRowHeightCondensed = baseRowHeightCondensed * fontScale; + // Clamp to prevent excessive row heights when fontScale exceeds the limit const rowHeight = Math.min(scaledRowHeight, baseRowHeight * FONT_SCALE_LIMIT); const rowHeightCondensed = Math.min(scaledRowHeightCondensed, baseRowHeightCondensed * FONT_SCALE_LIMIT);
75-75: Consider whether scaleFontSize should apply bounds.The function multiplies by
fontScalewithout limits, while row heights use clamping viaFONT_SCALE_LIMIT. This could produce extreme scaled values (e.g.,fontScale = 2.0→ 200px from 100px input).Verify whether unbounded scaling is intentional—allowing consumers to decide limits—or if
scaleFontSizeshould usefontScaleLimitedfor consistency with row height calculations.♻️ Alternative: Apply bounds in scaleFontSize
If bounding is desired:
- const scaleFontSize = useCallback((size: number): number => size * fontScale, [fontScale]); + const scaleFontSize = useCallback((size: number): number => size * fontScaleLimited, [fontScaleLimited]);Update dependencies in the context value memoization accordingly if you make this change.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
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.
Applied to files:
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧬 Code graph analysis (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
🔇 Additional comments (3)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (3)
1-5: LGTM!The new imports are properly used throughout the implementation for state management, memoization, and storage persistence.
97-108: LGTM!The fallback provides safe defaults that match the interface shape, preventing crashes when the hook is called outside the provider context. The identity function for
scaleFontSizeis a sensible no-op default.
38-46: The dual-type handling is necessary due to storage serialization inconsistency.The
getFontSizeFromStoragefunction handles both number and string types becauseuseUserPreferencesinternally serializes non-string types to JSON before storing them via MMKV. Components writing viauseUserPreferences<number>()store JSON-serialized numbers, while theUserPreferences.getNumber()method expects native MMKV numbers. The fallback togetString()and manual parsing correctly handles both formats.Note:
UserPreferencesViewincorrectly usesuseUserPreferences<string>()for this numeric preference; it should useuseUserPreferences<number>()for consistency, though this component only reads the value, so it doesn't cause write conflicts.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx:
- Around line 38-49: getFontSizeFromStorage currently only checks for >0 and
finiteness, allowing unreasonably large values; introduce a MAX_FONT_SIZE
constant (e.g., 48 or a value consistent with FONT_SIZE_OPTIONS) and validate
that both storedNumber and parsed are <= MAX_FONT_SIZE (or clamp them to
MAX_FONT_SIZE) before returning; update the checks in getFontSizeFromStorage to
enforce this upper bound and fall back to FONT_SIZE_OPTIONS.NORMAL if the value
is out of range, referencing FONT_SIZE_PREFERENCES_KEY and FONT_SIZE_OPTIONS in
the change.
- Around line 56-64: The effect that registers
initializeStorage.addOnValueChangedListener and calls setCustomFontSize is
unsafe for concurrent React because the listener closes over render state and
can cause tearing; refactor to use useSyncExternalStore: subscribe by returning
the listener remover that calls the provided callback only when changedKey ===
FONT_SIZE_PREFERENCES_KEY, provide getFontSizeFromStorage as the snapshot and
serverSnapshot selectors, and replace the local state/setCustomFontSize usage
with the value returned from useSyncExternalStore; if useSyncExternalStore isn’t
feasible, keep a ref of the latest font size (updated in an effect) and wrap
setCustomFontSize calls in startTransition inside the listener to avoid stale
closures and make updates non-blocking.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
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.
Applied to files:
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧬 Code graph analysis (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
🔇 Additional comments (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
100-111: The fallback returnswidth: 0andheight: 0when the context is unavailable, but this is an appropriate safety guard for an edge case that should never occur in production. TheResponsiveLayoutProvideris required at the application root and enforces the context throughout the component tree. When the hook is used normally,widthandheightcome directly from React Native'suseWindowDimensions(), which always returns actual device dimensions—never zero. The actual usage of these values (e.g.,const portrait = height > widthin RoomHeader) is safe with zero values and incurs no risk of division-by-zero errors. The jest setup deliberately omitswidthandheightfrom the mock, confirming they are not critical to core functionality. No changes needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (2)
38-50: Consider aligningMAX_FONT_SIZEwithFONT_SIZE_OPTIONSor documenting the discrepancy.
MAX_FONT_SIZE(5.0) is significantly higher thanFONT_SIZE_OPTIONS.EXTRA_LARGE(1.2). While this provides a defensive upper bound against corrupted storage values, it allows persisted values outside the UI's selectable range (0.9–1.2). If a user somehow stores 3.0, it would be accepted as valid.Consider either:
- Lowering
MAX_FONT_SIZEcloser to the actual max option (e.g., 1.5 with some buffer), or- Adding a comment explaining the defensive reasoning.
Also, the
as numbercast on line 47 is redundant sinceparsedis already validated as a number.Suggested improvement
const getFontSizeFromStorage = (): number => { - const MAX_FONT_SIZE = 5.0; + // Defensive upper bound - actual UI options max at 1.2 (EXTRA_LARGE) + const MAX_FONT_SIZE = 1.5; const storedNumber = userPreferences.getNumber(FONT_SIZE_PREFERENCES_KEY); if (typeof storedNumber === 'number' && !Number.isNaN(storedNumber) && Number.isFinite(storedNumber) && storedNumber > 0 && storedNumber <= MAX_FONT_SIZE) { return storedNumber; } const storedString = userPreferences.getString(FONT_SIZE_PREFERENCES_KEY); const parsed = storedString !== null ? Number(storedString) : undefined; if (parsed !== undefined && !Number.isNaN(parsed) && Number.isFinite(parsed) && parsed > 0 && parsed <= MAX_FONT_SIZE) { - return parsed as number; + return parsed; } return FONT_SIZE_OPTIONS.NORMAL; };
103-114: Consider memoizing the fallback object to avoid referential instability.When the context is unavailable, a new fallback object (including a new
scaleFontSizefunction) is created on every render. This could cause unnecessary re-renders in consumers that rely on referential equality.This is only an issue if components use
useResponsiveLayoutoutside the provider, which should be rare. If this edge case is expected to occur, consider extracting the fallback to a module-level constant.Optional: Extract fallback to module scope
+const DEFAULT_LAYOUT_CONTEXT: IResponsiveLayoutContextData = { + fontScale: 1.0, + width: 0, + height: 0, + isLargeFontScale: false, + fontScaleLimited: 1.0, + rowHeight: BASE_ROW_HEIGHT, + rowHeightCondensed: BASE_ROW_HEIGHT_CONDENSED, + scaleFontSize: (size: number) => size +}; + export const useResponsiveLayout = () => { const context = useContext(ResponsiveLayoutContext); if (!context || Object.keys(context).length === 0) { - return { - fontScale: 1.0, - width: 0, - height: 0, - isLargeFontScale: false, - fontScaleLimited: 1.0, - rowHeight: BASE_ROW_HEIGHT, - rowHeightCondensed: BASE_ROW_HEIGHT_CONDENSED, - scaleFontSize: (size: number) => size - }; + return DEFAULT_LAYOUT_CONTEXT; } return context; };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
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.
Applied to files:
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧬 Code graph analysis (1)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
app/lib/constants/keys.ts (1)
FONT_SIZE_PREFERENCES_KEY(23-23)
🔇 Additional comments (4)
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (4)
29-29:SMALL_FONT_THRESHOLDis not exported.The AI summary indicates this constant should be exposed, but it's declared without
export. If it's needed by other components (e.g., for conditional styling based on small font detection), consider exporting it. If it's intentionally internal-only, this is fine as-is.
81-81:scaleFontSizeuses unboundedfontScale— verify this is intentional.
scaleFontSizemultiplies by the uncappedfontScale(which could besystemFontScale * fontSizeMultiplier, potentially reaching 2.0+ for users with accessibility settings). Meanwhile,fontScaleLimitedcaps at 1.3 for row heights.If this is intentional for accessibility compliance (allowing text to scale fully), this is correct. If you want to cap scaled font sizes similar to row heights, consider using
fontScaleLimitedinstead.
52-98: Well-structured implementation using modern React patterns.Good use of:
useSyncExternalStorefor subscribing to storage changes — this is the correct React 18+/19 pattern for external stores.useCallbackforscaleFontSizeto maintain stable reference.useMemoforcontextValueto prevent unnecessary consumer re-renders.The row height clamping logic with
FONT_SCALE_LIMITappropriately prevents layout overflow while still allowing font accessibility scaling.
56-67: The code correctly uses the MMKV API. TheinitializeStorage.addOnValueChangedListener()method from react-native-mmkv v3.3.3 returns an object with a.remove()method, and the cleanup function at line 65 properly invokes it. TheuseSyncExternalStoresubscription pattern is implemented correctly.
|
@diegolmello you can check this |
2 participants
Issues Addressed
Demo
🎥 Preview Video
https://github.com/user-attachments/assets/43e70c7d-5328-48ab-81bb-2c0be7ac407f
How to Test
Screenshots (With Font Size Changes)
Type of Change
Checklist