BA-2366: improve messages components #242
Conversation
pt-tsl
requested review from
anicioalexandre,
deboracosilveira and
priscilladeroode
as code owners
|
WalkthroughThis pull request refactors and extends the messages module. Component and type names have been updated (e.g., from ChatRoomsList to AllChatRoomsList, CreateGroup to GroupChatCreate, etc.), and several UI elements have been reorganized into distinct Header and Body subcomponents. New styled components and type interfaces have been added, and obsolete files or interfaces have been removed. Exports have been reorganized to reflect the new structure, and Storybook configuration has been updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant SC as SingleChatCreate
participant H as Header
participant B as Body
U->>H: Click Back Icon
H->>SC: Trigger onHeaderClick event
U->>B: Enters search query
B->>SC: Calls handleSearchChange
SC->>B: Updates and displays search results
sequenceDiagram
participant U as User
participant GC as GroupChatCreate
participant H as Header
participant E as EditGroupTitleAndImage
U->>H: Click Create Button
H->>GC: Trigger onCreateButtonClicked event
GC->>E: Render/update group details form
U->>E: Input group title/image details
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/components/modules/messages/web/AllChatRoomsList/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
packages/components/modules/messages/web/AllChatRoomsList/styled.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
packages/components/modules/messages/web/ChatRoomsComponent/styled.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
pt-tsl
force-pushed
the
BA-2366-improve-messages-components
branch
from
a3ba828 to
08cdaab
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
packages/components/modules/messages/web/SingleChatCreate/index.tsx (1)
107-107: 🛠️ Refactor suggestionImprove height calculation for the Virtuoso component
The current height calculation uses a magic number formula with hardcoded pixel values, which may not work well with different header types as noted in the TODO comment.
Consider using a more robust approach:
- Use a ref to measure the available height dynamically
- Use CSS Grid or Flexbox for automatic sizing
- Define height in terms of viewport units with appropriate calculations
Example with refs:
const containerRef = useRef(null); const [containerHeight, setContainerHeight] = useState(0); useEffect(() => { if (containerRef.current) { const updateHeight = () => { const availableHeight = window.innerHeight - containerRef.current.offsetTop; setContainerHeight(availableHeight); }; updateHeight(); window.addEventListener('resize', updateHeight); return () => window.removeEventListener('resize', updateHeight); } }, []); // Then use containerHeight in the Virtuoso style
🧹 Nitpick comments (17)
packages/components/modules/messages/web/GroupChatCreate/Header/styled.tsx (1)
4-11: Consider adding responsive styling for smaller screensThe
HeaderContainerstyled component is well-structured and follows the design patterns used in similar components. However, unlike theSingleChatCreate/Header/styled.tsxcomponent, it doesn't include responsive styling for smaller screens.Consider adding responsive styling for smaller screens to maintain consistency with other header components:
export const HeaderContainer = styled(Box)(({ theme }) => ({ display: 'grid', gridTemplateColumns: '24px auto 24px', alignItems: 'center', width: '100%', padding: theme.spacing(2), borderBottom: `1px solid ${theme.palette.divider}`, + [theme.breakpoints.down('sm')]: { + padding: `${theme.spacing(2)} ${theme.spacing(1.5)} ${theme.spacing(2)}`, + }, }))packages/components/modules/messages/web/MessageCreate/__storybook__/MessagesModule.mdx (1)
3-3: Documentation title and heading updated, but example code needs revision.The title and heading have been correctly updated from "SendMessage" to "MessageCreate", but the example code on lines 35 and 38 still references the old component name "SendMessage" instead of "MessageCreate".
-import { SendMessage } from '@baseapp-frontend/design-system/web' +import { MessageCreate } from '@baseapp-frontend/design-system/web' const MyComponent = () => { - return <SendMessage roomId="123" /> + return <MessageCreate roomId="123" /> }Also applies to: 7-7
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1)
6-6: Consider using a more specific type instead ofanyThe
controlproperty is typed asany, which reduces type safety. If this is a form control from a library like React Hook Form, consider using a more specific type from that library.packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx (2)
20-20: Consider handling the case when avatar is undefinedThe
avatarprop is optional according to theBodyPropsinterface, but there's no fallback handling whenavataris undefined. Consider providing a fallback or default avatar.
31-42: Add a unique identifier to improve list accessibilityThe
Boxelement withrole="list"doesn't have anidattribute, which would improve accessibility by providing a unique identifier for the list. Consider adding anidattribute:- <Box role="list" aria-label="group members"> + <Box role="list" aria-label="group members" id="group-members-list">packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx (2)
15-17: Consider adding a data-testid for easier testingAdding a
data-testidattribute to the back button would make it easier to target in automated tests:- <IconButton onClick={onBackButtonClicked} aria-label="cancel group creation"> + <IconButton onClick={onBackButtonClicked} aria-label="cancel group creation" data-testid="cancel-group-creation-button">
21-23: Consider adding a data-testid for easier testingSimilarly, adding a
data-testidattribute to the create button would improve testability:- <IconButton aria-label="Create group" disabled={isDisabled} onClick={onCreateButtonClicked}> + <IconButton aria-label="Create group" disabled={isDisabled} onClick={onCreateButtonClicked} data-testid="create-group-button">packages/components/modules/messages/web/ChatRoom/index.tsx (3)
12-12: Update the variable name to match the imported componentThe import has been changed from
SendMessagetoMessageCreate, but the variable nameDefaultSendMessagewasn't updated to match. This creates confusion between the variable name and the component it references. Consider updating for consistency:- import DefaultSendMessage from '../MessageCreate' + import DefaultMessageCreate from '../MessageCreate'Then update all references to this import (like line 22):
- SendMessage = DefaultSendMessage, + SendMessage = DefaultMessageCreate,Or alternatively, align variable name with how it's used in the component:
- import DefaultSendMessage from '../MessageCreate' + import DefaultSendMessage from '../MessageCreate' // MessageCreate component used as SendMessage
22-23: Consider updating prop and component names for consistencyThe component name
SendMessageand its propsSendMessagePropsshould be updated to match the new component naming ofMessageCreatefor consistency:- SendMessage = DefaultSendMessage, - SendMessageProps = {}, + MessageCreate = DefaultSendMessage, + MessageCreateProps = {},This would require updating the
ChatRoomPropsinterface and other references as well.
55-55: Update component usage to maintain naming consistencyFor consistency with the component import change:
- <SendMessage roomId={roomId} {...SendMessageProps} /> + <SendMessage roomId={roomId} {...SendMessageProps} /> {/* Using MessageCreate component */}Or if you update the variable names as suggested above:
- <SendMessage roomId={roomId} {...SendMessageProps} /> + <MessageCreate roomId={roomId} {...MessageCreateProps} />packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx (1)
9-10: Naming inconsistency between component and styled componentI noticed that while the component has been renamed to
Header, the styled component is still calledGroupDetailsHeaderContainer. Consider renaming the styled component for better consistency.-import { GroupDetailsHeaderContainer } from './styled' +import { HeaderContainer } from './styled'And update the reference on line 17 as well.
packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx (1)
1-42: Well-structured new component with good customization optionsThe new
Bodycomponent looks well-implemented with good use of default props and proper handling of customization throughSearchbarProps. This approach makes the component more reusable and aligns with the PR objective of making components more customizable.However, I noticed there's a TODO comment in the type definition about improving the typing for the
Searchbarprop. Since it currently usesany, you should consider addressing this to improve type safety:// In types.ts file Searchbar: FC<SearchbarProps>packages/components/modules/messages/web/ChatRoomsComponent/types.ts (1)
16-29: Improve type safety for thesettingspropThe
settingsprop is typed asany, which reduces type safety. Consider defining a specific interface or using a more restrictive type likeRecord<string, unknown>if the exact structure is not known.- settings: any + settings: Record<string, unknown>Otherwise, the component props structure with optional components and their partial props is well-designed and allows for good customization.
packages/components/modules/messages/web/GroupChatEdit/index.tsx (1)
1-230: Consider component decomposition in future PRsThe component is quite large (over 200 lines) which might make it harder to maintain. Consider breaking it into smaller, more focused components in future PRs. This could include separate components for the header, form sections, and member management.
🧰 Tools
🪛 Biome (1.9.4)
[error] 104-104: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 106-106: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/components/modules/messages/web/ChatRoomsComponent/index.tsx (2)
122-123: Remove redundant hide property and TODO commentThe
hideprop is set to!shouldRenderLeftPanelbut the container is only rendered whenshouldRenderLeftPanelis true. This makes the hide prop redundant since the component won't be rendered when it would be hidden.{shouldRenderLeftPanel && ( - <ChatRoomsListContainer hide={!shouldRenderLeftPanel /* TODO: Why? */}> + <ChatRoomsListContainer> {renderLeftPanelContent()} </ChatRoomsListContainer> )}
127-128: Remove redundant hide property and TODO commentSimilar to the previous comment, the
hideprop here is redundant because the component is only rendered when it wouldn't be hidden.{shouldRenderRightPanel && ( - <ChatRoomContainer hide={!shouldRenderRightPanel /* TODO: Why? */}> + <ChatRoomContainer> {renderRightPanelContent()} </ChatRoomContainer> )}packages/components/modules/messages/web/ChatRoom/types.ts (1)
3-12: Rename prop names to match the component they referenceThe props are now using
MessageCreatePropstype but still named asSendMessageandSendMessageProps. Consider renaming them toMessageCreateandMessageCreatePropsfor consistency.export interface ChatRoomProps { roomId: string MessagesList?: FC<MessagesListProps> MessagesListProps?: Partial<MessagesListProps> - SendMessage?: FC<MessageCreateProps> - SendMessageProps?: Partial<MessageCreateProps> + MessageCreate?: FC<MessageCreateProps> + MessageCreateProps?: Partial<MessageCreateProps> onDisplayGroupDetailsClicked: () => void }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
packages/components/modules/messages/web/AllChatRoomsList/index.tsx(3 hunks)packages/components/modules/messages/web/AllChatRoomsList/styled.tsx(1 hunks)packages/components/modules/messages/web/AllChatRoomsList/types.ts(1 hunks)packages/components/modules/messages/web/ChatRoom/index.tsx(1 hunks)packages/components/modules/messages/web/ChatRoom/types.ts(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/constants.ts(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/index.tsx(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/styled.tsx(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/types.ts(1 hunks)packages/components/modules/messages/web/ChatRoomsList/styled.tsx(0 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/styled.tsx(1 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatCreate/index.tsx(4 hunks)packages/components/modules/messages/web/GroupChatCreate/styled.tsx(1 hunks)packages/components/modules/messages/web/GroupChatCreate/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Body/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx(2 hunks)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatDetails/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatEdit/index.tsx(2 hunks)packages/components/modules/messages/web/GroupChatEdit/types.ts(1 hunks)packages/components/modules/messages/web/MessageCreate/__storybook__/MessagesModule.mdx(1 hunks)packages/components/modules/messages/web/MessageCreate/__storybook__/stories.tsx(1 hunks)packages/components/modules/messages/web/MessageCreate/index.tsx(3 hunks)packages/components/modules/messages/web/MessageCreate/types.ts(1 hunks)packages/components/modules/messages/web/SendMessage/__storybook__/stories.tsx(0 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/styled.tsx(2 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/types.ts(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/styled.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts(1 hunks)packages/components/modules/messages/web/SingleChatCreate/index.tsx(2 hunks)packages/components/modules/messages/web/SingleChatCreate/styled.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/types.ts(1 hunks)packages/components/modules/messages/web/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/components/modules/messages/web/ChatRoomsList/styled.tsx
- packages/components/modules/messages/web/SendMessage/storybook/stories.tsx
🧰 Additional context used
🧬 Code Definitions (19)
packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (2)
packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (1) (1)
HeaderProps(1-5)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (1) (1)
HeaderProps(1-5)
packages/components/modules/messages/web/GroupChatDetails/Body/types.ts (1)
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1) (1)
BodyProps(5-14)
packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (2)
packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (1) (1)
HeaderProps(1-5)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1) (1)
HeaderProps(1-3)
packages/components/modules/messages/web/GroupChatCreate/Header/styled.tsx (2)
packages/components/modules/messages/web/GroupChatEdit/styled.tsx (1) (1)
HeaderContainer(4-10)packages/components/modules/messages/web/SingleChatCreate/Header/styled.tsx (1) (1)
HeaderContainer(6-13)
packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx (2)
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1) (1)
BodyProps(5-14)packages/components/modules/messages/web/SingleChatCreate/Body/styled.tsx (2) (2)
MainContainer(6-12)SearchbarContainer(14-19)
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1)
packages/components/modules/messages/web/GroupChatDetails/Body/types.ts (1) (1)
BodyProps(3-7)
packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx (3)
packages/components/modules/messages/web/GroupChatCreate/Header/styled.tsx (1) (1)
HeaderContainer(4-11)packages/components/modules/messages/web/GroupChatEdit/styled.tsx (1) (1)
HeaderContainer(4-10)packages/components/modules/messages/web/SingleChatCreate/Header/styled.tsx (1) (1)
HeaderContainer(6-13)
packages/components/modules/messages/web/ChatRoomsComponent/types.ts (5)
packages/components/modules/messages/web/AllChatRoomsList/types.ts (1) (1)
AllChatRoomsListProps(17-26)packages/components/modules/messages/web/GroupChatCreate/types.ts (1) (1)
GroupChatCreateProps(6-12)packages/components/modules/messages/web/GroupChatDetails/types.ts (1) (1)
GroupChatDetailsProps(11-22)packages/components/modules/messages/web/GroupChatEdit/types.ts (1) (1)
GroupChatEditProps(9-19)packages/components/modules/messages/web/SingleChatCreate/types.ts (1) (1)
SingleChatCreateProps(12-26)
packages/components/modules/messages/web/SingleChatCreate/styled.tsx (4)
packages/components/modules/messages/web/SingleChatCreate/ChatRoomListItem/styled.tsx (1) (1)
MainContainer(4-17)packages/components/modules/messages/web/SingleChatCreate/Body/styled.tsx (2) (2)
MainContainer(6-12)SearchbarContainer(14-19)packages/components/modules/messages/web/GroupChatEdit/AddMembersDialog/styled.tsx (1) (1)
SearchbarContainer(4-6)packages/components/modules/messages/web/GroupChatEdit/AddMembersMobile/styled.tsx (1) (1)
SearchbarContainer(4-6)
packages/components/modules/messages/web/GroupChatDetails/index.tsx (3)
packages/components/modules/messages/web/GroupChatDetails/types.ts (1) (1)
GroupChatDetailsProps(11-22)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (1) (1)
HeaderProps(1-5)packages/components/modules/messages/web/GroupChatDetails/Body/types.ts (1) (1)
BodyProps(3-7)
packages/components/modules/messages/web/GroupChatEdit/index.tsx (1)
packages/components/modules/messages/web/GroupChatEdit/types.ts (1) (1)
GroupChatEditProps(9-19)
packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (2)
packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (1) (1)
HeaderProps(1-5)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1) (1)
HeaderProps(1-3)
packages/components/modules/messages/web/ChatRoomsComponent/index.tsx (3)
packages/components/modules/messages/web/ChatRoomsComponent/types.ts (1) (1)
ChatRoomsComponentProps(16-29)packages/components/modules/messages/web/ChatRoomsComponent/constants.ts (2) (2)
LeftPanelContentValues(10-10)LEFT_PANEL_CONTENT(3-9)packages/components/modules/messages/web/ChatRoomsComponent/styled.tsx (3) (3)
ChatRoomsContainer(19-34)ChatRoomsListContainer(36-54)ChatRoomContainer(9-17)
packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx (3)
packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (1) (1)
HeaderProps(1-5)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (1) (1)
HeaderProps(1-5)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1) (1)
HeaderProps(1-3)
packages/components/modules/messages/web/MessageCreate/index.tsx (1)
packages/components/modules/messages/web/MessageCreate/types.ts (1) (1)
MessageCreateProps(5-9)
packages/components/modules/messages/web/ChatRoomsComponent/styled.tsx (1)
packages/components/modules/messages/web/ChatRoomsComponent/types.ts (1) (1)
HidableContainerProps(12-14)
packages/components/modules/messages/web/SingleChatCreate/index.tsx (3)
packages/components/modules/messages/web/SingleChatCreate/types.ts (1) (1)
SingleChatCreateProps(12-26)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1) (1)
HeaderProps(1-3)packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1) (1)
BodyProps(5-14)
packages/components/modules/messages/web/GroupChatCreate/index.tsx (1)
packages/components/modules/messages/web/GroupChatCreate/types.ts (1) (1)
GroupChatCreateProps(6-12)
packages/components/modules/messages/web/AllChatRoomsList/index.tsx (3)
packages/components/modules/messages/web/AllChatRoomsList/types.ts (1) (1)
AllChatRoomsListProps(17-26)packages/components/modules/messages/web/AllChatRoomsList/styled.tsx (2) (2)
Header(13-19)MainContainer(6-11)packages/components/modules/messages/web/AllChatRoomsList/constants.ts (1) (1)
CHAT_TAB_VALUES(1-5)
🪛 GitHub Actions: Main Workflow
packages/components/modules/messages/web/ChatRoomsComponent/index.tsx
[error] 92-92: Type '{ allProfilesRef: ChatRoomsQuery$data; Body?: FC | undefined; BodyProps?: Partial | undefined; ... 10 more ...; children?: ReactNode; }' is not assignable to type 'SingleChatCreateProps'.
🔇 Additional comments (54)
packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1)
1-3: Properly defined header interaction prop typeThe
HeaderPropsinterface appropriately defines a click handler for the header component, making it more customizable as per the PR objectives. This approach aligns with similar header components in the codebase.packages/components/modules/messages/web/GroupChatDetails/Body/types.ts (1)
1-7: Well-structured interface with appropriate optional propertiesThe
BodyPropsinterface effectively extendsPropsWithChildrenallowing for composition flexibility. The optional properties for title, avatar, and participantsCount provide good customization options while maintaining type safety.packages/components/modules/messages/web/SingleChatCreate/Header/styled.tsx (1)
1-13: Well-implemented responsive header stylingThe
HeaderContainerstyled component is properly implemented with responsive design considerations using Material-UI's theming system. The component has appropriate styling for borders, width, and padding with specific adaptations for small screen sizes.packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (1)
1-5: Good interface renaming for consistencyChanging the interface name from
GroupDetailsHeaderPropstoHeaderPropsimproves naming consistency across the messaging components. This standardization makes the codebase more maintainable and easier to understand for developers.packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (1)
1-5: Properly structured HeaderProps interface for the Group Chat Create componentThe new
HeaderPropsinterface is well-defined and follows a similar pattern to other header components in the module. It includes essential properties for handling back navigation and create functionality, while also providing state control through theisDisabledproperty.This interface supports the PR objective of making components more customizable by providing clear props that allow developers to control the component's behavior.
packages/components/modules/messages/web/MessageCreate/types.ts (1)
5-5: Improved naming from SendMessageProps to MessageCreatePropsThe renaming of
SendMessagePropstoMessageCreatePropsmakes the interface name more descriptive and better aligned with its purpose. This change improves code readability and follows the PR objective of making components more developer-friendly.Ensure that all references to
SendMessagePropsthroughout the codebase have been updated to useMessageCreatePropsto maintain consistency.packages/components/modules/messages/web/AllChatRoomsList/types.ts (2)
17-17: Clear renaming from ChatRoomsListProps to AllChatRoomsListPropsThe renaming of the interface to
AllChatRoomsListPropsis more specific and better describes the component's purpose. This change improves code clarity and helps distinguish this component from other potential chat room list components.
19-19: Added onHeaderClick property enhances customizationThe addition of the
onHeaderClickproperty allows developers to define custom behavior when the header is clicked, which directly supports the PR objective of making components more customizable.Ensure that this property is properly documented if you have API documentation for these components.
packages/components/modules/messages/web/GroupChatEdit/types.ts (1)
7-7: Interface rename improves naming consistency.The renamed interface
GroupChatEditProps(previouslyEditGroupProps) and repositioned import provide better component naming consistency throughout the module. This aligns with the PR objectives to make message components more customizable and developer-friendly.Also applies to: 9-9
packages/components/modules/messages/web/GroupChatCreate/types.ts (1)
4-4: Interface rename enhances naming consistency.Renaming from
CreateGroupPropstoGroupChatCreatePropsand repositioning the import follow the same pattern as other components in this PR. This consistent naming scheme makes the component hierarchy more intuitive and improves developer experience.Also applies to: 6-6
packages/components/modules/messages/web/AllChatRoomsList/styled.tsx (1)
1-19: Well-structured styled components for improved customization.The new styled components provide a clear separation of layout concerns, which enhances customizability as intended in the PR objectives. The components use responsive design principles with theme-based spacing.
packages/components/modules/messages/web/MessageCreate/__storybook__/stories.tsx (1)
1-19:❓ Verification inconclusive
New Storybook story for MessageCreate component.
The Storybook story is well-structured and follows standard practices, but needs mock resolvers to be uncommented or implemented. Consider completing the implementation to ensure the story functions properly during development.
🏁 Script executed:
#!/bin/bash # Check if mock resolvers are defined elsewhere in the codebase for MessageCreate echo "Searching for potential mock resolvers for MessageCreate..." rg -A 5 "mockResolvers.*MessageCreate" --type tsx --type tsLength of output: 218
Attention Required: Verify and Implement Mock Resolvers for MessageCreate
The Storybook story for the MessageCreate component is well-structured; however, the commented-out
mockResolversremains unaddressed. A search usingrgfor patterns matchingmockResolvers.*MessageCreateresulted in an error due to file type recognition issues (tsx files were not properly detected). This inconclusive output means we couldn’t confirm whether the mock resolvers are defined elsewhere in the codebase or simply pending implementation.• File:
packages/components/modules/messages/web/MessageCreate/__storybook__/stories.tsx(Lines 1–19)
• Concern: ThemockResolversis commented out; if it’s meant to be active, its implementation needs to exist and be integrated.Next Steps:
- Please manually verify if
mockResolversfor MessageCreate exist elsewhere or are intended to be added.- If required, implement or uncomment the mock resolver code so that the Storybook story functions as expected during development.
- Otherwise, consider removing the commented-out section to reduce confusion.
packages/components/modules/messages/web/GroupChatCreate/styled.tsx (1)
1-18: Improved type definition enhances component clarity.The ProfilesContainer is now explicitly typed as FC, which improves type safety and makes the component's interface clearer. This change aligns with good TypeScript practices and makes the component more maintainable.
packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx (1)
1-36: Well-structured header component with proper accessibility.The new Header component follows a consistent pattern with other header components in the codebase. The implementation properly handles accessibility with aria-labels and maintains good component structure.
A few observations:
- The grid layout ensures proper spacing and alignment
- The component is correctly typed as FC with appropriate props
- The aria-label on the IconButton enhances accessibility
This encapsulation of the header functionality will make it easier to customize in parent components.
packages/components/modules/messages/web/ChatRoomsComponent/constants.ts (1)
1-10: Good use of constants with proper typing.The LEFT_PANEL_CONTENT constant object with the
as constassertion is an excellent approach for defining panel states. This prevents magic numbers throughout the codebase and enhances maintainability.The ValueOf utility type ensures type safety when using these constants, which is a good practice.
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1)
1-14: Overall implementation looks goodThe
BodyPropsinterface is well-structured, extendingPropsWithChildrenand providing a clear contract for components that implement it. The properties are named descriptively, and most are properly typed.packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx (1)
1-49: Well-structured and accessible component implementationThe Body component is well-designed with good separation of concerns, proper use of Material-UI components, and attention to accessibility with appropriate ARIA attributes. The layout structure using grid is clean and the component correctly handles its props.
packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx (1)
1-27: Clean and accessible header implementationThe Header component is well-implemented with good accessibility through aria-labels and a clean, focused design. The component is properly typed and follows good React functional component patterns.
packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx (1)
12-40: Component renaming looks goodThe component has been correctly renamed from
GroupDetailsHeadertoHeaderand changed to a default export, which aligns with the PR objectives to make the Messages components more customizable.packages/components/modules/messages/web/ChatRoomsComponent/types.ts (1)
12-14:HidableContainerPropsimplementation looks goodThe interface for the hidable container is well-defined, extending Material-UI's
BoxPropswith an additionalhideproperty.packages/components/modules/messages/web/GroupChatEdit/index.tsx (2)
34-36: Component renaming looks goodThe component has been correctly renamed from
EditGrouptoGroupChatEdit, which aligns with the PR objectives of making component names more consistent and clear.
221-227: Wrapped component update is consistentThe
WrappedEditGroupcomponent has been updated to use the renamedGroupChatEditcomponent consistently.packages/components/modules/messages/web/SingleChatCreate/Body/styled.tsx (3)
1-3: Good addition of type imports for enhanced type safetyThe addition of
ComponentTypefrom React andBoxPropsfrom Material-UI properly sets up the necessary types for the styled components in this file.
14-19: Well-implemented type annotation for SearchbarContainerAdding the
ComponentType<BoxProps>type annotation improves type safety while maintaining the responsive design with appropriate spacing adjustments for different screen sizes.
21-30: Well-structured GroupChatContainer with proper type annotationThe
ComponentType<BoxProps>type annotation properly defines the expected props, and the responsive grid layout with cursor styling creates a clear interactive element for users.packages/components/modules/messages/web/MessageCreate/index.tsx (4)
19-19: Consistent renaming of import typeThe import has been properly updated from
SendMessagePropstoMessageCreatePropsto align with the component rename, maintaining type consistency.
24-24: Updated JSDoc to reflect new component nameThe documentation has been correctly updated to reference "MessageCreate Component" instead of "SendMessage Component".
76-76: Renamed component for better semantic clarityRenaming from
SendMessagetoMessageCreateprovides a clearer description of the component's purpose, which aligns with the overall PR goal of improving component clarity.
133-133: Updated export statement for consistencyThe export statement has been correctly updated to match the new component name.
packages/components/modules/messages/web/GroupChatDetails/types.ts (3)
7-8: Good addition of type imports for component customizationImporting
BodyPropsandHeaderPropsproperly sets up the type definitions needed for the component customization features being added.
11-11: Improved type naming for better clarityRenaming from
GroupDetailsPropstoGroupChatDetailsPropsprovides more specific and consistent naming, which improves code readability and maintainability.
15-18: Enhanced component customization through propsThe addition of optional
Header,HeaderProps,Body, andBodyPropsproperties toGroupChatDetailsPropsdirectly addresses the PR objective of making components more customizable. This pattern allows developers to override default components and their properties, providing excellent flexibility.packages/components/modules/messages/web/SingleChatCreate/styled.tsx (4)
1-5: Well-structured imports for the styled componentsThe imports correctly include React's
ComponentTypefor typing and Material-UI's components and styling utilities, following best practices for typed styled components.
6-12: Well-designed MainContainer layoutThe
MainContaineruses a grid layout with appropriate dimensions and template rows, creating a solid foundation for the component's structure.
14-19: Responsive SearchbarContainer with proper typingThe
SearchbarContainerimplements responsive padding based on screen size and includes proper type annotation, ensuring a consistent user experience across devices.
21-30: Interactive GroupChatContainer with responsive designThe
GroupChatContainerfeatures a well-structured grid layout with appropriate spacing, cursor styling for interaction, and responsive adjustments for smaller screens.packages/components/modules/messages/web/ChatRoomsComponent/index.tsx (1)
1-135: Component implementation looks good overallThe ChatRoomsComponent is well-structured with clear separation of concerns and good use of conditional rendering based on responsive design. It allows for customization through component props, making it flexible for different use cases.
🧰 Tools
🪛 GitHub Actions: Main Workflow
[error] 92-92: Type '{ allProfilesRef: ChatRoomsQuery$data; Body?: FC | undefined; BodyProps?: Partial | undefined; ... 10 more ...; children?: ReactNode; }' is not assignable to type 'SingleChatCreateProps'.
packages/components/modules/messages/web/SingleChatCreate/types.ts (1)
8-26: Interface updates look goodThe renaming and restructuring of the interface improves clarity and aligns with the component's functionality. The addition of
Body,Header, and their respective props enhances modularity.packages/components/modules/messages/web/SingleChatCreate/index.tsx (1)
121-136: Component structure looks clean and well-organizedThe refactoring to use separate Header and Body components improves the modularity and maintainability of the code. The rendering is now more straightforward and easier to understand.
packages/components/modules/messages/web/GroupChatCreate/index.tsx (3)
23-25: Good refactoring by extracting the Header to a separate component.The import updates reflect better code organization by moving the Header to its own component and file. This promotes single responsibility and makes the code more maintainable.
27-33: Improved component naming for better clarity.Renaming from
CreateGrouptoGroupChatCreatemakes the component's purpose clearer and follows a more consistent naming convention across the module.
130-161: Enhanced component structure with better composition.The JSX structure has been significantly improved:
- Using a Box as a container provides better layout control
- The Header is now a separate component that receives its props clearly
- EditGroupTitleAndImage props are more clearly structured
This approach follows good component composition patterns and improves readability.
packages/components/modules/messages/web/ChatRoomsComponent/styled.tsx (3)
9-17: Well-designed responsive container with conditional visibility.The
ChatRoomContainerimplements responsive design principles by using theme breakpoints to conditionally display content based on viewport size and thehideprop.
19-34: Good layout implementation for the ChatRoomsContainer.This styled component effectively:
- Uses grid layout for precise arrangement
- Applies appropriate spacing using theme values
- Handles responsive behavior for different screen sizes
- Properly handles borders and margins
The responsive approach using breakpoints is particularly helpful for mobile devices.
36-54: Well-structured list container with attention to scroll behavior.The
ChatRoomsListContainerimplementation:
- Uses grid for consistent layout
- Manages overflow properly
- Handles responsive visibility through the
hideprop- Addresses landscape orientation specifically
The special handling for smaller height screens shows good attention to detail for different usage contexts.
packages/components/modules/messages/web/GroupChatDetails/index.tsx (4)
23-27: Good modularization with component imports.Importing
DefaultBodyandDefaultHeaderas separate components promotes better code organization and reusability. This follows the principle of separating concerns.
29-40: Enhanced component customization through props composition.The component now offers much better customizability by:
- Accepting optional
HeaderandBodycomponents with default implementations- Supporting props passing to these sub-components
- Maintaining backward compatibility
This approach follows the "composition over inheritance" pattern and makes the component more flexible for different use cases.
129-142: Improved component structure with cleaner prop passing.The refactored implementation:
- Uses the Header component with all necessary props and spread operator for additional props
- Passes title, avatar, and participantsCount directly to Body component
- Uses children prop pattern for renderMembers
This approach is cleaner and more maintainable than the previous implementation.
147-168: Consistent updates to SuspendedGroupDetails component.The SuspendedGroupDetails has been properly updated to match the new component structure and naming, ensuring consistency throughout the codebase.
packages/components/modules/messages/web/AllChatRoomsList/index.tsx (4)
14-17: Added necessary imports for enhanced UI components.The addition of
IconifyandButtonimports indicates improved UI with actionable components, which enhances the user experience.
27-33: Updated component name and props for better API design.Renaming to
AllChatRoomsListand adding theonHeaderClickprop with a default empty function:
- Makes the component's purpose clearer
- Enhances the component's API with an event handler
- Maintains backward compatibility with sensible defaults
This follows good React component design practices.
165-180: Added a well-structured header section for better UX.The new header section:
- Provides a clear title "Messages" using Typography
- Adds a "New" button with an appropriate icon
- Uses grid layout for proper alignment
- Connects the button to the onHeaderClick prop
This significantly improves the user experience by adding a clear navigation point and action button.
181-240: Reorganized main container structure for better component hierarchy.The main container now:
- Uses a dedicated styled component for layout consistency
- Properly spaces the searchbar with responsive padding
- Maintains the existing tabs and virtuoso list functionality
- Groups related elements logically
This restructuring improves readability and maintainability while preserving all functionality.
packages/components/modules/messages/web/index.ts (1)
3-31: Component exports reorganized for better modularity and developer experience.The changes to this index file align well with the PR objectives. The component naming is now more descriptive and consistent (e.g., AllChatRoomsList, GroupChatCreate, GroupChatEdit), making the API more intuitive. The addition of components like ChatRoomsComponent fulfills the requirement to move the ChatRooms page component into the GitHub package, which will facilitate easier updates for dependent projects.
The export structure consistently follows the pattern of exporting the component followed by its types, which is a good practice for TypeScript modules.
packages/components/modules/messages/web/SingleChatCreate/types.ts
Outdated
Show resolved
Hide resolved
pt-tsl
force-pushed
the
BA-2366-improve-messages-components
branch
from
08cdaab to
2d94d23
Compare
|
|
||
| export interface ChatRoomsComponentProps { | ||
| chatRoomsQueryData: ChatRoomsQuery$data | ||
| settings: any |
There was a problem hiding this comment.
packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx
Outdated
Show resolved
Hide resolved
packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1)
14-15: 🛠️ Refactor suggestionReplace
anytype with a proper component typeThe
Searchbarproperty is typed asanywith a TODO comment acknowledging this needs improvement. Consider properly typing this as a React component.- // TODO: type this better - Searchbar: any + Searchbar: React.ComponentType<SearchbarProps>
🧹 Nitpick comments (8)
packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx (1)
37-39: Consider optimizing the onClick handlerThe arrow function in the onClick handler will be recreated on each render. For better performance, especially if the component re-renders frequently, consider extracting this to a useCallback or direct function reference.
- onClick={() => { - onSubmit() - }} + onClick={onSubmit}packages/components/modules/messages/web/GroupChatCreate/index.tsx (1)
122-134: Clean implementation of customizable header and form componentsThe restructured JSX provides a cleaner implementation with proper prop spreading for customization. This satisfies the requirement to make the Messages components receive props that allow modification of child components.
One consideration: The header's create button is directly tied to form submission logic. While this works well for the current implementation, you might want to document this coupling for developers who customize the header.
Consider adding a brief code comment explaining that the header's create button triggers form validation and submission, which would help developers who customize this component.
packages/components/modules/messages/web/GroupChatDetails/index.tsx (3)
8-8: Ensure necessary import is properly utilizedThe Box import from MUI has been added, but it's only used once at line 63 for padding. Consider if this could be handled through the styled components instead for consistency with the rest of the module's styling approach.
49-49: Address TODO commentThere's a lingering TODO about finding a safer way to ensure the current profile ID is not undefined. This should be addressed to improve type safety.
- // TODO: Is there a safer way to ensure the current profile id is not undefined? + // Use non-null assertion or provide a fallback to handle undefined profile ID useRoomListSubscription({ - profileId, + profileId: profileId || '', connections, onRemoval: onBackButtonClicked, })
120-120: Consider addressing the TODO commentThere's a pending TODO about handling sole admin removal. If this will be addressed in another story, consider adding a ticket reference or more specific information about when this will be addressed.
- {/* TODO: Deal with sole admin removal (will be done in another story) */} + {/* TODO: Deal with sole admin removal (will be addressed in BA-XXXX) */}packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/types.ts (1)
5-9: Simplified prop interfaceThe interface has been significantly simplified by removing individual form control props (
handleRemoveImage,imageError,setValue,trigger,watch) in favor of the consolidatedformprop. This is a cleaner approach that reduces prop drilling.Consider adding a JSDoc comment to explain what this interface is used for and how it should be used, especially since this is a breaking change that might require updates in other components.
+/** + * Props for the EditGroupTitleAndImage component + * @param form - React Hook Form's UseFormReturn object that contains all form methods and state + * @param FORM_VALUE - Record mapping form field names to their keys + * @param isMutationInFlight - Boolean indicating if a mutation is currently being processed + */ export interface EditGroupTitleAndImageProps { form: UseFormReturn<TitleAndImage> FORM_VALUE: Record<keyof TitleAndImage, keyof TitleAndImage> isMutationInFlight: boolean }packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx (1)
28-34: Consider adding a test ID attribute for easier testingFor better testability, consider adding a data-testid attribute to the IconButton component.
<IconButton aria-label="return to existing chat rooms" onClick={onHeaderClick} sx={{ maxWidth: 'fit-content' }} + data-testid="single-chat-create-back-button" > <Iconify icon={backIcon} width={24} {...backIconProps} /> </IconButton>packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx (1)
34-38: Consider adding a test ID attribute to the AvatarButtonFor better testability, consider adding a data-testid attribute to the AvatarButton component.
<AvatarButton onClick={onGroupChatCreationButtonClicked} caption="New Group" Icon={AvatarButtonIcon} + data-testid="group-chat-creation-button" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
packages/components/modules/messages/web/ChatRoomsComponent/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatCreate/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatCreate/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Body/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatDetails/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatEdit/index.tsx(4 hunks)packages/components/modules/messages/web/GroupChatEdit/types.ts(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/types.ts(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts(1 hunks)packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/index.tsx(2 hunks)packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/types.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/components/modules/messages/web/GroupChatEdit/index.tsx
- packages/components/modules/messages/web/ChatRoomsComponent/types.ts
- packages/components/modules/messages/web/GroupChatCreate/types.ts
🧰 Additional context used
🧬 Code Definitions (9)
packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (3)
packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (1)
HeaderProps(5-12)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1)
HeaderProps(5-11)packages/components/modules/messages/web/GroupChatEdit/Header/types.ts (1)
HeaderProps(5-14)
packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (3)
packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (1)
HeaderProps(5-13)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (1)
HeaderProps(5-12)packages/components/modules/messages/web/GroupChatEdit/Header/types.ts (1)
HeaderProps(5-14)
packages/components/modules/messages/web/GroupChatDetails/Body/types.ts (1)
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1)
BodyProps(7-17)
packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (3)
packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (1)
HeaderProps(5-13)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1)
HeaderProps(5-11)packages/components/modules/messages/web/GroupChatEdit/Header/types.ts (1)
HeaderProps(5-14)
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1)
packages/components/modules/messages/web/GroupChatDetails/Body/types.ts (1)
BodyProps(5-12)
packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx (5)
packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (1)
HeaderProps(5-13)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (1)
HeaderProps(5-12)packages/components/modules/messages/web/GroupChatEdit/Header/types.ts (1)
HeaderProps(5-14)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1)
HeaderProps(5-11)packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx (1)
Container(4-10)
packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx (2)
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1)
BodyProps(7-17)packages/components/modules/messages/web/SingleChatCreate/Body/styled.tsx (2)
MainContainer(6-12)SearchbarContainer(14-19)
packages/components/modules/messages/web/GroupChatCreate/index.tsx (3)
packages/components/modules/messages/web/GroupChatCreate/types.ts (1)
GroupChatCreateProps(8-18)packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (1)
HeaderProps(5-13)packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/types.ts (1)
EditGroupTitleAndImageProps(5-9)
packages/components/modules/messages/web/GroupChatEdit/types.ts (4)
packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (1)
HeaderProps(5-13)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (1)
HeaderProps(5-12)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1)
HeaderProps(5-11)packages/components/modules/messages/web/GroupChatEdit/Header/types.ts (1)
HeaderProps(5-14)
🔇 Additional comments (31)
packages/components/modules/messages/web/GroupChatDetails/Body/types.ts (1)
5-12: Well-structured interface with flexible customization optionsThe
BodyPropsinterface is well-designed, extendingPropsWithChildrento allow for child components and providing several optional props for customization. This aligns well with the PR objective of making components more customizable and developer-friendly.The inclusion of styling props like
participantsCountStyleandtitlePropsof typeTypographyPropsgives developers granular control over the appearance of text elements, which is a good practice.packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (1)
5-12: Good interface design with appropriate icon customizationThe
HeaderPropsinterface provides good flexibility with optional icon customization throughbackIconandbackIconProps, and allows for a customEditIconcomponent. The use ofVoidFunctionfor callbacks is a clean approach for event handlers.This interface aligns with the PR objectives by allowing developers to customize the appearance and behavior of the header component.
packages/components/modules/messages/web/GroupChatEdit/Header/types.ts (1)
5-14: Well-structured interface with appropriate mutation state handlingThe
HeaderPropsinterface is well-designed with appropriate properties for the group chat edit context. The inclusion ofisMutationInFlightandisEditButtonDisabledproperties allows for proper UI state management during edit operations.The optional component props (
CloseIconandEditIcon) enhance customizability, which aligns with the PR objectives of making components more developer-friendly.packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (1)
5-13: Consistent interface with good component customization optionsThe
HeaderPropsinterface maintains a consistent pattern with the other header interfaces while providing specific properties relevant to the group chat creation context. The clear separation of action handlers (onCreateButtonClickedandonBackButtonClicked) promotes good component design.Allowing custom icon components through the
CloseIconandCreateIconprops enhances the customizability of the component, aligning with the PR objectives.packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx (1)
16-44: Well-structured component with good customization options!The Header component is well-designed with appropriate customization options via props. The default icons, title text, and the ability to customize button states make this component flexible for different use cases.
packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/index.tsx (3)
14-20: Improved form handling approachThe switch from a direct
controlprop to a comprehensiveformprop with destructuring of multiple form utilities is a good enhancement. This approach gives the component more flexibility in form manipulation.
21-28: Clean implementation of image removalThe
handleRemoveImagefunction properly manages both error clearing and value setting with appropriate validation options.
33-33: Safer error state handlingThe update to use
getFieldState()?.erroris safer than directly accessing the error with a non-null assertion operator.packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx (2)
14-54: Well-structured and customizable Body componentThe Body component has a clean layout using grid and provides good customization options through props. The default values for non-essential props make it easier to use while still allowing for customization when needed.
39-50: Accessible group members sectionGreat job adding appropriate ARIA attributes to the members list for accessibility. The role="list" and aria-label="group members" ensure screen readers can properly identify and announce this section.
packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx (2)
16-36: Customizable header with configurable iconsThe Header component implementation is clean and allows for good customization. The ability to override the default icons via props is especially useful for different design contexts.
26-28: Good accessibility for cancel buttonThe aria-label on the cancel button provides clear context for screen readers about the button's purpose.
packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx (4)
12-15: Good implementation of customizable iconsThe refactoring allows for customization of both the back icon and edit icon through props, making the component more flexible and reusable. This successfully addresses the previous review comment about icon customization.
26-26: Well-implemented icon customizationGood job using the Iconify component with customizable icon prop and spreading additional properties. This approach provides excellent flexibility for consumers of this component.
37-37: Clean implementation of customizable edit iconThe EditIcon prop with PenEditIcon as default nicely enables customization while maintaining backward compatibility.
43-43: Good export patternChanging to a default export aligns with component naming and makes imports cleaner in parent components.
packages/components/modules/messages/web/GroupChatDetails/types.ts (1)
11-16: Well-structured component customization propsThe type definition with optional Body and Header components along with their respective props allows for excellent component composition and customization. This matches the PR objective of making Messages components more customizable and developer-friendly.
packages/components/modules/messages/web/GroupChatCreate/index.tsx (3)
27-37: Good implementation of component customizationThe component now accepts customizable subcomponents with sensible defaults, which addresses the previous review comment and aligns with the PR objectives. The naming convention is also more consistent and descriptive.
149-149: Good use of Material-UI Box componentUsing the Material-UI Box component instead of a custom container simplifies the component structure and provides better integration with the Material-UI ecosystem.
153-153: Appropriate component renamingThe renamed export matches the component name and follows a more consistent naming convention across the codebase.
packages/components/modules/messages/web/GroupChatEdit/types.ts (1)
10-16: Well-structured type definition with customization supportThe renamed interface with added Header customization options aligns perfectly with the PR objective of improving component customization. The type structure is consistent with other components in the module.
packages/components/modules/messages/web/GroupChatDetails/index.tsx (5)
23-24: Good refactoring to support component customizationThe addition of DefaultBody and DefaultHeader components as defaults with the ability to override them through props enhances the component's flexibility and reusability. This aligns well with the PR's objective of making Messages components more customizable.
Also applies to: 30-33
29-40: Improved component APIThe refactoring from GroupDetails to GroupChatDetails with additional customization props (Body, BodyProps, Header, HeaderProps) provides better developer experience and flexibility. This matches the PR objective of making components more customizable and developer-friendly.
129-134: Good implementation of customizable Header componentThe Header implementation correctly passes both specific props and spreads additional HeaderProps, allowing for custom configuration while maintaining core functionality. This is a good pattern for component customization.
135-142: Well-structured Body component implementationThe Body component now receives explicit props (title, avatar, participantsCount) and renders children, which creates a cleaner and more maintainable structure compared to the previous implementation. This modular approach facilitates easier customization.
147-168: Consistent suspended component implementationThe SuspendedGroupDetails component properly cascades props to GroupChatDetails and maintains the same customization capabilities as the main component. This ensures a consistent API across both the suspended and loaded states.
packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/types.ts (1)
1-1: Improved form handling with UseFormReturnReplacing multiple individual form-related props with a single
form: UseFormReturn<TitleAndImage>prop is a good refactoring. This consolidates all form methods and state into a single object, making the component more maintainable and easier to use.This change streamlines the interface and follows react-hook-form best practices by passing the entire form context rather than individual pieces. Makes sure that any components using this interface are updated to match this new pattern.
Also applies to: 6-6
packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx (1)
13-41: Well-structured Header component with good customization optionsThe new Header component is well-implemented with:
- Clear layout using grid for consistent spacing
- Proper accessibility with aria-label on the back button
- Good defaults for title and icon
- Customization options through props (backIcon, backIconProps, titleProps)
packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1)
5-11: HeaderProps interface is consistent with other header componentsThe HeaderProps interface is well-defined and follows the same pattern as other header components in the project, such as GroupChatDetails/Header. It provides the necessary customization options as required by the PR objectives.
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1)
8-17: Well-structured BodyProps with good customization optionsThe BodyProps interface provides good customization options for the Body component, allowing for flexibility as required in the PR objectives.
packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx (1)
12-41: Well-implemented Body component with good defaultsThe Body component is well-implemented with appropriate defaults and clear structure. It properly forwards props to child components and uses the default Searchbar component when none is provided.
pt-tsl
force-pushed
the
BA-2366-improve-messages-components
branch
from
cdb78b7 to
cb898bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx (3)
27-29: Consider extracting hardcoded font size to a design token.The font size is hardcoded as '24px' in the SX prop. Consider using a design system token instead for better maintainability and consistency.
- <CloseIcon sx={{ fontSize: '24px' }} /> + <CloseIcon sx={{ fontSize: theme => theme.typography.pxToRem(24) }} />
30-32: Consider using a semantic heading element for the title.Using a "span" with the Typography component may not be semantically correct for a title. Consider using a heading element like "h2" or "h3" for better accessibility.
- <Typography component="span" variant="subtitle2" sx={{ textAlign: 'center' }} {...titleProps}> + <Typography component="h2" variant="subtitle2" sx={{ textAlign: 'center' }} {...titleProps}>
33-42: Simplify onClick handler and check hardcoded font size.
- The onClick handler creates a new function unnecessarily since it just calls
onSubmitwithout modification.- Similar to the CloseIcon, the font size is hardcoded.
<IconButton aria-label="Edit group" disabled={isEditButtonDisabled} isLoading={isMutationInFlight} - onClick={() => { - onSubmit() - }} + onClick={onSubmit} > - <EditIcon sx={{ fontSize: '24px' }} /> + <EditIcon sx={{ fontSize: theme => theme.typography.pxToRem(24) }} /> </IconButton>packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx (2)
14-22: Props design looks good but consider improving TypeScript typingsThe component accepts props for customization (avatar, title, styling) which aligns with the PR objective to make components more customizable. Default values are provided where appropriate.
However, the
participantsCountStyleandtitlePropscould benefit from more specific TypeScript types rather than using empty objects as defaults. Consider using the proper MUI Typography prop types.- participantsCountStyle = {}, + participantsCountStyle = {} as TypographyProps, - titleProps = {}, + titleProps = {} as TypographyProps,Also, consider importing and using the appropriate type:
+import { TypographyProps } from '@mui/material'
38-52: Consider extracting "Members" title to a prop for better customizationThe "Members" title is currently hardcoded. For maximum customization, consider making this a prop with a default value.
- <Typography - variant="subtitle2" - color="text.primary" - sx={{ - padding: theme.spacing(2), - borderBottom: `1px solid ${theme.palette.divider}`, - }} - > - Members - </Typography> + <Typography + variant="subtitle2" + color="text.primary" + sx={{ + padding: theme.spacing(2), + borderBottom: `1px solid ${theme.palette.divider}`, + }} + {...membersHeaderProps} + > + {membersHeaderText || 'Members'} + </Typography>This would require extending the BodyProps interface.
packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx (1)
25-40: Consider improving accessibility of the grid layoutThe grid layout with specific column sizes works well for layout purposes. Good inclusion of aria-labels for accessibility.
However, consider adding more semantic HTML rather than using generic Box components, especially for the empty div at the end. If the empty div is only for spacing, you could use margins instead.
- <div /> + <Box sx={{ width: '24px' }} aria-hidden="true" />This makes the purpose of the empty space clearer and improves accessibility.
packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx (1)
25-35: Icon components should accept size via propsCurrent implementation hardcodes the icon size in the sx prop. For better customization, consider accepting a size prop.
// In props + closeIconSize = '24px', + createIconSize = '24px', // In JSX - <CloseIcon sx={{ fontSize: '24px' }} /> + <CloseIcon sx={{ fontSize: closeIconSize }} /> - <CreateIcon sx={{ fontSize: '24px' }} /> + <CreateIcon sx={{ fontSize: createIconSize }} />This would enhance the component's customization capabilities, furthering the PR objective.
packages/components/modules/messages/web/ChatRoomsComponent/styled.tsx (1)
36-54: Comprehensive styling with attention to edge cases.The
ChatRoomsListContainerhandles various viewport scenarios well, including landscape orientation for shorter screens. The styling for overflows is properly managed, ensuring content remains accessible while maintaining the UI's visual integrity.One suggestion to improve the scrollbar styling:
'@media (max-height: 600px) and (orientation: landscape)': { overflowY: 'auto', scrollbarWidth: 'none', + '&::-webkit-scrollbar': { + display: 'none', + }, },This change ensures the scrollbar is hidden in Webkit browsers (Chrome, Safari) as well as Firefox.
packages/components/modules/messages/web/AllChatRoomsList/index.tsx (3)
181-240: Consider breaking down the component for better maintainability.While the functionality works well, the component is becoming quite large and handles multiple responsibilities (search, tabs, virtualized list, etc.). Consider breaking it down into smaller, more focused sub-components to improve maintainability.
For example, you could extract:
- A
SearchSectioncomponent- A
TabsSectioncomponent- A
ChatRoomListcomponent for the virtualized listThis would make the main component more concise and easier to maintain:
const AllChatRoomsList: FC<AllChatRoomsListProps> = (props) => { // state and handlers... return ( <> <Header> <HeaderContent title="Messages" buttonText="New" onButtonClick={props.onHeaderClick} /> </Header> <MainContainer> <SearchSection control={control} onChange={handleSearchChange} onClear={handleSearchClear} isPending={isPending} {...props.SearchbarProps} /> <TabsSection value={tab} onChange={handleChange} isPending={isRefetchPending} /> <ChatRoomList rooms={chatRooms} renderItem={renderItem} isLoading={isLoadingNext} hasNext={hasNext} loadNext={loadNext} emptyState={renderListContent()} {...props.VirtuosoProps} /> </MainContainer> </> ) }
98-103: Document the Virtuoso bug workaround for future maintenance.The comment explains the workaround for a Virtuoso bug, but it would be helpful to add more context for future maintenance. Consider adding:
- A link to the relevant issue in the Virtuoso repository
- Whether there's a planned fix in a future version
- Any conditions under which this workaround might be removed
// Virtuoso has a bug where it does not recognize endReached if the data is changed while the component is still mounted. // The `renderList` state is a workaround to force the component to dismount and remount, // to ensure that Virtuoso will recognized endReached everytime the tab is changed + // See: https://github.com/petyosi/react-virtuoso/issues/ISSUE_NUMBER + // TODO: Remove this workaround once the issue is fixed in Virtuoso version X.Y.Z or higher useEffect(() => { setRenderList(true) }, [data])
190-204: Improve search functionality with keyboard accessibility.The search bar implementation is good, but consider enhancing keyboard accessibility by adding proper ARIA attributes and handling keyboard events explicitly.
You could enhance the SearchbarProps to include:
{ ...SearchbarProps, 'aria-label': 'Search messages', onKeyDown: (e) => { if (e.key === 'Escape') { handleSearchClear(); } } }This would improve the user experience for keyboard and screen reader users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
packages/components/modules/messages/web/AllChatRoomsList/index.tsx(3 hunks)packages/components/modules/messages/web/AllChatRoomsList/styled.tsx(1 hunks)packages/components/modules/messages/web/AllChatRoomsList/types.ts(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/constants.ts(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/index.tsx(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/styled.tsx(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/types.ts(1 hunks)packages/components/modules/messages/web/ChatRoomsList/styled.tsx(0 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/styled.tsx(1 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatCreate/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatCreate/styled.tsx(1 hunks)packages/components/modules/messages/web/GroupChatCreate/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Body/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatDetails/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatEdit/index.tsx(4 hunks)packages/components/modules/messages/web/GroupChatEdit/types.ts(1 hunks)packages/components/modules/messages/web/GroupDetails/GroupDetailsHeader/types.ts(0 hunks)packages/components/modules/messages/web/SendMessage/__storybook__/stories.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/styled.tsx(2 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/types.ts(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/styled.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts(1 hunks)packages/components/modules/messages/web/SingleChatCreate/index.tsx(2 hunks)packages/components/modules/messages/web/SingleChatCreate/styled.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/types.ts(1 hunks)packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/index.tsx(2 hunks)packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/types.ts(1 hunks)packages/components/modules/messages/web/index.ts(2 hunks)
💤 Files with no reviewable changes (2)
- packages/components/modules/messages/web/ChatRoomsList/styled.tsx
- packages/components/modules/messages/web/GroupDetails/GroupDetailsHeader/types.ts
🚧 Files skipped from review as they are similar to previous changes (26)
- packages/components/modules/messages/web/GroupChatCreate/Header/types.ts
- packages/components/modules/messages/web/GroupChatCreate/Header/styled.tsx
- packages/components/modules/messages/web/GroupChatEdit/Header/types.ts
- packages/components/modules/messages/web/SingleChatCreate/Header/types.ts
- packages/components/modules/messages/web/SingleChatCreate/Header/styled.tsx
- packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx
- packages/components/modules/messages/web/shared/EditGroupTitleAndImage/index.tsx
- packages/components/modules/messages/web/GroupChatCreate/styled.tsx
- packages/components/modules/messages/web/GroupChatDetails/Header/types.ts
- packages/components/modules/messages/web/ChatRoomsComponent/constants.ts
- packages/components/modules/messages/web/SingleChatCreate/styled.tsx
- packages/components/modules/messages/web/AllChatRoomsList/styled.tsx
- packages/components/modules/messages/web/SendMessage/storybook/stories.tsx
- packages/components/modules/messages/web/ChatRoomsComponent/index.tsx
- packages/components/modules/messages/web/SingleChatCreate/Body/styled.tsx
- packages/components/modules/messages/web/ChatRoomsComponent/types.ts
- packages/components/modules/messages/web/SingleChatCreate/types.ts
- packages/components/modules/messages/web/GroupChatDetails/types.ts
- packages/components/modules/messages/web/GroupChatDetails/Body/types.ts
- packages/components/modules/messages/web/GroupChatCreate/types.ts
- packages/components/modules/messages/web/SingleChatCreate/Body/types.ts
- packages/components/modules/messages/web/GroupChatEdit/types.ts
- packages/components/modules/messages/web/AllChatRoomsList/types.ts
- packages/components/modules/messages/web/shared/EditGroupTitleAndImage/types.ts
- packages/components/modules/messages/web/GroupChatEdit/index.tsx
- packages/components/modules/messages/web/index.ts
🧰 Additional context used
🧬 Code Definitions (6)
packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx (2)
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1)
BodyProps(7-17)packages/components/modules/messages/web/SingleChatCreate/Body/styled.tsx (2)
MainContainer(6-12)SearchbarContainer(14-19)
packages/components/modules/messages/web/AllChatRoomsList/index.tsx (3)
packages/components/modules/messages/web/AllChatRoomsList/types.ts (1)
AllChatRoomsListProps(17-26)packages/components/modules/messages/web/AllChatRoomsList/styled.tsx (2)
Header(13-19)MainContainer(6-11)packages/components/modules/messages/web/AllChatRoomsList/constants.ts (1)
CHAT_TAB_VALUES(1-5)
packages/components/modules/messages/web/SingleChatCreate/index.tsx (3)
packages/components/modules/messages/web/SingleChatCreate/types.ts (1)
SingleChatCreateProps(12-26)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1)
HeaderProps(5-11)packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1)
BodyProps(7-17)
packages/components/modules/messages/web/GroupChatDetails/index.tsx (3)
packages/components/modules/messages/web/GroupChatDetails/types.ts (1)
GroupChatDetailsProps(11-22)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (1)
HeaderProps(5-12)packages/components/modules/messages/web/GroupChatDetails/Body/types.ts (1)
BodyProps(5-12)
packages/components/modules/messages/web/ChatRoomsComponent/styled.tsx (1)
packages/components/modules/messages/web/ChatRoomsComponent/types.ts (1)
HidableContainerProps(14-16)
packages/components/modules/messages/web/GroupChatCreate/index.tsx (3)
packages/components/modules/messages/web/GroupChatCreate/types.ts (1)
GroupChatCreateProps(8-18)packages/components/modules/messages/web/GroupChatCreate/Header/types.ts (1)
HeaderProps(5-13)packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/types.ts (1)
EditGroupTitleAndImageProps(5-9)
🔇 Additional comments (24)
packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx (3)
1-15: Looks good! Client component declaration and imports are well organized.The import structure is clean and well-organized, with external library imports properly grouped. Using default exports for icons allows for good customization options later in the component.
16-25: Component props provide good customization options.The use of default parameters for optional props is a good practice. This allows consumers to use the component with minimal configuration while still providing flexibility for customization when needed.
1-47:✅ Verification successful
Verify HeaderProps type definitions
The
HeaderPropsinterface is imported but not defined in this file. Make sure all props are properly typed and documented in the./typesfile, including:
- Required callbacks (
onCancellation,onSubmit)- Optional customization props (
CloseIcon,EditIcon,title,titleProps)- State props (
isEditButtonDisabled,isMutationInFlight)
🏁 Script executed:
#!/bin/bash # Check the HeaderProps type definition cat packages/components/modules/messages/web/GroupChatEdit/Header/types.tsLength of output: 414
HeaderProps type definitions verified: The
HeaderPropsinterface inpackages/components/modules/messages/web/GroupChatEdit/Header/types.tscorrectly defines all the expected properties:
- Required callbacks:
onCancellation,onSubmit- Optional customization props:
CloseIcon,EditIcon,title,titleProps- State props:
isEditButtonDisabled,isMutationInFlightNo modifications are needed.
packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx (1)
26-37: LGTM for the group header container implementationThe header section is well structured with responsive design principles. Using
TypographyWithEllipsisfor the title is a good choice to handle overflow.packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx (1)
13-24: Great use of default props for customizationThe component follows the PR objective of enhancing component customization by allowing props for icon, title, and styling customization.
packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx (1)
16-24: Excellent customization options for icons and titleThe implementation allows passing custom icons and title which aligns well with the PR's objective to improve component customization.
This directly addresses a previous review comment about allowing changes to the close and checkmark icons via props.
packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx (1)
12-22: Great implementation of customizable componentsThe Body component provides excellent customization options through props, allowing consumers to:
- Replace the default icons with custom ones
- Pass custom props to the Searchbar
- Control form state through control prop
- Customize event handlers
This implementation aligns perfectly with the PR objective to make components more customizable and developer-friendly.
packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx (4)
12-16: Excellent customization of both icons.The updated component now allows for customization of both the back and edit icons, addressing the previously noted feedback. This provides greater flexibility for parent components to control the visual presentation.
26-27: Good use of icon customization.The Iconify component now properly accepts the customizable backIcon and backIconProps, improving component flexibility.
37-38: Dynamic EditIcon implementation.The EditIcon is now used as a component reference rather than hardcoded, which allows for complete icon replacement when needed.
43-43: Clean export pattern.Switching to default export aligns with the component's new role as a sub-component.
packages/components/modules/messages/web/GroupChatCreate/index.tsx (3)
27-37: Well-structured component with excellent customization options.The component has been refactored to allow for greater customization through props. This makes the component more reusable and flexible, addressing the previous feedback about making header components customizable. The default components are properly set with sensible fallbacks.
122-149: Clean, modular implementation of component UI.The component structure has been significantly improved by:
- Using a Box container for proper layout
- Using a customizable Header component
- Directly passing form-related props to EditGroupTitleAndImage
- Providing proper prop spreading for customization
This implementation allows for better separation of concerns and enhanced flexibility.
153-153: Component naming is now more consistent.Renaming from CreateGroup to GroupChatCreate improves naming consistency across the module.
packages/components/modules/messages/web/SingleChatCreate/index.tsx (3)
21-35: Well-structured component with comprehensive customization options.The component has been refactored to follow a consistent pattern with other components in the module. It allows for customization of both Header and Body components, with appropriate default implementations and prop spreading. This makes the component much more flexible for different use cases.
121-137: Clean, composable component structure.The rendering structure has been improved by:
- Using a React Fragment as the container
- Separating concerns between Header and Body components
- Passing the required props to each component
- Using children pattern to render the list content
This approach makes the component more maintainable and easier to customize.
140-140: Component naming is now more consistent.Renaming from CreateChatRoomList to SingleChatCreate improves naming consistency across the module.
packages/components/modules/messages/web/GroupChatDetails/index.tsx (3)
29-40: Well-structured component with comprehensive customization options.The component has been refactored to follow a consistent pattern with other components in the module. It properly implements customizable Header and Body components with appropriate defaults, making it more flexible and reusable.
129-143: Clean, composable layout structure.The rendering structure has been improved by:
- Using separate Header and Body components
- Passing relevant props like title, avatar, and participantsCount to the Body
- Using the children pattern to render members list
This approach provides better separation of concerns and makes the component more maintainable.
147-169: Consistent updates to SuspendedGroupDetails component.The SuspendedGroupDetails component has been properly updated to align with the new naming and props structure, ensuring consistency throughout the codebase.
packages/components/modules/messages/web/ChatRoomsComponent/styled.tsx (2)
9-17: Responsive container implementation looks good.The
ChatRoomContainercomponent implements responsive behavior correctly by using the Material-UI breakpoint system. The container properly hides its content on smaller screens based on thehideprop, which allows for better mobile experiences.
19-34: Well-structured grid layout with responsive adjustments.The
ChatRoomsContainerprovides a clean grid structure that adapts nicely to different screen sizes. The responsive design is well-implemented:
- Desktop: Two-column grid with border and margin
- Mobile: Single column without borders and margins
This approach ensures a consistent user experience across devices.
packages/components/modules/messages/web/AllChatRoomsList/index.tsx (2)
30-39: Component props update improves customization.The renaming from
ChatRoomsListtoAllChatRoomsListwith corresponding prop type updates enhances clarity about the component's purpose. The addition of theonHeaderClickprop with a default implementation follows good React patterns and increases the component's flexibility.
165-180: Header design provides clear user actions.The new header section with the "Messages" title and "New" button creates a clear hierarchy and action path for users. The grid layout ensures proper alignment and spacing between elements.
The implementation of
onHeaderClickallows parent components to customize the button's behavior, aligning with the PR objective of making components more developer-friendly.
packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx
Outdated
Show resolved
Hide resolved
anicioalexandre
added
needs changes
pt-tsl
force-pushed
the
BA-2366-improve-messages-components
branch
from
cb898bd to
58b02a8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (12)
packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx (3)
35-37: Consider using a semantic heading element for better accessibility.While using Typography with component="span" works visually, for better semantics and accessibility, consider using a heading element instead.
- <Typography component="span" variant="subtitle2" sx={{ textAlign: 'center' }} {...titleProps}> + <Typography component="h1" variant="subtitle2" sx={{ textAlign: 'center' }} {...titleProps}>
13-19: Add prop type validation for required onHeaderClick.The onHeaderClick prop is required but has no validation. Consider adding a default no-op function or using TypeScript to enforce this requirement explicitly.
- onHeaderClick, + onHeaderClick = () => {},Alternatively, in your types.ts file, you could ensure it's properly marked as required:
export interface HeaderProps { backIcon?: string; backIconProps?: object; onHeaderClick: () => void; // Already marked as required title?: string; titleProps?: object; }
15-16: Consider using specific type for backIconProps.Using a generic
objecttype loses type safety. Consider using a more specific interface that matches the expected props for the Iconify component.- backIconProps = {}, + backIconProps: React.ComponentProps<typeof Iconify> = {},And in your types.ts file:
import { ComponentProps } from 'react'; import { Iconify } from '@baseapp-frontend/design-system/components/web/images'; export interface HeaderProps { backIcon?: string; backIconProps?: ComponentProps<typeof Iconify>; // other props... }packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx (4)
16-25: Component props naming could be more intuitiveThe prop
EditIconis used for a submission/confirmation action, not for editing. Similarly,isEditButtonDisabledrefers to the submission button state.Consider renaming for clarity:
- EditIcon = CheckMarkIcon, - isEditButtonDisabled, + SubmitIcon = CheckMarkIcon, + isSubmitButtonDisabled,And update related props in the component usage and types file.
33-43: Simplify the onClick handler for the submit buttonThe current implementation uses an unnecessary arrow function wrapper.
- onClick={() => { - onSubmit() - }} + onClick={onSubmit}
34-35: Inconsistent capitalization in aria-labelsThe aria-labels have inconsistent capitalization between buttons.
- aria-label="Edit group" + aria-label="edit group"
26-43: Consider making icon sizes customizableThe component uses hardcoded icon sizes which limits customization.
Consider accepting size props:
// In types.ts export interface HeaderProps { CloseIcon?: ComponentType EditIcon?: ComponentType + iconSize?: string | number isEditButtonDisabled: boolean // ...other props } // In component const Header: FC<HeaderProps> = ({ CloseIcon = DefaultCloseIcon, EditIcon = CheckMarkIcon, + iconSize = '24px', // ...other props }) => ( <Container> <IconButton onClick={onCancellation} aria-label="cancel editing group"> - <CloseIcon sx={{ fontSize: '24px' }} /> + <CloseIcon sx={{ fontSize: iconSize }} /> </IconButton> {/* ...other elements */} <IconButton {/* ...button props */} > - <EditIcon sx={{ fontSize: '24px' }} /> + <EditIcon sx={{ fontSize: iconSize }} /> </IconButton> </Container> )packages/components/modules/messages/web/GroupChatDetails/index.tsx (2)
49-49: Consider addressing TODO commentThere's a TODO comment about finding a safer way to ensure the current profile ID is not undefined. Since you're refactoring this component, it might be a good opportunity to address this concern.
A potential solution could be to use a nullish coalescing operator with a more descriptive fallback:
- const profileId = currentProfile?.id ?? '' + const profileId = currentProfile?.id ?? 'unknown-profile-id'
120-120: Consider tracking the TODO for sole admin removalThe TODO comment mentions handling sole admin removal "in another story." Consider creating a ticket/issue to track this if it doesn't already exist.
packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx (2)
26-28: Consider making the icon size configurableThe icon size is currently hardcoded to '24px'. Consider making this a prop to allow for more customization options.
- <CloseIcon sx={{ fontSize: '24px' }} /> + <CloseIcon sx={{ fontSize: iconSize || '24px', ...closeIconSxProps }} />And add these props to the interface:
iconSize?: string; closeIconSxProps?: SxProps; createIconSxProps?: SxProps;
32-34: Maintain consistent implementation between similar componentsThe implementation of this Header is different from SingleChatCreate/Header (based on the types shown in the relevant snippets). For a consistent developer experience, consider aligning the prop interfaces between different header components.
packages/components/modules/messages/web/GroupChatCreate/types.ts (1)
6-6: Consider standardizing header prop interfacesThe HeaderProps interface for GroupChatCreate/Header seems to be different from SingleChatCreate/Header (based on the relevant code snippets). For a more consistent developer experience, consider standardizing header interfaces across different components or documenting the differences clearly.
Also applies to: 14-15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
packages/components/modules/messages/web/ChatRoomsComponent/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatCreate/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatCreate/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Body/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatDetails/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatEdit/AddMembersMobile/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatEdit/index.tsx(4 hunks)packages/components/modules/messages/web/GroupChatEdit/types.ts(1 hunks)packages/components/modules/messages/web/SendMessage/__storybook__/stories.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/types.ts(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts(1 hunks)packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/index.tsx(2 hunks)packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/types.ts(1 hunks)packages/components/modules/messages/web/index.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/components/modules/messages/web/GroupChatEdit/AddMembersMobile/index.tsx
🚧 Files skipped from review as they are similar to previous changes (17)
- packages/components/modules/messages/web/GroupChatDetails/Body/types.ts
- packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx
- packages/components/modules/messages/web/GroupChatCreate/Header/types.ts
- packages/components/modules/messages/web/GroupChatEdit/Header/types.ts
- packages/components/modules/messages/web/SingleChatCreate/Header/types.ts
- packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx
- packages/components/modules/messages/web/GroupChatDetails/Header/types.ts
- packages/components/modules/messages/web/GroupChatDetails/types.ts
- packages/components/modules/messages/web/SendMessage/storybook/stories.tsx
- packages/components/modules/messages/web/SingleChatCreate/Body/types.ts
- packages/components/modules/messages/web/shared/EditGroupTitleAndImage/index.tsx
- packages/components/modules/messages/web/ChatRoomsComponent/types.ts
- packages/components/modules/messages/web/GroupChatEdit/types.ts
- packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx
- packages/components/modules/messages/web/shared/EditGroupTitleAndImage/types.ts
- packages/components/modules/messages/web/GroupChatEdit/index.tsx
- packages/components/modules/messages/web/index.ts
🧰 Additional context used
🧬 Code Definitions (3)
packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx (2)
packages/components/modules/messages/web/GroupChatEdit/Header/types.ts (1)
HeaderProps(5-14)packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx (1)
Container(4-10)
packages/components/modules/messages/web/GroupChatCreate/types.ts (2)
packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/types.ts (1)
EditGroupTitleAndImageProps(5-9)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1)
HeaderProps(5-11)
packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx (2)
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1)
BodyProps(7-17)packages/components/modules/messages/web/SingleChatCreate/Body/styled.tsx (2)
MainContainer(6-12)SearchbarContainer(14-19)
🔇 Additional comments (20)
packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx (5)
12-22: Well-structured component with sensible defaults and destructured propsThe component is well-organized with clear prop destructuring and appropriate default values. The component accepts customizable elements like
AvatarButtonIconandSearchbar, which aligns with the PR objectives of making components more customizable.
20-21: Consider improving type safety for Searchbar and SearchbarPropsThe types.ts file has a TODO comment about typing the Searchbar prop better. Instead of using
any, consider usingReact.ComponentType<SearchbarPropsType>for better type safety.- Searchbar: any + Searchbar: React.ComponentType<SearchbarPropsType>Where
SearchbarPropsTypewould be the actual type of props that the Searchbar component accepts.
36-37: Make "New Group" caption customizableThe "New Group" text is hardcoded while most other aspects of the component are customizable. Consider making this a prop with a default value to maintain consistency with the customization approach used elsewhere.
// In props definition (line 12-22) + groupButtonCaption = 'New Group', // In JSX (line 36) - caption="New Group" + caption={groupButtonCaption}
25-32: Props spreading pattern correctly implementedThe component properly spreads the
SearchbarPropsafter explicitly setting the required props. This is a good pattern that allows for props overriding while maintaining required functionality.
23-40: Clean component structure with children supportThe overall component structure is clean, with logical organization of the search container, avatar button, and children elements. The inclusion of
childrenprop supports further customization by parent components.packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx (2)
19-41: Component structure is well-organized with good layout.The grid-based layout provides good structure with proper alignment. The empty div at line 38 is intentionally used as a spacer to maintain the three-column layout and keep the title centered, which is a valid approach.
28-34: Good accessibility implementation with aria-label.The IconButton correctly includes an aria-label, which ensures screen reader users understand its purpose.
packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx (3)
27-29: LGTM: Clear cancel button implementationThe cancel button implementation is well-structured with appropriate aria-label for accessibility.
30-32: LGTM: Typography implementation with flexible propsGood implementation allowing for title customization while providing sensible defaults.
1-47: LGTM: Overall component structureThe component is well-structured and follows modern React practices with clear separation of concerns. The code is clean, modular, and provides good customization options through props.
packages/components/modules/messages/web/GroupChatDetails/index.tsx (4)
23-27: Good modularization of componentsThe new imports for DefaultBody and DefaultHeader along with the updated type import reflect a well-structured modular approach that aligns with the PR objective of making components more customizable.
29-38: Well-implemented component customization patternThe refactored component signature makes excellent use of default props, allowing consumers to:
- Use the component as-is with sensible defaults
- Customize specific aspects by providing custom props
- Replace entire subcomponents with custom implementations
This implementation directly addresses the PR objective of making the Messages components more customizable and developer-friendly.
129-143: Excellent component composition patternThe implementation of separate Header and Body components with prop forwarding is a clean pattern that:
- Improves component reusability
- Makes the component structure more intuitive
- Enables targeted customization
- Maintains clear parent-child relationships
This is a significant improvement over the previous implementation.
147-168: Good consistency in SuspendedGroupDetails implementationThe updated SuspendedGroupDetails component maintains consistency with the new component structure and naming conventions. Nice work ensuring that both the main component and its suspended version offer the same customization options.
packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx (1)
16-36: Well-structured header component with customizable iconsThe implementation of the Header component follows good practices:
- Default icons are provided, but can be overridden via props (lines 17-18)
- Proper aria-labels for accessibility (lines 26, 32)
- Consistent styling with fontSize applied to both icons
- Clear title handling with default text and customizable props
This addresses the previous feedback about allowing icon customization.
packages/components/modules/messages/web/GroupChatCreate/index.tsx (4)
27-37: Good implementation of customizable componentsThe component now allows for customization of all child components (Header, EditGroupTitleAndImage, GroupChatMembersList) with sensible defaults. This addresses the previous feedback about making the header customizable.
122-134: Clean and modular header implementationThe previous inline header implementation has been replaced with a dedicated Header component, which improves modularity and makes the code more maintainable. Props are correctly passed to the component.
129-134: EditGroupTitleAndImage receives form prop directlyThe component now receives the form object directly instead of individual form properties, which aligns with the updated type definition and improves the component's flexibility.
27-27: Component name change improves consistencyRenaming from CreateGroup to GroupChatCreate follows a more consistent naming pattern across the codebase, making the purpose of the component clearer.
Also applies to: 153-153
packages/components/modules/messages/web/GroupChatCreate/types.ts (1)
8-18: Enhanced props structure for better customizationThe updated interface properly supports the PR objective of making components more customizable:
- Allows replacing child components (EditGroupTitleAndImage, GroupChatMembersList, Header)
- Provides the ability to pass props to default components
- Maintains consistent naming conventions
This implementation aligns perfectly with the goal of enhancing developer flexibility.
pt-tsl
force-pushed
the
BA-2366-improve-messages-components
branch
from
58b02a8 to
86d677e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx (1)
1-47: Great implementation of a customizable Header component!The component is well-structured and aligns with the PR objectives to make components more customizable. I like how you've implemented:
- Customizable icons with sensible defaults
- Configurable title and title props
- Proper loading state handling for the edit button
- Accessibility support through aria-labels
A few minor suggestions for improvement:
- The onClick handler for the edit button could be simplified:
- onClick={() => { - onSubmit() - }} + onClick={onSubmit}
- Consider extracting duplicate icon styling to maintain consistency:
+ const iconStyle = { fontSize: '24px' }; ... - <CloseIcon sx={{ fontSize: '24px' }} /> + <CloseIcon sx={iconStyle} /> ... - <EditIcon sx={{ fontSize: '24px' }} /> + <EditIcon sx={iconStyle} />These changes would make the code slightly more maintainable while preserving all functionality.
packages/components/modules/messages/web/GroupChatCreate/index.tsx (3)
9-9: Consider using a more semantic container than BoxUsing
Boxas a wrapper is functional but lacks semantic meaning. Consider using a more descriptive styled component or at least adding anaria-labelfor accessibility.-<Box> +<Box aria-label="Group chat creation form">Also applies to: 122-122, 149-149
33-34: Consider adding TypeScript types for custom propsThe
HeaderPropsis defaulted to an empty object without specifying its type. Consider adding explicit types for better type safety and developer experience.- HeaderProps = {}, + HeaderProps = {} as Partial<HeaderProps>,
30-31: Consider adding TypeScript types for custom propsSimilar to
HeaderProps, consider adding explicit types forEditGroupTitleAndImagePropsfor better type safety.- EditGroupTitleAndImageProps = {}, + EditGroupTitleAndImageProps = {} as Partial<EditGroupTitleAndImageProps>,packages/components/modules/messages/web/GroupChatDetails/index.tsx (1)
49-54: Consider addressing the TODO commentThere's an existing TODO comment about finding a safer way to ensure the profile ID is not undefined. Since you're making significant changes to the component structure, this might be a good opportunity to address this comment.
// TODO: Is there a safer way to ensure the current profile id is not undefined? useRoomListSubscription({ - profileId, + profileId: profileId || '', connections, onRemoval: onBackButtonClicked, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
packages/components/modules/messages/web/ChatRoomsComponent/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatCreate/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatCreate/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Body/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatDetails/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatEdit/AddMembersMobile/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatEdit/index.tsx(4 hunks)packages/components/modules/messages/web/GroupChatEdit/types.ts(1 hunks)packages/components/modules/messages/web/SendMessage/__storybook__/stories.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/types.ts(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts(1 hunks)packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/index.tsx(2 hunks)packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/types.ts(1 hunks)packages/components/modules/messages/web/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (20)
- packages/components/modules/messages/web/GroupChatEdit/AddMembersMobile/index.tsx
- packages/components/modules/messages/web/SingleChatCreate/Header/types.ts
- packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx
- packages/components/modules/messages/web/GroupChatEdit/Header/types.ts
- packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx
- packages/components/modules/messages/web/GroupChatDetails/Header/types.ts
- packages/components/modules/messages/web/SendMessage/storybook/stories.tsx
- packages/components/modules/messages/web/shared/EditGroupTitleAndImage/index.tsx
- packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx
- packages/components/modules/messages/web/GroupChatDetails/types.ts
- packages/components/modules/messages/web/GroupChatDetails/Body/types.ts
- packages/components/modules/messages/web/GroupChatCreate/types.ts
- packages/components/modules/messages/web/ChatRoomsComponent/types.ts
- packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx
- packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx
- packages/components/modules/messages/web/GroupChatCreate/Header/types.ts
- packages/components/modules/messages/web/shared/EditGroupTitleAndImage/types.ts
- packages/components/modules/messages/web/GroupChatEdit/types.ts
- packages/components/modules/messages/web/GroupChatEdit/index.tsx
- packages/components/modules/messages/web/index.ts
🧰 Additional context used
🧬 Code Definitions (1)
packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx (2)
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (1)
BodyProps(7-18)packages/components/modules/messages/web/SingleChatCreate/Body/styled.tsx (2)
MainContainer(6-12)SearchbarContainer(14-19)
🔇 Additional comments (13)
packages/components/modules/messages/web/SingleChatCreate/Body/types.ts (2)
9-9: Replaceanytype forcontrolwith a specific typeThe
controlprop is typed asany, which loses type safety. Since this appears to be related to react-hook-form, consider using a more specific type:- control: any + control: Control<any>You'll need to add the import:
- import { ChangeEventHandler, FC, PropsWithChildren } from 'react' + import { ChangeEventHandler, FC, PropsWithChildren } from 'react' + import { Control } from 'react-hook-form'
15-16: Implement better typing forSearchbarpropThe TODO comment acknowledges the need for better typing. Consider using React's
ComponentTypeto properly type this component prop:- // TODO: type this better - Searchbar: any + Searchbar: React.ComponentType<SearchbarProps>This provides type safety while maintaining flexibility, ensuring that the component passed as
Searchbaraccepts the expected props.packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx (1)
12-42: Implementation looks good!The Body component is well-structured with appropriate default values and proper prop handling. Good job implementing the customizable caption for the group chat creation button as suggested in previous reviews.
packages/components/modules/messages/web/GroupChatCreate/index.tsx (3)
27-37: Props structure looks well-designed for customizationThe component allows for customizing header and edit components with sensible defaults. This addresses the previous feedback about making the header a default component and allowing customization.
123-128: Header implementation is now properly modularizedThe header is now a separate component that can be fully customized or replaced, with key behaviors passed as props. This implementation satisfies the previous feedback about allowing custom headers with configurable icons.
129-134: Good pattern for component customizationThe pattern of providing a default component with customizable props is well-implemented here. The form and other critical values are passed directly to the component, maintaining functionality while allowing for customization.
packages/components/modules/messages/web/GroupChatDetails/index.tsx (7)
8-8: Good addition of the Box componentAdding the Box component from MUI provides a clean way to handle layout spacing in the loading state renderer.
23-24: Good refactoring to support component customizationThese imports reflect the component-based architecture that allows for customization - importing default implementations that can be overridden via props. This aligns well with the PR objective to make Messages components more developer-friendly and customizable.
Also applies to: 27-27
29-39: Great enhancement to component customizationThe component props have been restructured to support customization by allowing consumers to provide their own Body and Header implementations. This follows the composition pattern where defaults are provided but can be overridden, aligning perfectly with the PR objective of making components more customizable.
129-134: Good implementation of header customizationThe Header component is now properly receiving the implementation through props with sensible defaults. The
shouldDisplayEditButtonconditional logic is maintained, ensuring the component behavior remains consistent while allowing for customization.
135-142: Good implementation of body customizationThe Body component now receives all necessary data as props while maintaining the ability to render children (the members list). This separation of concerns makes the component more maintainable and flexible.
147-168: Proper adaptation of SuspendedGroupDetails to the new architectureThe SuspendedGroupDetails component has been updated to match the new naming and props structure, ensuring consistency. The fallback logic has been preserved while adopting the customizable approach.
120-128: Consider addressing the TODO for sole admin removalThere's a TODO comment about handling sole admin removal. While the comment indicates it will be done in another story, it might be worth checking if this story has been completed or if it should be tracked in a separate issue.
pt-tsl
force-pushed
the
BA-2366-improve-messages-components
branch
from
86d677e to
5c37ea0
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/components/modules/messages/web/ChatRoomsComponent/styled.tsx (2)
23-23: Consider using relative units for grid column width.The fixed pixel value
390pxfor the first column may not be responsive enough across all device sizes. Consider using relative units (like fr, %, or viewport units) or a more flexible approach to ensure the layout adapts well to different screen sizes.- gridTemplateColumns: '390px auto', + gridTemplateColumns: 'minmax(300px, 390px) auto',
50-53: Consider usability implications of hidden scrollbars.The media query correctly addresses landscape mode, but hiding scrollbars with
scrollbarWidth: 'none'might affect usability as users won't have visual cues that content is scrollable. Consider adding alternative scrolling indicators or keeping a minimal scrollbar.'@media (max-height: 600px) and (orientation: landscape)': { overflowY: 'auto', - scrollbarWidth: 'none', + // Consider one of these alternatives: + // 1. Keep minimal scrollbar + scrollbarWidth: 'thin', + // 2. Or add custom scrolling indicators },packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx (1)
20-40: Consider adopting a more reusable Header patternThe Header layout is quite similar across different components. Consider extracting a more generic HeaderBase component that could be shared, reducing duplication while still allowing for customization.
- <HeaderContainer> - <Box - display="grid" - width="100%" - gridTemplateColumns="24px auto 24px" - gap={1.5} - alignItems="center" - > - <IconButton - aria-label="return to existing chat rooms" - onClick={onHeaderClick} - sx={{ maxWidth: 'fit-content' }} - > - <Iconify icon={backIcon} width={24} {...backIconProps} /> - </IconButton> - <Typography component="span" variant="subtitle2" sx={{ textAlign: 'center' }} {...titleProps}> - {title} - </Typography> - <div /> - </Box> - </HeaderContainer>A common pattern across all headers (GroupChatEdit, SingleChatCreate, GroupChatCreate) could improve maintainability.
packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx (1)
32-34: Consider adding loading state for consistencyThe create button doesn't have a loading state unlike the edit button in GroupChatEdit (
isLoading={isMutationInFlight}). Consider adding this for consistency across components, especially since these buttons likely trigger similar network operations.- <IconButton aria-label="Create group" disabled={isDisabled} onClick={onCreateButtonClicked}> + <IconButton aria-label="Create group" disabled={isDisabled} isLoading={isMutationInFlight} onClick={onCreateButtonClicked}>This would require adding a new
isMutationInFlightprop to the HeaderProps interface.packages/components/modules/messages/web/AllChatRoomsList/index.tsx (1)
165-180: Added customizable header with action button.The new header implementation includes a title and a "New" button with an add icon, enhancing the component's usability. Consider making the text strings ("Messages" and "New") configurable for internationalization support.
- <Typography variant="h4" component="span"> - Messages - </Typography> + <Typography variant="h4" component="span"> + {headerTitle || "Messages"} + </Typography> - <Button - variant="contained" - color="inherit" - startIcon={<Iconify icon="mingcute:add-line" />} - onClick={onHeaderClick} - > - New - </Button> + <Button + variant="contained" + color="inherit" + startIcon={<Iconify icon={headerButtonIcon || "mingcute:add-line"} />} + onClick={onHeaderClick} + > + {headerButtonText || "New"} + </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (40)
packages/components/modules/messages/web/AllChatRoomsList/index.tsx(3 hunks)packages/components/modules/messages/web/AllChatRoomsList/styled.tsx(1 hunks)packages/components/modules/messages/web/AllChatRoomsList/types.ts(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/constants.ts(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/index.tsx(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/styled.tsx(1 hunks)packages/components/modules/messages/web/ChatRoomsComponent/types.ts(1 hunks)packages/components/modules/messages/web/ChatRoomsList/styled.tsx(0 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/styled.tsx(1 hunks)packages/components/modules/messages/web/GroupChatCreate/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatCreate/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatCreate/styled.tsx(1 hunks)packages/components/modules/messages/web/GroupChatCreate/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Body/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatDetails/index.tsx(3 hunks)packages/components/modules/messages/web/GroupChatDetails/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatEdit/AddMembersMobile/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx(1 hunks)packages/components/modules/messages/web/GroupChatEdit/Header/types.ts(1 hunks)packages/components/modules/messages/web/GroupChatEdit/index.tsx(4 hunks)packages/components/modules/messages/web/GroupChatEdit/types.ts(1 hunks)packages/components/modules/messages/web/GroupDetails/GroupDetailsHeader/types.ts(0 hunks)packages/components/modules/messages/web/SendMessage/__storybook__/stories.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/styled.tsx(2 hunks)packages/components/modules/messages/web/SingleChatCreate/Body/types.ts(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/index.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/styled.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts(1 hunks)packages/components/modules/messages/web/SingleChatCreate/index.tsx(2 hunks)packages/components/modules/messages/web/SingleChatCreate/styled.tsx(1 hunks)packages/components/modules/messages/web/SingleChatCreate/types.ts(1 hunks)packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/index.tsx(2 hunks)packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/types.ts(1 hunks)packages/components/modules/messages/web/index.ts(2 hunks)
💤 Files with no reviewable changes (2)
- packages/components/modules/messages/web/GroupDetails/GroupDetailsHeader/types.ts
- packages/components/modules/messages/web/ChatRoomsList/styled.tsx
🚧 Files skipped from review as they are similar to previous changes (29)
- packages/components/modules/messages/web/GroupChatEdit/AddMembersMobile/index.tsx
- packages/components/modules/messages/web/GroupChatDetails/Body/types.ts
- packages/components/modules/messages/web/GroupChatCreate/Header/styled.tsx
- packages/components/modules/messages/web/GroupChatDetails/Header/types.ts
- packages/components/modules/messages/web/GroupChatCreate/styled.tsx
- packages/components/modules/messages/web/AllChatRoomsList/styled.tsx
- packages/components/modules/messages/web/GroupChatEdit/index.tsx
- packages/components/modules/messages/web/SingleChatCreate/Body/types.ts
- packages/components/modules/messages/web/GroupChatEdit/types.ts
- packages/components/modules/messages/web/GroupChatEdit/Header/types.ts
- packages/components/modules/messages/web/SingleChatCreate/Header/styled.tsx
- packages/components/modules/messages/web/SingleChatCreate/Header/types.ts
- packages/components/modules/messages/web/GroupChatCreate/Header/types.ts
- packages/components/modules/messages/web/SingleChatCreate/styled.tsx
- packages/components/modules/messages/web/GroupChatCreate/index.tsx
- packages/components/modules/messages/web/ChatRoomsComponent/constants.ts
- packages/components/modules/messages/web/GroupChatDetails/types.ts
- packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx
- packages/components/modules/messages/web/shared/EditGroupTitleAndImage/index.tsx
- packages/components/modules/messages/web/SendMessage/storybook/stories.tsx
- packages/components/modules/messages/web/ChatRoomsComponent/index.tsx
- packages/components/modules/messages/web/GroupChatDetails/Header/index.tsx
- packages/components/modules/messages/web/SingleChatCreate/Body/styled.tsx
- packages/components/modules/messages/web/GroupChatDetails/Body/index.tsx
- packages/components/modules/messages/web/SingleChatCreate/Body/index.tsx
- packages/components/modules/messages/web/shared/EditGroupTitleAndImage/types.ts
- packages/components/modules/messages/web/AllChatRoomsList/types.ts
- packages/components/modules/messages/web/ChatRoomsComponent/types.ts
- packages/components/modules/messages/web/SingleChatCreate/types.ts
🧰 Additional context used
🧬 Code Definitions (5)
packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx (2)
packages/components/modules/messages/web/GroupChatEdit/Header/types.ts (1)
HeaderProps(5-14)packages/components/modules/messages/web/GroupChatEdit/Header/styled.tsx (1)
Container(4-10)
packages/components/modules/messages/web/GroupChatCreate/types.ts (2)
packages/components/modules/messages/web/__shared__/EditGroupTitleAndImage/types.ts (1)
EditGroupTitleAndImageProps(5-9)packages/components/modules/messages/web/SingleChatCreate/Header/types.ts (1)
HeaderProps(5-11)
packages/components/modules/messages/web/ChatRoomsComponent/styled.tsx (1)
packages/components/modules/messages/web/ChatRoomsComponent/types.ts (1)
HidableContainerProps(14-16)
packages/components/modules/messages/web/AllChatRoomsList/index.tsx (3)
packages/components/modules/messages/web/AllChatRoomsList/types.ts (1)
AllChatRoomsListProps(17-26)packages/components/modules/messages/web/AllChatRoomsList/styled.tsx (2)
Header(13-19)MainContainer(6-11)packages/components/modules/messages/web/AllChatRoomsList/constants.ts (1)
CHAT_TAB_VALUES(1-5)
packages/components/modules/messages/web/GroupChatDetails/index.tsx (3)
packages/components/modules/messages/web/GroupChatDetails/types.ts (1)
GroupChatDetailsProps(11-22)packages/components/modules/messages/web/GroupChatDetails/Header/types.ts (1)
HeaderProps(5-12)packages/components/modules/messages/web/GroupChatDetails/Body/types.ts (1)
BodyProps(5-12)
🔇 Additional comments (26)
packages/components/modules/messages/web/ChatRoomsComponent/styled.tsx (5)
44-44: Verify overflow behavior is intentional.Setting
overflowY: 'hidden'by default will hide any content that exceeds the container height. This could potentially hide important content from users. Ensure this is the desired behavior, especially since it's changed toautoonly in landscape mode.
9-17: LGTM! Well-implemented responsive container.The
ChatRoomContainerimplementation is clean and properly handles responsive behavior based on thehideprop for mobile viewports.
19-34: LGTM! Responsive grid layout with proper breakpoints.The
ChatRoomsContainerimplementation effectively transitions from a two-column layout to a single column on smaller devices, with appropriate border and margin adjustments.
36-49: LGTM! Grid implementation for list container.The grid template configuration properly separates the header and body content with
gridTemplateRows: 'min-content auto', and the responsive handling looks good.
1-7: LGTM! Well-organized imports.The imports are properly organized with React imports first, followed by external libraries, then internal components.
packages/components/modules/messages/web/GroupChatDetails/index.tsx (3)
29-40: Good improvement to component customizability!The refactoring to accept custom
BodyandHeadercomponents with sensible defaults makes this component more flexible for developers. This change aligns with the PR objectives of enhancing customizability.
135-142: Props passing looks greatThe component now correctly passes appropriate props to the Body component (title, avatar, participantsCount) and renders the members list as children. This structure makes it easy to customize while maintaining the core functionality.
147-168: Good consistency in the suspended componentThe suspended version has been properly updated to match the new structure and prop types. Good job maintaining consistency between the two versions.
packages/components/modules/messages/web/GroupChatEdit/Header/index.tsx (2)
16-25: Well-designed component with good defaultsThe Header component has a clean design with appropriate default values and customization options. The props interface is well thought out.
33-42: Good use of loading stateThe edit button correctly handles loading state with
isMutationInFlightwhich provides visual feedback to users during operations.packages/components/modules/messages/web/GroupChatCreate/Header/index.tsx (1)
16-24: Good implementation of customizable iconsThe component now allows changing both the close and create icons via props as suggested in a previous review comment. This provides better customization options for developers.
packages/components/modules/messages/web/GroupChatCreate/types.ts (2)
4-6: Improved import organization and naming.The component now uses more descriptive and consistent imports, enhancing readability and maintainability. The naming convention follows a clear pattern with the "GroupChat" prefix.
8-18: Well-structured interface with enhanced customization options.The renaming from
CreateGroupPropstoGroupChatCreatePropsprovides more clarity, and the addition ofEditGroupTitleAndImage,Header, and their respective props allows for better component customization. This implementation aligns perfectly with the PR objective of making Messages components more customizable and developer-friendly.packages/components/modules/messages/web/SingleChatCreate/index.tsx (4)
16-19: Improved component structure with modular imports.The addition of
DefaultBodyandDefaultHeaderimports supports the modular architecture approach, allowing for better component composition.
21-35: Enhanced component API with customization options.The component has been renamed from
CreateChatRoomListtoSingleChatCreatefor clarity, and now accepts additional props for customizing the Header and Body components. This implementation allows developers to easily modify child components, aligning with the PR objectives.
121-136: Improved component composition with dedicated Header and Body components.The render method has been refactored to use a more modular approach with separate Header and Body components. This change:
- Makes the code more maintainable
- Provides clear separation of concerns
- Allows for easier customization of individual parts
The implementation properly passes all necessary props to child components.
140-140: Consistent export naming.The export name has been updated to match the component name, maintaining consistency throughout the codebase.
packages/components/modules/messages/web/AllChatRoomsList/index.tsx (5)
14-17: Added necessary imports for enhanced UI functionality.The addition of
IconifyandButtonimports supports the new header functionality with the "New" button.
27-28: Improved component structure with styled components.The imports for
HeaderandMainContainerstyled components and updated type imports enhance the component's structure and readability.
30-33: Enhanced component API with header interaction.The component has been renamed from
ChatRoomsListtoAllChatRoomsListfor clarity, and now includes anonHeaderClickprop to support header button interactions. This allows for greater customization of the component's behavior.
181-240: Improved layout structure with MainContainer.The implementation now uses a
MainContainerstyled component with a well-structured grid layout, improving the organization of the component's UI elements. The search functionality, tabs, and virtuoso list are properly nested within this structure.
245-245: Consistent export naming.The export name has been updated to match the component name, maintaining consistency throughout the codebase.
packages/components/modules/messages/web/index.ts (4)
3-4: Added export for AllChatRoomsList component.The export for the newly renamed
AllChatRoomsListcomponent and its types has been properly added to the module exports.
9-10: Updated export for ChatRoomsComponent.The export for
ChatRoomsListhas been renamed toChatRoomsComponent, maintaining consistency with the component renaming throughout the codebase.
12-19: Updated exports for GroupChat components.The exports for various group chat components (
GroupChatCreate,GroupChatDetails,GroupChatEdit) and their types have been properly updated, reflecting the renaming of these components for better clarity and consistency.
30-31: Added export for SingleChatCreate component.The export for the newly renamed
SingleChatCreatecomponent and its types has been properly added to the module exports.
Acceptance Criteria
Enhance Messages components to be more customizable and developer-friendly.
Approvd
https://app.approvd.io/silverlogic/BA/stories/39775
Summary by CodeRabbit
New Features
Refactor
Style