BA-2070 Fix chatroom search #177
Conversation
|
|
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/ChatRoomsList/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📥 CommitsReviewing files that changed from the base of the PR and between 39c5f43a35699657c1c8a153d622e1fc324bab41 and 7b84de8. 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/components/modules/messages/ChatRoomsList/index.tsx (2)
48-52: Consider extracting common refetch parameters.The refetch parameters are duplicated between
handleSearchChangeandhandleSearchClear. Consider extracting them into a helper function to improve maintainability.+const getRefetchParams = (searchText: string) => ({ + q: searchText, + unreadMessages: isInUnreadTab, + archived: isInArchivedTab, +}) const handleSearchChange: ChangeEventHandler<HTMLInputElement> = (e) => { const value = e.target.value || '' startTransition(() => { - refetch({ - q: value, - unreadMessages: isInUnreadTab, - archived: isInArchivedTab, - }) + refetch(getRefetchParams(value)) }) } const handleSearchClear = () => { startTransition(() => { reset() - refetch({ - q: '', - unreadMessages: isInUnreadTab, - archived: isInArchivedTab, - }) + refetch(getRefetchParams('')) }) }Also applies to: 59-63
168-172: Consider lifting debounce state up instead of using key prop.While using the
keyprop works as a workaround for the debounce issue, it forces a complete remount of the Searchbar component on every tab change. This might impact performance and cause unnecessary re-renders.A more robust solution would be to lift the debounce state up to the parent component or use a ref that's updated when the tab changes.
Example approach:
// Option 1: Lift debounce up const debouncedSearch = useCallback( debounce((value: string, tab: ChatTabValues) => { refetch({ q: value, unreadMessages: tab === CHAT_TAB_VALUES.unread, archived: tab === CHAT_TAB_VALUES.archived, }) }, 300), [] ) // Option 2: Update ref on tab change const searchConfigRef = useRef({ tab }) useEffect(() => { searchConfigRef.current = { tab } }, [tab])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 6340985 and 39c5f43a35699657c1c8a153d622e1fc324bab41.
📒 Files selected for processing (1)
packages/components/modules/messages/ChatRoomsList/index.tsx(3 hunks)
🔇 Additional comments (3)
packages/components/modules/messages/ChatRoomsList/index.tsx (3)
43-43: LGTM! Good use of form state tracking.The addition of
searchValueusingwatch('search')is a clean way to track the search input state and enables persistence across tab changes.
Line range hint
72-76: LGTM! Good implementation of search persistence across tabs.The inclusion of
searchValuein the tab change handler ensures that search context is maintained when switching tabs, which directly addresses the reported issue.
Line range hint
43-172: Verify that these changes fix the reported search issues.The implementation looks solid and should address the reported issues with search functionality in the Archive Chat list. Please verify:
- Search works correctly in all tabs (Active, Unread, Archived)
- Search persists when switching between tabs
- "No results found" state appears and clears correctly
- The search is case-insensitive
Let's verify the search functionality implementation:
✅ Verification successful
Search functionality implementation verified
The implementation correctly handles:
- Search state persistence across tab switches
- Consistent search behavior in all tabs (Active, Unread, Archived)
- Proper loading states and transitions
- Search clearing and reset functionality
The code structure ensures that the search value is maintained and properly synchronized during all operations, which should resolve the reported issues with search functionality in the Archive Chat list.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify search-related code implementation # Check if the search functionality is consistently implemented across all tabs echo "Checking search implementation consistency..." rg -A 2 "refetch\(" packages/components/modules/messages/ChatRoomsList/ # Verify case-insensitive search implementation echo "Checking case-sensitivity handling..." ast-grep --pattern 'q: $value' packages/components/modules/messages/ChatRoomsList/Length of output: 1093
Script:
#!/bin/bash # Check GraphQL query and form state implementation # Find GraphQL query definition echo "Checking GraphQL query..." rg -B 2 -A 5 "chatRooms\(" packages/components/modules/messages/ChatRoomsList/ # Check form state management echo "Checking form state management..." rg -B 2 "useForm|watch\(" packages/components/modules/messages/ChatRoomsList/Length of output: 1260
tsl-ps2
force-pushed
the
BA-2070-fix-chatroom-search-frontend
branch
from
39c5f43 to
7b84de8
Compare
|
Description
Acceptance Criteria
Given I am using the search bar to locate a chat room, when I type the complete name or a partial name of a chat room into the search bar, then the displayed list of chat rooms should dynamically update to show only those chat rooms whose names contain the entered text (case-insensitive match).
And if no matching chat rooms are found, an appropriate "No results found" message should be displayed.
It should filter the Chat List that is selected.
Current behavior
When I use the search bar in the Archive Chat list, is doesnt show chat rooms even tho they contain the entered text. It shows the empty state when no chat rooms were found.
This empty state persists even tho I clear the search bar.
https://www.loom.com/share/7e249f5eb0ab4814a7c3544302b1f047
Approvd
https://app.approvd.io/silverlogic/BA/stories/38023
Summary by CodeRabbit
New Features
Bug Fixes