feat: Add search functionality to POI list#29
Open
jricardo27 wants to merge 11 commits into
Open
Conversation
Implements a search filter for the Points of Interest (POI) list within the SavedFeaturesDrawer. Key changes: - Added a TextField to the SavedFeaturesDrawer for search input. - Implemented state management for the search query. - Introduced filtering logic that searches POI names and descriptions (case-insensitive). - Refactored the filtering logic into a separate utility function `filterFeatures` in `filterUtils.ts` for better code organization and testability. - Added comprehensive unit tests for the `filterFeatures` function to ensure correctness across various scenarios, including partial matches, case-insensitivity, and handling of missing properties.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @jricardo27, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello team,
Gemini here, providing a summary of this pull request authored by @jricardo27.
This PR introduces search functionality to the Points of Interest (POI) list displayed within the Saved Features Drawer. The primary goal is to allow users to quickly filter their saved features by typing into a search box, making it easier to locate specific items, especially when dealing with a large number of saved locations.
The implementation involves adding a search input field to the drawer UI, managing the search query state, and applying filtering logic to the list of features displayed. The core filtering logic has been extracted into a dedicated utility function for better organization and testability, and comprehensive unit tests have been added for this new utility.
Highlights
- Search Input Added: A
TextFieldcomponent from Material UI has been added to the top of the Saved Features Drawer to serve as the search input field. - Search State Management: A new state variable
searchQueryis introduced inSavedFeaturesDrawer.tsxto hold the current value of the search input. - Filtering Logic Extracted: The logic for filtering features based on the search query has been moved into a new utility function
filterFeaturesinfilterUtils.ts. This function performs a case-insensitive search against thenameanddescriptionproperties of each GeoJSON feature. - Feature List Filtering: The
FeatureListcomponent within the drawer now receives thefilteredFeaturesarray (derived from applying thefilterFeaturesutility to the current category's features) instead of the full list, ensuring only matching items are displayed. - Unit Tests for Filtering: A new test file
filterUtils.test.tshas been added with extensive unit tests for thefilterFeaturesutility, covering various scenarios including partial matches, case-insensitivity, and handling features with missing name or description properties.
Changelog
Click here to see the changelog
- src/components/SavedFeaturesDrawer/SavedFeaturesDrawer.tsx
- Added import for
TextFieldfrom@mui/material. - Added import for
filterFeaturesutility function. - Introduced
searchQuerystate variable usinguseState. - Added
handleSearchChangefunction to updatesearchQuerystate. - Implemented filtering by calling
filterFeatureswith current features andsearchQuery. - Added
TextFieldcomponent to the drawer UI for search input. - Passed
filteredFeaturesto theFeatureListcomponent.
- Added import for
- src/components/SavedFeaturesDrawer/filterUtils.test.ts
- New file created for unit tests of the
filterFeaturesutility. - Includes mock GeoJsonFeature data for testing.
- Contains tests for various filtering conditions: empty query, exact/partial/case-insensitive matches in name/description, handling missing properties, and combining name/description matches.
- New file created for unit tests of the
- src/components/SavedFeaturesDrawer/filterUtils.ts
- New file created containing the
filterFeaturesutility function. - Defines a simplified
GeoJsonFeatureinterface. - Implements case-insensitive filtering logic based on feature
nameordescriptionproperties. - Handles cases where
properties,name, ordescriptionmight be missing or not strings.
- New file created containing the
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A list of points, quite long,
Search box appears, makes strong
A filter's might,
Finds just what's right,
No longer lost, where you belong.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Implements long-press functionality to open context menus on touch devices for categories and POI items. Key changes: - Added a new `useLongPress` custom hook (`src/hooks/useLongPress.tsx`) to detect long press gestures, with tolerance for minor touch movement. - Integrated `useLongPress` into `DroppableTab.tsx` to enable long-press context menus for category tabs. - Integrated `useLongPress` into `SortableFeatureItem.tsx` to enable long-press context menus for POI items. - Updated prop types for `handleContextMenu` where necessary to accept TouchEvent. This addresses Issue #2 by providing an alternative to right-click for accessing context menus on mobile devices.
jricardo27
force-pushed
the
feat/poi-search
branch
from
25ea519 to
b3a9dba
Compare
I'm resubmitting the fix for the bug where drag-and-drop functionality for POIs was broken when a search filter was active. The issue was caused by dnd-kit operating on a filtered list of items, leading to ID and index mismatches when interacting with the original unfiltered feature list in the drag event handlers. Changes: - `SavedFeaturesDrawer.tsx` now passes the full (unfiltered) list of features for the current tab and the search query to `FeatureList`. - `FeatureList.tsx` passes the search query down to `SortableFeatureItem`. - `SortableFeatureItem.tsx` is now responsible for its own visibility. It renders `null` if it doesn't match the search query, effectively hiding it visually while allowing dnd-kit to operate on the complete list of items for the category. This ensures that dnd-kit has a consistent view of all items, allowing drag-and-drop to function correctly regardless of the active search filter.
Addresses a bug where drag-and-drop functionality for POIs (especially between categories) was broken when a search filter was active. The issue was caused by dnd-kit operating on a filtered list of items, leading to ID and index mismatches when interacting with the original unfiltered feature list in the drag event handlers. Changes: - `SavedFeaturesDrawer.tsx` now passes the full (unfiltered) list of features for the current tab and the search query to `FeatureList`. - `FeatureList.tsx` passes the search query down to `SortableFeatureItem`. - `SortableFeatureItem.tsx` is now responsible for its own visibility. It renders `null` if it doesn't match the search query, effectively hiding it visually while allowing dnd-kit to operate on the complete list of items for the category. This ensures that dnd-kit has a consistent view of all items, allowing drag-and-drop to function correctly regardless of the active search filter.
Adds functionality to navigate the map to a POI selected from the
saved features list.
Key changes:
- Modified `FeatureMap.tsx` to manage a `dynamicCenter` state for the map
and introduced a `handleNavigateToCoordinates` callback. This callback
is passed down to `SavedFeaturesDrawer`.
- Threaded the `navigateToCoordinates` callback and the `onClose` (drawer)
function through `SavedFeaturesDrawer.tsx` to `FeatureList.tsx`.
- Added a "Go to on Map" button to the POI details section in
`FeatureList.tsx`.
- Implemented the button's `onClick` handler to:
- Extract coordinates from the selected POI's geometry (Point,
LineString, or Polygon).
- Convert coordinates to Leaflet's [lat, lng] format.
- Invoke the `navigateToCoordinates` callback.
- Close the features drawer.
- Updated `MapComponent.tsx` with a `useEffect` to ensure its internal
map center state updates when its `center` prop changes, allowing
programmatic map recentering.
Corrects an issue in the "Navigate to POI" functionality where coordinates stored as strings in GeoJSON features were causing navigation to fail. The onClick handler for the "Go to on Map" button in `FeatureList.tsx` has been updated to: - Use `parseFloat()` to convert longitude and latitude values to numbers. - Validate that the parsed coordinates are indeed valid numbers (`!isNaN`). - Log an error if coordinates are invalid or cannot be properly parsed.
Improves type safety and robustness of coordinate parsing in the "Navigate to POI" functionality. This addresses your feedback about avoiding the `any` type. Changes in the `onClick` handler in `FeatureList.tsx`: - Removed the `rawCoordsAny` variable. - Longitude and latitude values are now extracted into `lngStr` and `latStr` variables (typed as `string | number | undefined`). - Added more specific checks for the structure of `geometry.coordinates` (e.g., array length) before accessing elements. - Explicitly convert `lngStr` and `latStr` to strings using `String()` before passing them to `parseFloat()`. - Updated console error messages for better diagnostics.
Refactors the "Navigate to POI" functionality to use an icon button on each item in the POI list instead of a button in the details section. Key changes: - `navigateToCoordinates` and `onClose` props are now passed down to `SortableFeatureItem.tsx` from `FeatureList.tsx`. - Added an `IconButton` with `MdNearMe` icon as a `ListItemSecondaryAction` in `SortableFeatureItem.tsx`. This button is conditionally rendered if navigation is possible for the feature. - The `onClick` logic for this icon button now handles coordinate extraction (Point, LineString, Polygon), parsing, validation, calling `navigateToCoordinates`, closing the drawer, and stopping event propagation to prevent item selection. - Removed the old "Go to on Map" button from the `Collapse` section in `FeatureList.tsx`. - Updated `SortableFeatureItemProps` to include the new callbacks. This provides a more direct way to trigger navigation for a POI.
Updates `filterUtils.ts` to use the application-wide `GeoJsonFeature` type definition from `src/data/types` instead of its own simplified, less type-safe interface. This ensures that the feature filtering logic operates with the correct and more specific GeoJSON types, particularly for geometry and coordinate data, aligning it with the rest of the application and improving overall type safety.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This commit incorporates several fixes, refactorings, and tests based on your feedback:
1. **`useLongPress` Hook (Desktop Context Menu & Long Left-Click):**
- Fixed desktop right-click (onContextMenu) to correctly trigger the provided callback.
- Implemented long left-click functionality for desktop mouse users, mirroring touch behavior.
- Cleaned up stale comments.
2. **Filtering Refactor (Efficiency & DnD Stability):**
- Moved POI list filtering logic from `SortableFeatureItem` back to `SavedFeaturesDrawer`.
- Filtering in `SavedFeaturesDrawer` is now memoized using `useMemo`.
- Items passed to `FeatureList` now include their `originalIndex` from the unfiltered list.
- `SortableFeatureItem` uses this `originalIndex` for its `dnd-kit` ID, ensuring drag-and-drop stability when search filters are active.
3. **Coordinate Extraction Refactor:**
- Moved the logic for extracting navigation coordinates from GeoJSON features out of `SortableFeatureItem`.
- Created a new utility function `getCoordinatesForNavigation` in `src/utils/navigationUtils.ts` for this purpose.
- `SortableFeatureItem` now calls this utility.
4. **Code Cleanup:**
- Removed temporary development comments from `FeatureMap.tsx`.
- Removed commented-out old filtering code from `SavedFeaturesDrawer.tsx`.
5. **Unit Tests:**
- Added a comprehensive test suite for `getCoordinatesForNavigation` utility, covering various GeoJSON types and edge cases.
- Added a comprehensive test suite for the `useLongPress` hook, testing touch and mouse event handling, timer logic, and callback invocation.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Incorporates further feedback:
1. **Use `filterFeatures` utility in `SavedFeaturesDrawer`**:
- Modified `filterUtils.ts` to adapt `filterFeatures` to work with
items containing original indices (`Array<{ feature: GeoJsonFeature; originalIndex: number }>`).
- `SavedFeaturesDrawer.tsx` now uses this utility in its `useMemo`
for `filteredItems`, removing duplicated filtering logic.
2. **Remove Redundant Check in `SortableFeatureItem`**:
- Removed the `if (!navigateToCoordinates)` check from the navigation
icon's `onClick` handler in `SortableFeatureItem.tsx`. This check
was redundant because the button itself is conditionally rendered
based on the presence of `navigateToCoordinates`.
Owner
Author
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a well-implemented search and navigation functionality for the Points of Interest (POI) list. The changes are comprehensive, covering UI additions, state management, filtering logic, and map interactions.
Particularly commendable aspects include:
- Thorough Unit Testing: The new utility functions (
filterFeatures,getCoordinatesForNavigation) and theuseLongPresshook are accompanied by robust unit tests, which significantly enhances confidence in their correctness and maintainability. - Clear Separation of Concerns: Refactoring filtering logic into
filterUtils.tsand creating a reusableuseLongPresshook are excellent examples of good code organization. - Robustness: The handling of potential issues like missing properties in GeoJSON features (in
filterFeaturesandgetCoordinatesForNavigation) and the use oforiginalIndexfor stable operations on filtered lists demonstrate attention to detail. - User Experience: The addition of case-insensitive search across names and descriptions, and the
useLongPresshook for context menus on touch devices, improve the overall user experience.
The code is clean, well-structured, and the new features are thoughtfully integrated. No issues of medium, high, or critical severity were identified in the changed code.
Summary of Findings
- Code Quality and Testing: The overall code quality is high. The introduction of comprehensive unit tests for new utilities and hooks (
filterFeatures,getCoordinatesForNavigation,useLongPress) is a significant positive, ensuring robustness and easier maintenance. - Functionality Implementation: The search and POI navigation functionalities are well-implemented. The filtering logic is sound, and the map interaction for navigation is clear. The use of
useMemofor derived data (filteredItems) is appropriate for performance. - User Experience Enhancements: The
useLongPresshook is a valuable addition for improving context menu accessibility on touch devices. Case-insensitive search also enhances usability.
Merge Readiness
Based on this review, the pull request appears to be in excellent shape and implements the described features effectively and robustly. No critical or high-severity issues were found that would prevent merging. However, as an AI reviewer, I am not authorized to approve pull requests. It is recommended that another team member conduct a final review and approve the changes before merging.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements a search filter for the Points of Interest (POI) list within the SavedFeaturesDrawer.
Key changes:
filterFeaturesinfilterUtils.tsfor better code organization and testability.filterFeaturesfunction to ensure correctness across various scenarios, including partial matches, case-insensitivity, and handling of missing properties.