feat: Room Invited State #6857
feat: Room Invited State #6857
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds invite support: subscription types gain optional Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant InvitedRoom as InvitedRoom UI
participant getInvitationData as getInvitationData
participant replyRoomInvite as replyRoomInvite
participant restApi as restApi
participant Server as Backend
User->>InvitedRoom: Tap Accept / Reject
InvitedRoom->>getInvitationData: invoke action callback
getInvitationData->>replyRoomInvite: replyRoomInvite(roomId, action)
replyRoomInvite->>restApi: sendInvitationReply(roomId, action)
restApi->>Server: POST rooms.invite { roomId, action }
alt Success
Server-->>restApi: 2xx
restApi-->>replyRoomInvite: resolve
replyRoomInvite-->>InvitedRoom: success (UI updates)
else Error
Server-->>restApi: error
restApi-->>replyRoomInvite: throw
replyRoomInvite-->>InvitedRoom: show i18n error alert
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (24)
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 |
aleksandernsilva
force-pushed
the
feat/invited-room-state
branch
from
4145042 to
1928efc
Compare
aleksandernsilva
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/views/RoomView/constants.ts (1)
39-50: Critical: Duplicate 'status' entry in roomAttrsUpdate array.The
statusfield appears twice in theroomAttrsUpdatearray (line 39 and line 48). This duplication could cause unexpected behavior in room attribute updates.🔎 Apply this diff to remove the duplicate:
'teamMain', 'teamId', - 'status', 'lastMessage', 'onHold', 't', 'autoTranslate', 'autoTranslateLanguage', 'unmuted', 'E2EKey', 'encrypted', 'status', 'inviter'
🧹 Nitpick comments (6)
app/lib/methods/helpers/mergeSubscriptionsRooms.ts (1)
17-17: Consider refactoring instead of suppressing the complexity warning.Suppressing the complexity warning without addressing the underlying issue is a code smell. The
mergefunction is over 100 lines with multiple conditional branches. Consider extracting logical groups (e.g., room property merging, field normalization) into smaller helper functions to improve maintainability and testability.app/views/RoomView/components/InvitedRoom.tsx (5)
28-29: Title truncation may break mid-word or mid-emoji.The
roomName.slice(0, 30)truncation could cut a word or multi-byte character in the middle. Consider using a more robust truncation that respects word boundaries or usesnumberOfLineson theTextcomponent instead.🔎 Suggested approach
- const title = - room.t === 'd' ? I18n.t('invited_room_title_dm') : I18n.t('invited_room_title_channel', { room_name: roomName.slice(0, 30) }); + const truncatedRoomName = roomName.length > 30 ? `${roomName.slice(0, 27)}...` : roomName; + const title = + room.t === 'd' ? I18n.t('invited_room_title_dm') : I18n.t('invited_room_title_channel', { room_name: truncatedRoomName });
33-40: Loading state remains stuck if navigation happens after accept/reject.If
replyRoomInvitesucceeds and the user navigates away (e.g., room view changes on accept),setLoading(false)still executes on an unmounted component, causing a React warning. Consider checking if the component is still mounted or using an abort pattern.🔎 Suggested fix using mounted ref
+import React, { useState, useRef, useEffect, type ReactElement } from 'react'; ... export const InvitedRoom = ({ room }: InvitedRoomProps): ReactElement => { const { colors } = useTheme(); const styles = useStyle(); const [loading, setLoading] = useState(false); + const isMounted = useRef(true); + + useEffect(() => { + return () => { + isMounted.current = false; + }; + }, []); const { rid, inviter } = room; const roomName = getRoomTitle(room); ... const handleReply = async (action: 'accept' | 'reject') => { try { setLoading(true); await replyRoomInvite(rid, action); } finally { - setLoading(false); + if (isMounted.current) { + setLoading(false); + } } };
51-53: NestingChipinsideTextmay cause layout issues on some platforms.Wrapping a component like
Chip(which likely contains its ownView) inside aTextcomponent can cause unexpected behavior in React Native. Consider using aViewcontainer instead.🔎 Suggested fix
- <Text style={styles.username}> + <View style={styles.username}> <Chip avatar={inviter.username} text={inviter.name || inviter.username} /> - </Text> + </View>
55-62: Both buttons share the same loading state, preventing user from switching action.When the user taps Accept, both buttons show loading. If the request takes time or fails, the user cannot tap Reject (and vice versa). Consider disabling only the non-pressed button or showing loading only on the pressed one.
68-115: StyleSheet is recreated on every render.
useStylecallsStyleSheet.createon each render. While not a major performance issue, extracting static styles outside the component or memoizing withuseMemowould be more efficient.🔎 Suggested approach using useMemo
+import React, { useState, useMemo, type ReactElement } from 'react'; ... const useStyle = () => { const { colors } = useTheme(); - const styles = StyleSheet.create({ + const styles = useMemo(() => StyleSheet.create({ root: { flex: 1, backgroundColor: colors.surfaceRoom }, // ... rest of styles - }); + }), [colors]); return styles; };
📜 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 (12)
app/definitions/ISubscription.ts(2 hunks)app/definitions/rest/v1/rooms.ts(1 hunks)app/i18n/locales/en.json(3 hunks)app/lib/database/model/Subscription.js(2 hunks)app/lib/database/model/migrations.js(1 hunks)app/lib/database/schema/app.js(2 hunks)app/lib/methods/helpers/mergeSubscriptionsRooms.ts(2 hunks)app/lib/methods/replyRoomInvite.ts(1 hunks)app/lib/services/restApi.ts(1 hunks)app/views/RoomView/components/InvitedRoom.tsx(1 hunks)app/views/RoomView/constants.ts(1 hunks)app/views/RoomView/index.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
app/views/RoomView/components/InvitedRoom.tsx (4)
app/definitions/ISubscription.ts (1)
IInviteSubscription(122-125)app/theme.tsx (1)
useTheme(29-29)app/lib/methods/helpers/helpers.ts (1)
getRoomTitle(43-60)app/lib/methods/replyRoomInvite.ts (1)
replyRoomInvite(6-13)
app/lib/methods/replyRoomInvite.ts (2)
app/lib/services/restApi.ts (1)
sendInvitationReply(1103-1103)app/lib/methods/helpers/info.ts (1)
showErrorAlert(5-6)
app/lib/database/model/Subscription.js (1)
app/lib/database/utils.ts (1)
sanitizer(19-19)
app/views/RoomView/index.tsx (2)
app/definitions/ISubscription.ts (1)
isInviteSubscription(191-192)app/views/RoomView/components/InvitedRoom.tsx (1)
InvitedRoom(20-66)
app/definitions/ISubscription.ts (1)
app/definitions/IUser.ts (1)
IUser(127-151)
🔇 Additional comments (17)
app/lib/methods/helpers/mergeSubscriptionsRooms.ts (1)
116-117: LGTM: Proper normalization of invitation fields.The explicit normalization of
statusandinviterfields using the nullish coalescing operator is correct and consistent with the surrounding code style (similar toblocker,blocked, andhideMentionStatusnormalization on lines 118-120). This ensures these optional fields are explicitly set toundefinedwhen not present, maintaining consistency across the subscription object.app/definitions/rest/v1/rooms.ts (1)
57-59: LGTM!The new
rooms.inviteendpoint definition is well-structured with type-safe action constraints and consistent parameter naming.app/lib/services/restApi.ts (1)
1103-1103: LGTM!The
sendInvitationReplyfunction is correctly implemented and matches the REST endpoint definition inapp/definitions/rest/v1/rooms.ts.app/lib/database/model/Subscription.js (2)
156-158: LGTM!The new
statusandinviterfields are correctly decorated and follow existing patterns in the model. The use of@jsonforinviteris appropriate given it stores a user object.
226-228: LGTM!The
asPlain()method correctly includes the newstatusandinviterfields in the output.app/lib/database/model/migrations.js (1)
335-346: LGTM!The migration to version 28 correctly adds the
statusandinvitercolumns to the subscriptions table, following the established migration pattern and aligning with the schema changes.app/definitions/ISubscription.ts (3)
122-125: LGTM!The
IInviteSubscriptioninterface correctly extendsISubscriptionand narrows the types appropriately for invite-specific subscriptions.
191-192: LGTM!The
isInviteSubscriptiontype guard correctly validates both the status and presence of inviter, providing proper type narrowing for TypeScript.
119-120: All referenced fields exist in IUser interface.IUser contains
_id(required string),username(required string via ILoggedUser), andname(optional string via ILoggedUser). The inviter field is correctly typed.app/lib/database/schema/app.js (2)
4-4: LGTM!Schema version correctly bumped to 28 to reflect the addition of new columns.
73-76: LGTM!The new
statusandinvitercolumns are correctly defined as optional strings, aligning with the migration inapp/lib/database/model/migrations.js.app/i18n/locales/en.json (3)
420-423: LGTM!The new localization strings follow the existing naming conventions and are grammatically correct. The
invited_room_*keys properly support both channel and DM invitation scenarios with appropriate placeholders.
340-340: LGTM!The error key follows the existing
error-*naming pattern and provides a clear, user-friendly message.
692-692: LGTM!The "reject" key maintains consistency with the "accept" key (line 15), both using sentence case capitalization.
app/views/RoomView/index.tsx (3)
69-71: LGTM!The imports are properly structured, adding
RoomTypeandisInviteSubscriptiontype guard which are used correctly in the component logic.
647-651: LGTM!The early return for invite subscriptions correctly bypasses message loading and member fetching, which would be unnecessary and potentially cause errors for unaccepted invitations. The type guard
isInviteSubscriptionensures type safety.
333-337: The existing guard already prevents unnecessary calls on every render.The condition
prevState.roomUpdate.status === 'INVITED' && roomUpdate.status !== 'INVITED'is a proper safeguard that only triggers when transitioning out of the INVITED status. Each legitimate transition would callinit()once. Multiple calls would only occur if the status oscillates between INVITED and non-INVITED states—an unlikely scenario in normal subscription sync. If this oscillation is a genuine concern, consider tracking initialization with a flag to prevent re-initializing on redundant transitions.
aleksandernsilva
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
aleksandernsilva
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
aleksandernsilva
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
aleksandernsilva
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
aleksandernsilva
had a problem deploying
to
official_ios_build
GitHub Actions
Error
aleksandernsilva
had a problem deploying
to
official_android_build
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/views/RoomView/components/InvitedRoom.tsx (1)
53-99:StyleSheet.createis called on every render, creating unnecessary overhead.
StyleSheet.createshould be called once outside the component. Although the styles depend on theme colors, consider extracting static styles and only computing dynamic color values within the hook.🔎 Proposed refactor
+const staticStyles = StyleSheet.create({ + root: { + flex: 1 + }, + container: { + flex: 1, + marginHorizontal: 24, + justifyContent: 'center' + }, + textView: { alignItems: 'center' }, + icon: { + width: 58, + height: 58, + borderRadius: 30, + marginBottom: GAP, + alignItems: 'center', + justifyContent: 'center' + }, + title: { + ...sharedStyles.textBold, + fontSize: 24, + lineHeight: 32, + textAlign: 'center', + marginBottom: GAP + }, + description: { + ...sharedStyles.textRegular, + fontSize: 16, + lineHeight: 24, + textAlign: 'center' + }, + username: { + ...sharedStyles.textRegular, + fontSize: 16, + lineHeight: 24, + textAlign: 'center', + marginBottom: GAP + } +}); + const useStyle = () => { const { colors } = useTheme(); - const styles = StyleSheet.create({ - root: { - flex: 1, - backgroundColor: colors.surfaceRoom - }, - // ... all styles - }); - return styles; + return { + ...staticStyles, + root: [staticStyles.root, { backgroundColor: colors.surfaceRoom }], + icon: [staticStyles.icon, { backgroundColor: colors.surfaceNeutral }], + title: [staticStyles.title, { color: colors.fontTitlesLabels }], + description: [staticStyles.description, { color: colors.fontDefault }], + username: [staticStyles.username, { color: colors.fontDefault }] + }; };app/lib/methods/getInvitationData.ts (1)
10-10: Consider adding ellipsis for truncated room names.The room name is truncated at 30 characters without indication. Long names may appear cut off abruptly.
🔎 Proposed improvement
+const truncateWithEllipsis = (text: string, maxLength: number) => { + if (text.length <= maxLength) return text; + return `${text.slice(0, maxLength - 1)}…`; +}; + export const getInvitationData = (room: IInviteSubscription) => { const title = room.t === 'd' ? I18n.t('invited_room_title_dm') - : I18n.t('invited_room_title_channel', { room_name: getRoomTitle(room).slice(0, 30) }); + : I18n.t('invited_room_title_channel', { room_name: truncateWithEllipsis(getRoomTitle(room), 30) });
📜 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/methods/getInvitationData.tsapp/views/RoomView/RightButtons.tsxapp/views/RoomView/components/InvitedRoom.tsxapp/views/RoomView/index.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
app/views/RoomView/components/InvitedRoom.tsx (4)
app/definitions/ISubscription.ts (1)
IInviteSubscription(122-125)app/theme.tsx (1)
useTheme(29-29)app/lib/constants/colors.ts (1)
colors(280-302)app/containers/message/Components/Attachments/Image/Button.tsx (1)
Button(13-26)
app/views/RoomView/index.tsx (3)
app/definitions/ISubscription.ts (1)
isInviteSubscription(191-192)app/lib/methods/getInvitationData.ts (1)
getInvitationData(6-21)app/views/RoomView/components/InvitedRoom.tsx (1)
InvitedRoom(23-51)
app/lib/methods/getInvitationData.ts (3)
app/definitions/ISubscription.ts (1)
IInviteSubscription(122-125)app/lib/methods/helpers/helpers.ts (1)
getRoomTitle(43-60)app/lib/methods/replyRoomInvite.ts (1)
replyRoomInvite(6-13)
🔇 Additional comments (6)
app/views/RoomView/RightButtons.tsx (1)
482-484: LGTM! The early return for invited rooms correctly hides header actions.Minor note: Consider using a shared constant for
'INVITED'to stay consistent withisInviteSubscriptionusage elsewhere in the codebase, but this works correctly as-is.app/views/RoomView/index.tsx (4)
334-338: LGTM! Correctly triggers initialization when an invitation is accepted, ensuring room data loads after the status transition.
649-653: LGTM! Appropriately skips message loading for invite subscriptions since there's no room content to display yet.
546-547: LGTM! Disabling the room header for invite subscriptions prevents navigation to room actions before acceptance.
1581-1589: SafeAreaView wrapping correctly addresses the safe area concern from previous review.Note: The
loadingprop is not passed toInvitedRoom, so users won't see loading feedback during accept/reject. See the related comment onInvitedRoom.tsxfor a fix suggestion.app/lib/methods/getInvitationData.ts (1)
18-19: Useroom.ridinstead ofroom.idfor the API call.While both properties exist on
ISubscriptionand hold the same value, thereplyRoomInvitefunction signature explicitly expectsrid: stringas its first parameter. The interface comment states "the same as id" for theridproperty, and Rocket.Chat conventions useridas the standard room identifier. Update lines 18–19 to useroom.ridfor semantic consistency:accept: () => replyRoomInvite(room.rid, 'accept'), reject: () => replyRoomInvite(room.rid, 'reject')
| export const InvitedRoom = ({ title, description, inviter, loading, onAccept, onReject }: InvitedRoomProps): ReactElement => { | ||
| const { colors } = useTheme(); | ||
| const styles = useStyle(); | ||
|
|
||
| return ( | ||
| <View style={styles.root}> | ||
| <View style={styles.container}> | ||
| <View style={styles.textView}> | ||
| <View style={styles.icon}> | ||
| <CustomIcon name='mail' size={42} color={colors.fontSecondaryInfo} /> | ||
| </View> | ||
| <Text style={styles.title}>{title}</Text> | ||
| <Text style={styles.description}>{description}</Text> | ||
| <View style={styles.username}> | ||
| <Chip avatar={inviter.username} text={inviter.name || inviter.username} /> | ||
| </View> | ||
| </View> | ||
| <Button title={I18n.t('accept')} loading={loading} onPress={onAccept} /> | ||
| <Button | ||
| title={I18n.t('reject')} | ||
| type='secondary' | ||
| loading={loading} | ||
| backgroundColor={colors.surfaceTint} | ||
| onPress={onReject} | ||
| /> | ||
| </View> | ||
| </View> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Loading state is not wired up in the parent component.
The loading prop exists but is never passed from RoomView/index.tsx (line 1586). Users won't see feedback while accept/reject actions are in progress. Consider either:
- Managing loading state in the parent and passing it down
- Managing loading state internally in this component
🔎 Example: internal loading state
-export const InvitedRoom = ({ title, description, inviter, loading, onAccept, onReject }: InvitedRoomProps): ReactElement => {
+export const InvitedRoom = ({ title, description, inviter, onAccept, onReject }: InvitedRoomProps): ReactElement => {
const { colors } = useTheme();
const styles = useStyle();
+ const [loading, setLoading] = React.useState(false);
+
+ const handleAccept = async () => {
+ setLoading(true);
+ try {
+ await onAccept();
+ } finally {
+ setLoading(false);
+ }
+ };
+
+ const handleReject = async () => {
+ setLoading(true);
+ try {
+ await onReject();
+ } finally {
+ setLoading(false);
+ }
+ };
return (
// ...
- <Button title={I18n.t('accept')} loading={loading} onPress={onAccept} />
+ <Button title={I18n.t('accept')} loading={loading} onPress={handleAccept} />
<Button
title={I18n.t('reject')}
type='secondary'
loading={loading}
backgroundColor={colors.surfaceTint}
- onPress={onReject}
+ onPress={handleReject}
/>
// ...
);
};🤖 Prompt for AI Agents
In app/views/RoomView/components/InvitedRoom.tsx around lines 23-51: the
component accepts a loading prop but the parent (RoomView/index.tsx at ~line
1586) never passes it, so users get no feedback during accept/reject operations;
fix by either A) add loading state in the parent and pass it down: create a
boolean state (e.g., isInvitedActionLoading), set it true before awaiting the
accept/reject async call and false afterwards (handle errors and finally block),
then pass loading={isInvitedActionLoading} into <InvitedRoom>, or B) manage
loading internally in InvitedRoom: add a local useState loading flag, wrap
onAccept/onReject handlers to setLoading(true) before awaiting the provided
callback and setLoading(false) in finally, and use that state for the Button
loading prop; ensure callbacks remain async-safe and errors are handled.
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
aleksandernsilva
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
…t.Chat.ReactNative into feat/invited-room-state
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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/RoomItem/RoomItem.tsxapp/containers/RoomItem/index.tsxapp/containers/RoomItem/interfaces.tsapp/i18n/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- app/i18n/locales/en.json
🧰 Additional context used
🧬 Code graph analysis (2)
app/containers/RoomItem/index.tsx (1)
app/lib/methods/isInviteSubscription.ts (1)
isInviteSubscription(3-4)
app/containers/RoomItem/RoomItem.tsx (3)
app/containers/RoomItem/interfaces.ts (1)
IRoomItemProps(101-128)app/theme.tsx (1)
useTheme(29-29)app/lib/constants/colors.ts (1)
colors(280-302)
🔇 Additional comments (5)
app/containers/RoomItem/interfaces.ts (1)
110-110: LGTM!The addition of the optional
isInvitedboolean property toIRoomItemPropsis clean and appropriate for tracking invitation state.app/containers/RoomItem/RoomItem.tsx (2)
19-20: LGTM!The imports for
CustomIconanduseThemeare appropriate for rendering the invited state indicator.
58-59: LGTM!The addition of the
isInvitedprop and the use ofuseThemeto access theme colors are correctly implemented.Also applies to: 63-63
app/containers/RoomItem/index.tsx (2)
8-8: LGTM!The import of
isInviteSubscriptionis appropriate for determining invitation status.
65-65: LGTM!The
isInvitedprop is correctly computed using theisInviteSubscriptiontype guard and passed to theRoomItemcomponent.
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/views/RoomView/components/InvitedRoom.tsx (1)
23-51: Loading state is not wired up in the parent component.The
loadingprop exists but is never passed fromRoomView/index.tsx(line 1586). Users won't see feedback while accept/reject actions are in progress. This issue was previously flagged and remains unresolved.
🧹 Nitpick comments (2)
app/views/RoomView/components/InvitedRoom.tsx (2)
32-46: Add accessibility labels for better screen reader support.The icon and buttons lack accessibility labels, which impacts users relying on screen readers. Consider adding:
accessibilityLabelto the CustomIcon (line 32)accessibilityLabeloraccessibilityHintto both Button components (lines 40, 41-46)🔎 Suggested accessibility improvements
<CustomIcon name='mail' size={42} color={colors.fontSecondaryInfo} + accessibilityLabel={I18n.t('invitation_icon')} />-<Button title={I18n.t('accept')} loading={loading} onPress={onAccept} /> +<Button + title={I18n.t('accept')} + loading={loading} + onPress={onAccept} + accessibilityHint={I18n.t('accept_invitation_hint')} +/> <Button title={I18n.t('reject')} type='secondary' loading={loading} backgroundColor={colors.surfaceTint} onPress={onReject} + accessibilityHint={I18n.t('reject_invitation_hint')} />Note: You'll need to add the corresponding i18n keys to
app/i18n/locales/en.json.
90-97: Simplify theusernamestyle definition.The
usernamestyle includes text-related properties (textRegular,fontSize,lineHeight,textAlign,color) but is applied to aViewwrapping theChipcomponent (line 36-38). Only themarginBottomproperty will take effect. Consider simplifying to only the properties that apply:🔎 Proposed simplification
username: { - ...sharedStyles.textRegular, - fontSize: 16, - lineHeight: 24, - textAlign: 'center', - color: colors.fontDefault, + alignItems: 'center', marginBottom: GAP }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
app/containers/Chip/__snapshots__/Chip.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
app/containers/Chip/Chip.stories.tsxapp/containers/Chip/index.tsxapp/views/RoomView/components/InvitedRoom.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/containers/Chip/Chip.stories.tsx (1)
app/containers/Chip/index.tsx (1)
IChip(35-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🔇 Additional comments (5)
app/containers/Chip/index.tsx (1)
41-65: LGTM! Clean implementation of fullWidth prop.The fullWidth prop is well-implemented:
- Properly typed as optional boolean
- Correctly removes maxWidth constraints from both the pressable container and text container when enabled
- Maintains text truncation via numberOfLines={1} to prevent overflow
The chip will now expand to fill its parent's width when fullWidth is true, while maintaining the original 192px maxWidth constraint by default.
app/containers/Chip/Chip.stories.tsx (1)
18-20: LGTM! Prop forwarding correctly implemented.The ChipWrapped component properly accepts and forwards the fullWidth prop to the underlying Chip component.
app/views/RoomView/components/InvitedRoom.tsx (3)
1-12: LGTM!Imports and constants are well-organized. The GAP constant provides consistent spacing throughout the component.
40-46: Verify shared loading state behavior between accept and reject buttons.Both buttons use the same
loadingprop, meaning both will be disabled simultaneously during any action. Confirm this is intentional to prevent race conditions between accept/reject. If users should be able to cancel an accept action by clicking reject (or vice versa), consider separate loading states.
14-21: Theinviterfield is correctly defined as non-nullable.The type definition in
app/definitions/ISubscription.tsexplicitly declaresinviter: NonNullable<ISubscription['inviter']>withinIInviteSubscription. This ensures the inviter prop cannot be null or undefined, making the direct property access on line 37 (inviter.username,inviter.name) type-safe without additional null checks.
|
|
||
| export const ChipWithoutAvatarAndIcon = () => <ChipWrapped text='Without Avatar and Icon' />; | ||
|
|
||
| export const ChipFullWidth = () => <ChipWrapped text='Full Width Text With Long Text That Should Be Wrapped' fullWidth />; |
There was a problem hiding this comment.
Minor discrepancy: Story text mentions "wrapped" but text actually truncates.
The story text says "Should Be Wrapped" but the Chip component uses numberOfLines={1}, which causes text to truncate with an ellipsis rather than wrap to multiple lines. Consider updating the story text to better reflect the actual behavior, e.g., "Full Width Chip With Long Text That Truncates".
🔎 Suggested text update
-export const ChipFullWidth = () => <ChipWrapped text='Full Width Text With Long Text That Should Be Wrapped' fullWidth />;
+export const ChipFullWidth = () => <ChipWrapped text='Full Width Chip With Long Text That Truncates' fullWidth />;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const ChipFullWidth = () => <ChipWrapped text='Full Width Text With Long Text That Should Be Wrapped' fullWidth />; | |
| export const ChipFullWidth = () => <ChipWrapped text='Full Width Chip With Long Text That Truncates' fullWidth />; |
🤖 Prompt for AI Agents
In app/containers/Chip/Chip.stories.tsx around line 34, the story label claims
the text "Should Be Wrapped" but the Chip uses numberOfLines={1} causing
truncation; update the story text to accurately describe behavior (e.g., "Full
Width Chip With Long Text That Truncates") or remove the "wrapped" wording so
the story matches the component's single-line truncation behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/containers/RoomItem/RoomItem.stories.tsx (1)
193-198: LGTM! Consider adding more realistic props.The story correctly demonstrates the invited room state in both display modes and follows the established pattern.
For a more realistic visual representation, consider adding typical room properties like
name,avatar, orlastMessageto better demonstrate how the invited state appears in actual usage scenarios.Optional enhancement with more context
export const InvitedRoom = () => ( <> - <RoomItem isInvited /> - <RoomItem isInvited displayMode={DisplayMode.Condensed} /> + <RoomItem isInvited name='general' avatar='general' /> + <RoomItem isInvited name='support-team' avatar='support-team' displayMode={DisplayMode.Condensed} /> </> );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
app/containers/RoomItem/__snapshots__/RoomItem.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
app/containers/RoomItem/RoomItem.stories.tsxapp/containers/RoomItem/RoomItem.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/containers/RoomItem/RoomItem.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
diegolmello
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_android_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
Proposed changes
This pull request introduces support for handling room invitations, including displaying invitation details to users and allowing them to accept or reject invites.
Invitation Handling and UI Integration:
InvitedRoomReact component to display invitation information and provide accept/reject actions in the room view.RoomViewlogic to detect invite subscriptions, render the invitation UI, and initialize the room only after the invitation is accepted.Data Model and Schema Updates:
ISubscriptioninterface with newstatusandinviterfields, and introducedIInviteSubscriptionand a type guard function for invite subscriptions.statusandinviterfields in thesubscriptionstable.Room State Management:
statusandinviterfields. (app/views/RoomView/constants.ts)Issue(s)
FB-157
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Localization
✏️ Tip: You can customize this high-level summary in your review settings.