Added Room Search Functionality #561
Conversation
|
🎉 Welcome @Mayank4352!
We appreciate your contribution! 🚀 |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces live room search functionality by integrating MeiliSearch backend into RoomsController, adding reactive state management for search and filtered results, creating a new SearchOverlay UI component, updating home screen to display search UI with filtered room lists, and extending localization support across multiple languages for search-related strings. Changes
Sequence DiagramsequenceDiagram
actor User
participant HomeScreen
participant SearchOverlay
participant RoomsController
participant MeiliSearch
User->>HomeScreen: Taps search icon
HomeScreen->>HomeScreen: Show SearchOverlay
SearchOverlay->>SearchOverlay: Focus text input
User->>SearchOverlay: Type search query
SearchOverlay->>RoomsController: onSearchChanged(query)
activate RoomsController
RoomsController->>RoomsController: isSearching = true
RoomsController->>MeiliSearch: searchRooms(query)
alt MeiliSearch Available
MeiliSearch-->>RoomsController: Return hits
RoomsController->>RoomsController: convertMeilisearchResults()
else Fallback to Local Search
RoomsController->>RoomsController: Local filter
end
RoomsController->>RoomsController: updateFilteredRooms()
deactivate RoomsController
RoomsController-->>HomeScreen: filteredRooms updated
HomeScreen->>HomeScreen: Rebuild LiveRoomListView<br/>display filteredRooms
User->>SearchOverlay: Taps clear or back
SearchOverlay->>RoomsController: onClose() → clearSearch()
RoomsController->>RoomsController: Reset filters & state
HomeScreen->>HomeScreen: Hide SearchOverlay
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes introduce new search logic in the controller, integrate MeiliSearch with fallback mechanisms, add UI components with lifecycle management, and extend localization across multiple languages. While the individual pieces are straightforward, the heterogeneous nature (controller logic, UI widgets, localization entries, screen integration) and multiple file types require context-switching during review. The logic density is moderate with state management and conditional filtering logic. Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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 |
M4dhav
left a comment
M4dhav
left a comment
There was a problem hiding this comment.
Hey @Mayank4352 , I think the UI can be improved, let's move the Search button to the right of the Live and Upcoming buttons on the same bar with them and then the search text field should open as sort of a popup on the lower listview when the search button is tapped, so that the searchbar doesn't permanently take up space on the screen
WhatsApp.Video.2025-10-14.at.10.41.27_d9b3b818.mp4Hey @M4dhav I've Implemented the changes, Kindly review the changes |
|
Hey @Mayank4352 , is there a backend PR linked to this? |
|
yes, there will be |
|
Hey @M4dhav I've opened pr for meilisearch integration in backend kindly do review it aswell |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
lib/l10n/app_hi.arb (1)
863-894: Unify Hindi terms (“रूम्स” vs “कमरों”).Keep terminology consistent with existing strings that use “रूम/रूम्स”.
Apply this diff for consistency:
- "searchingRooms": "रूम खोजे जा रहे हैं...", + "searchingRooms": "रूम्स खोजे जा रहे हैं...", - "searchRoomsError": "कमरों की खोज असफल हुई। कृपया दोबारा कोशिश करें।", + "searchRoomsError": "रूम्स की खोज असफल हुई। कृपया दोबारा कोशिश करें।", - "searchUpcomingRoomsError": "आगामी कमरों की खोज असफल हुई। कृपया दोबारा कोशिश करें।", + "searchUpcomingRoomsError": "आगामी रूम्स की खोज असफल हुई। कृपया दोबारा कोशिश करें।",lib/l10n/app_localizations_hi.dart (1)
668-693: Optional: mirror the Hindi wording consistency here too.If you adopt the ARB edits, reflect them in getters.
- String get searchingRooms => 'रूम खोजे जा रहे हैं...'; + String get searchingRooms => 'रूम्स खोजे जा रहे हैं...'; - String get searchRoomsError => - 'कमरों की खोज असफल हुई। कृपया दोबारा कोशिश करें।'; + String get searchRoomsError => + 'रूम्स की खोज असफल हुई। कृपया दोबारा कोशिश करें।'; - String get searchUpcomingRoomsError => - 'आगामी कमरों की खोज असफल हुई। कृपया दोबारा कोशिश करें।'; + String get searchUpcomingRoomsError => + 'आगामी रूम्स की खोज असफल हुई। कृपया दोबारा कोशिश करें।';lib/controllers/rooms_controller.dart (6)
43-47: Avoid RxList aliasing; use assignAll/copy for filteredRooms.Setting filteredRooms.value = rooms aliases lists; updates may not notify dependents correctly.
- filteredRooms.value = rooms; + filteredRooms.assignAll(rooms);And in clear/update:
- void clearSearch() { - filteredRooms.value = rooms; + void clearSearch() { + filteredRooms.assignAll(rooms); searchBarIsEmpty.value = true; } void updateFilteredRooms() { if (searchBarIsEmpty.value) { - filteredRooms.value = rooms; + filteredRooms.assignAll(rooms); } }Also applies to: 297-306
163-233: Debounce user input to avoid flooding queries.Multiple calls per keystroke will spam Appwrite/Meili. Add a debounced worker or a Timer.
Additional code (outside this hunk):
// In controller: final RxString searchText = ''.obs; late final Worker _debounce; @override void onInit() { super.onInit(); _debounce = debounce(searchText, (q) => searchRooms(q as String), time: const Duration(milliseconds: 300)); } @override void onClose() { _debounce.dispose(); super.onClose(); } // In UI: update controller.searchText(value) instead of calling searchRooms directly.
185-186: Prefer query-object API for MeiliSearch and restrict attributes searched.Use IndexSearchQuery to future‑proof and limit scope to name/description/tags. Based on learnings.
- final meilisearchResult = await indexToUse.search(query); + final meilisearchResult = await indexToUse.search( + query, + IndexSearchQuery( + attributesToSearchOn: ['name', 'description', 'tags'], + // Optionally: showRankingScoreDetails: false, + ), + );
235-295: N+1 Appwrite reads per search hit for avatars; consider deferring or batching.Fetching participants for every hit increases latency/cost. Options:
- Defer avatars (show placeholders in search list; fetch on room open).
- Or batch by querying participants for all roomIds then map in memory (if API supports multi‑value equals).
Example (outline):
// 1) Collect ids: final ids = meilisearchHits.map((h) => h['\$id'] as String).toList(); // 2) Batch fetch participants once (if supported) and build a map<roomId, avatars>. // 3) Map hits -> AppwriteRoom using the prebuilt map without per-hit network calls.
235-295: Make converters typed; avoid dynamic.Split into two methods for clarity and type safety.
- Future<dynamic> convertMeilisearchResults( + Future<List<AppwriteRoom>> _convertLiveResults( List<Map<String, dynamic>> meilisearchHits, { - required bool isLiveRooms, - List<AppwriteUpcommingRoom>? originalUpcomingRooms, + // any extra deps }) async { ... } + List<AppwriteUpcommingRoom> _convertUpcomingResults( + List<Map<String, dynamic>> meilisearchHits, { + required List<AppwriteUpcommingRoom> originalUpcomingRooms, + }) { ... }And call the appropriate one from searchRooms.
12-12: Naming nit: “Upcomming” → “Upcoming”.If feasible in a separate PR, rename AppwriteUpcommingRoom and its file to avoid the typo and reduce future confusion.
lib/views/widgets/search_rooms.dart (2)
162-164: Redundant onChanged callback.The
onChangedcallback is unnecessary since the controller listener (lines 45-50) already invokeswidget.onSearchChangedon every text change.Apply this diff to remove the redundant callback:
decoration: InputDecoration( hintText: AppLocalizations.of(context)!.searchRooms, hintStyle: TextStyle( color: Theme.of( context, ).colorScheme.onSurface.withValues(alpha: 0.6), ), border: InputBorder.none, contentPadding: EdgeInsets.symmetric( horizontal: UiSizes.width_8, vertical: UiSizes.height_12, ), ), - onChanged: (value) { - widget.onSearchChanged(value); - }, onSubmitted: (value) { _focusNode?.unfocus(); },
194-194: Use more specific tooltip text.Consider using
clearSearchinstead ofclearfor better clarity, as this button specifically clears the search input.Apply this diff:
- tooltip: AppLocalizations.of(context)!.clear, + tooltip: AppLocalizations.of(context)!.clearSearch,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
lib/controllers/rooms_controller.dart(4 hunks)lib/l10n/app_en.arb(2 hunks)lib/l10n/app_hi.arb(1 hunks)lib/l10n/app_localizations.dart(1 hunks)lib/l10n/app_localizations_en.dart(1 hunks)lib/l10n/app_localizations_gu.dart(1 hunks)lib/l10n/app_localizations_hi.dart(1 hunks)lib/views/screens/home_screen.dart(8 hunks)lib/views/widgets/search_rooms.dart(1 hunks)
🔇 Additional comments (8)
lib/l10n/app_en.arb (2)
2-2: No action needed on locale marker change.
863-894: LGTM: added search strings/readable descriptions are consistent.lib/l10n/app_localizations_en.dart (1)
666-690: LGTM: English getters align with ARB and usage.lib/views/widgets/search_rooms.dart (1)
165-167: Clarify submit behavior.The
onSubmittedcallback only unfocuses the input. Consider whether pressing Enter should close the overlay (by calling_closeOverlay()) or remain as-is to keep the overlay open while hiding the keyboard.If the intent is to close the overlay on submit, apply this diff:
onSubmitted: (value) { - _focusNode?.unfocus(); + _closeOverlay(); },lib/l10n/app_localizations.dart (1)
1357-1403: LGTM!The new localization getters for search functionality are well-documented and follow the existing patterns in the codebase.
lib/views/screens/home_screen.dart (3)
178-248: LGTM!The search integration for upcoming rooms is well-implemented with proper empty states and clear user feedback for no results vs. no rooms.
250-336: LGTM!The live room search implementation provides good UX with loading indicators, search results, and helpful empty states.
123-176: LGTM!The search button integration in the app bar is clean and follows the existing patterns. The tooltip and styling are appropriate.
| Future<void> searchRooms( | ||
| String query, { | ||
| bool isLiveRooms = true, | ||
| List<AppwriteUpcommingRoom>? upcomingRooms, | ||
| }) async { | ||
| if (query.isEmpty) { | ||
| if (isLiveRooms) { | ||
| filteredRooms.value = rooms; | ||
| searchBarIsEmpty.value = true; | ||
| } else { | ||
| filteredUpcomingRooms.value = upcomingRooms ?? []; | ||
| } | ||
| return; | ||
| } | ||
| if (isLiveRooms) { | ||
| isSearching.value = true; | ||
| searchBarIsEmpty.value = false; | ||
| } | ||
| try { | ||
| if (isUsingMeilisearch) { | ||
| try { | ||
| final indexToUse = isLiveRooms ? roomsIndex : upcomingRoomsIndex; | ||
| final meilisearchResult = await indexToUse.search(query); | ||
|
|
||
| if (isLiveRooms) { | ||
| filteredRooms.value = await convertMeilisearchResults( | ||
| meilisearchResult.hits, | ||
| isLiveRooms: true, | ||
| ); | ||
| } else { | ||
| filteredUpcomingRooms.value = await convertMeilisearchResults( | ||
| meilisearchResult.hits, | ||
| isLiveRooms: false, | ||
| originalUpcomingRooms: upcomingRooms ?? [], | ||
| ); | ||
| } | ||
| return; | ||
| } catch (meilisearchError) { | ||
| log( | ||
| 'Meilisearch failed, falling back to local search: $meilisearchError', | ||
| ); | ||
| } | ||
| } | ||
| // Local search | ||
| final lowerQuery = query.toLowerCase(); | ||
| if (isLiveRooms) { | ||
| filteredRooms.value = rooms.where((room) { | ||
| return room.name.toLowerCase().contains(lowerQuery) || | ||
| room.description.toLowerCase().contains(lowerQuery) || | ||
| room.tags.any((tag) => tag.toLowerCase().contains(lowerQuery)); | ||
| }).toList(); | ||
| } else { | ||
| filteredUpcomingRooms.value = (upcomingRooms ?? []).where((room) { | ||
| return room.name.toLowerCase().contains(lowerQuery) || | ||
| room.description.toLowerCase().contains(lowerQuery) || | ||
| room.tags.any((tag) => tag.toLowerCase().contains(lowerQuery)); | ||
| }).toList(); | ||
| } | ||
| } catch (e) { | ||
| log('Error searching ${isLiveRooms ? 'rooms' : 'upcoming rooms'}: $e'); | ||
| if (isLiveRooms) { | ||
| filteredRooms.value = []; | ||
| } else { | ||
| filteredUpcomingRooms.value = []; | ||
| } | ||
| } finally { | ||
| if (isLiveRooms) { | ||
| isSearching.value = false; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Filter out reported rooms in search results (consistency with getRooms).
Currently, search can surface rooms that getRooms filtered out (reportedUsers contains current user).
- if (isLiveRooms) {
- filteredRooms.value = await convertMeilisearchResults(
+ if (isLiveRooms) {
+ final meiliHits = await convertMeilisearchResults(
meilisearchResult.hits,
isLiveRooms: true,
);
+ final uid = Get.find<AuthStateController>().uid!;
+ filteredRooms.assignAll(
+ meiliHits.where((r) => !r.reportedUsers.contains(uid)),
+ );
...Also apply the same filter after the local search branch if you ever remove the getRooms pre-filter:
- filteredRooms.value = rooms.where((room) {
+ final uid = Get.find<AuthStateController>().uid!;
+ filteredRooms.assignAll(rooms.where((room) {
return room.name.toLowerCase().contains(lowerQuery) ||
room.description.toLowerCase().contains(lowerQuery) ||
room.tags.any((tag) => tag.toLowerCase().contains(lowerQuery));
- }).toList();
+ }).where((r) => !r.reportedUsers.contains(uid)).toList());Committable suggestion skipped: line range outside the PR's diff.
| SearchOverlay( | ||
| isVisible: _showSearchOverlay, | ||
| onSearchChanged: (query) { | ||
| roomsController.searchRooms( | ||
| query, | ||
| isLiveRooms: isLiveSelected, | ||
| upcomingRooms: isLiveSelected | ||
| ? null | ||
| : upcomingRoomsController.upcomingRooms, | ||
| ); | ||
| }, | ||
| onClose: () { | ||
| setState(() { | ||
| _showSearchOverlay = false; | ||
| }); | ||
| roomsController.clearSearch(); | ||
| }, | ||
| isSearching: isLiveSelected | ||
| ? roomsController.isSearching.value | ||
| : false, | ||
| ), |
There was a problem hiding this comment.
Loading state missing for upcoming room search.
The isSearching parameter (lines 112-114) is only set to roomsController.isSearching.value when isLiveSelected is true. When searching upcoming rooms, isSearching is always false, so no loading indicator is shown.
Apply this diff to show loading state for both room types:
isSearching: isLiveSelected
? roomsController.isSearching.value
- : false,
+ : roomsController.isSearching.value,🤖 Prompt for AI Agents
In lib/views/screens/home_screen.dart around lines 95 to 115, the
SearchOverlay's isSearching currently only uses
roomsController.isSearching.value when isLiveSelected is true, so upcoming room
searches never show a loading state; update the isSearching argument to choose
the correct controller based on isLiveSelected (e.g. use
roomsController.isSearching.value when isLiveSelected is true, otherwise use
upcomingRoomsController.isSearching.value), ensuring you access the proper
observable/value safely to avoid null-safety issues.
|
Hey @Mayank4352 , based on. one of CodeRabbit's comments, I realised that we don't actually need the meilisearch integration for this feature, as we are not fetching the records from the server and instead just filtering the locally fetched rooms. As a result, you can remove the meilisearch integration. I believe with this the Backend PR will also become obsolete, so feel free to close that whenever |
M4dhav
left a comment
M4dhav
left a comment
There was a problem hiding this comment.
Please comment on my assessment, and if you agree, please remove meilisearch from the added functions.
|
Yes, I thought about the same at time of implementing but after seeing your integration of meilisearch, I figured maybe we'll be switching to a server in future. Sure, I'll close the PR on the backend and make the changes. |
|
Hey @M4dhav , I've removed the meilisearch integrations you can review the rest |
| RxList<AppwriteUpcommingRoom> filteredUpcomingRooms = | ||
| <AppwriteUpcommingRoom>[].obs; |
There was a problem hiding this comment.
Search Logic for Upcoming rooms should be in the Upcoming Rooms Controller
| } | ||
| } | ||
|
|
||
| Future<void> searchRooms( |
There was a problem hiding this comment.
refactor to put upcoming rooms search login in upcoming rooms controller
M4dhav
left a comment
M4dhav
left a comment
There was a problem hiding this comment.
Please run flutter gen-l10n and check the untranslated.txt file into source
|
✅ PR Closed - Thank You, @Mayank4352!
We appreciate your effort and look forward to more contributions from you! 🤝 |
|
🔔 Translation Check Notice The following untranslated language codes were found in 📣 Notifying maintainers per language: Automated post-merge notice by Translation Notifier |
2 participants
Description
Search for Live Rooms capability has been added within the rooms interface. Users should be able to type in a search query and instantly search from the list of available rooms to find the one they want to join.
Fixes #556
Type of change
How Has This Been Tested?
Tested Locally
Please include screenshots below if applicable.
WhatsApp.Video.2025-10-10.at.17.44.51_232e78de.mp4
Checklist:
Maintainer Checklist
Summary by CodeRabbit
New Features
Localization