web add contact to multiple groups - #405
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughAdds shared group-selection toggling and implements a web flow for selecting groups and adding a contact to multiple chat rooms. The profile summary opens the new dialog from the existing add-contact action. Native imports use the shared helper. ChangesAdd contact to groups
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ProfileSummaryBody
participant AddContactToGroupDialog
participant GroupsList
participant ChatRoomsMutation
ProfileSummaryBody->>AddContactToGroupDialog: open with contactProfileId
AddContactToGroupDialog->>GroupsList: render group selection
GroupsList->>GroupsList: search and paginate groups
AddContactToGroupDialog->>ChatRoomsMutation: submit selected roomIds
ChatRoomsMutation-->>AddContactToGroupDialog: return mutation result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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/common/__tests__/toggleGroupSelection.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. packages/components/modules/messages/common/index.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. packages/components/modules/messages/common/toggleGroupSelection.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/components/modules/messages/web/AddContactToGroupDialog/GroupsList/GroupItem/index.tsx (1)
25-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImprove keyboard accessibility for the interactive row.
The
MainContaineracts as a clickable row, but because it relies on a standarddiv(via MUI'sBox), it lacks semantic interactions such as arole="button", anaria-disabledstate, andonKeyDownhandlers for full keyboard support. While the innerCheckboxcomponent safely provides keyboard focus and interaction, users utilizing screen readers or keyboard navigation may have a degraded experience if they interact with the row container itself.Consider using MUI's
ListItemButtoninstead, which handles all of these accessibility features—includingdisabledbehavior and full keyboard navigation—out of the box.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/components/modules/messages/web/AddContactToGroupDialog/GroupsList/GroupItem/index.tsx` around lines 25 - 52, Replace the clickable MainContainer in GroupItem with MUI’s ListItemButton, preserving the existing disabled state, click handler, layout styling, and checkbox interaction. Ensure the row retains accessible button semantics and keyboard activation through ListItemButton while keeping the inner Checkbox behavior unchanged.packages/components/modules/messages/native/AddContactToGroupPage/index.tsx (1)
57-61: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider debouncing the search input.
Updating the search parameter on every keystroke triggers an immediate Relay
refetchinside theGroupsListcomponent via itsuseFocusEffectdependency. Consider debouncing this update to prevent dispatching excessive network requests while the user is rapidly typing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/components/modules/messages/native/AddContactToGroupPage/index.tsx` around lines 57 - 61, Debounce the search state update in handleSearchChange so rapid typing does not trigger a Relay refetch through GroupsList on every keystroke. Preserve the existing FORM_VALUES.search update and startTransition behavior, applying the update only after the user pauses typing and cancelling or replacing pending updates when new input arrives.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/components/modules/messages/native/AddContactToGroupPage/GroupsList/GroupListItem/index.tsx`:
- Around line 36-49: Constrain the group item text layout: in GroupListItem,
apply styles.textContainer to the wrapper, limit the title Text to one line, and
remove the manual styles.spacer view. In
packages/components/modules/messages/native/AddContactToGroupPage/GroupsList/GroupListItem/styles.ts
lines 19-21, replace the spacer style with textContainer using flex: 1.
In
`@packages/components/modules/messages/native/AddContactToGroupPage/GroupsList/index.tsx`:
- Around line 35-44: Update handleEmptyState to safely evaluate the chat room
edge count when data.chatRooms.edges is nullish by optional-chaining edges or
applying a nullish fallback before checking for an empty state. Preserve the
existing SearchNotFoundState and EmptyGroupsState behavior.
---
Nitpick comments:
In `@packages/components/modules/messages/native/AddContactToGroupPage/index.tsx`:
- Around line 57-61: Debounce the search state update in handleSearchChange so
rapid typing does not trigger a Relay refetch through GroupsList on every
keystroke. Preserve the existing FORM_VALUES.search update and startTransition
behavior, applying the update only after the user pauses typing and cancelling
or replacing pending updates when new input arrives.
In
`@packages/components/modules/messages/web/AddContactToGroupDialog/GroupsList/GroupItem/index.tsx`:
- Around line 25-52: Replace the clickable MainContainer in GroupItem with MUI’s
ListItemButton, preserving the existing disabled state, click handler, layout
styling, and checkbox interaction. Ensure the row retains accessible button
semantics and keyboard activation through ListItemButton while keeping the inner
Checkbox behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea900683-86f0-4d34-8b52-2f2923172938
📒 Files selected for processing (33)
packages/components/modules/messages/common/__tests__/toggleGroupSelection.test.tspackages/components/modules/messages/common/graphql/fragments/AddContactToGroupItem.tspackages/components/modules/messages/common/graphql/fragments/AddContactToGroupsList.tspackages/components/modules/messages/common/graphql/mutations/AddParticipantToChatRooms.tspackages/components/modules/messages/common/graphql/queries/AddContactToGroupsQuery.tspackages/components/modules/messages/common/index.tspackages/components/modules/messages/common/toggleGroupSelection.tspackages/components/modules/messages/native/AddContactToGroupPage/EmptyGroupsState/index.tsxpackages/components/modules/messages/native/AddContactToGroupPage/GroupsList/GroupListItem/index.tsxpackages/components/modules/messages/native/AddContactToGroupPage/GroupsList/GroupListItem/styles.tspackages/components/modules/messages/native/AddContactToGroupPage/GroupsList/GroupListItem/types.tspackages/components/modules/messages/native/AddContactToGroupPage/GroupsList/index.tsxpackages/components/modules/messages/native/AddContactToGroupPage/GroupsList/types.tspackages/components/modules/messages/native/AddContactToGroupPage/constants.tspackages/components/modules/messages/native/AddContactToGroupPage/index.tsxpackages/components/modules/messages/native/AddContactToGroupPage/styles.tspackages/components/modules/messages/native/AddContactToGroupPage/types.tspackages/components/modules/messages/native/SingleChatDetailsPage/Options/index.tsxpackages/components/modules/messages/native/SingleChatDetailsPage/Options/type.tspackages/components/modules/messages/native/SingleChatDetailsPage/index.tsxpackages/components/modules/messages/native/index.tspackages/components/modules/messages/web/AddContactToGroupDialog/GroupsList/GroupItem/index.tsxpackages/components/modules/messages/web/AddContactToGroupDialog/GroupsList/GroupItem/styled.tsxpackages/components/modules/messages/web/AddContactToGroupDialog/GroupsList/GroupItem/types.tspackages/components/modules/messages/web/AddContactToGroupDialog/GroupsList/index.tsxpackages/components/modules/messages/web/AddContactToGroupDialog/GroupsList/styled.tsxpackages/components/modules/messages/web/AddContactToGroupDialog/GroupsList/types.tspackages/components/modules/messages/web/AddContactToGroupDialog/constants.tspackages/components/modules/messages/web/AddContactToGroupDialog/index.tsxpackages/components/modules/messages/web/AddContactToGroupDialog/types.tspackages/components/modules/messages/web/ProfileSummary/Body/index.tsxpackages/components/modules/messages/web/index.tspackages/components/schema.graphql
a68703f to
6da1495
Compare
ad83382 to
f137c00
Compare
f137c00 to
59c9689
Compare
cf8630a to
b0f6fec
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
3 participants
Summary by CodeRabbit