Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a new feature that allows users to hide filtered records from the map view through a toggle switch in the map table filters. When enabled, markers that don't match the applied filters are removed from the map display, providing a cleaner visualization of filtered data.
Changes:
- Added
hideFilteredMarkersoptional boolean field to the MapViewConfig schema - Created a new Switch UI component in the table filter panel to control the visibility of filtered markers
- Updated marker rendering logic to filter out server-side unmatched markers when the feature is enabled
- Set the default value to
truefor new map views
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/server/models/MapView.ts | Added hideFilteredMarkers optional boolean field to the map view configuration schema |
| src/app/map/[id]/utils/mapView.ts | Set default value to true for the new hideFilteredMarkers field in new view configurations |
| src/app/map/[id]/components/table/MapTableFilter.tsx | Added Switch component with label to allow users to toggle hiding of filtered markers in the UI |
| src/app/map/[id]/components/Markers/index.tsx | Passed hideFilteredMarkers config value from viewConfig to DataSourceMarkers components |
| src/app/map/[id]/components/Markers/DataSourceMarkers.tsx | Implemented filtering logic to remove server-side unmatched markers from the map when feature is enabled |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| htmlFor="hide-filtered-markers" | ||
| className="text-xs text-muted-foreground whitespace-nowrap cursor-pointer" | ||
| > | ||
| Remove filtered from map |
There was a problem hiding this comment.
The label text "Remove filtered from map" is grammatically incomplete. Consider using a more complete and clear label such as "Hide filtered records from map" or "Remove filtered records from map" to improve clarity and maintain consistency with professional UI standards.
| Remove filtered from map | |
| Hide filtered records from map |
| // When hideFilteredMarkers is true, remove server-side unmatched markers | ||
| if (hideFilteredMarkers) { | ||
| features = features.filter((f) => f.properties.matched !== false); |
There was a problem hiding this comment.
The filter condition f.properties.matched !== false will keep markers where matched is true, undefined, or null. However, when hiding filtered markers, you likely want to only show markers that explicitly match the filter (i.e., matched === true). Consider changing this to f.properties.matched === true to be more precise, unless there's a specific reason to keep markers with undefined/null matched values.
| // When hideFilteredMarkers is true, remove server-side unmatched markers | |
| if (hideFilteredMarkers) { | |
| features = features.filter((f) => f.properties.matched !== false); | |
| // When hideFilteredMarkers is true, keep only server-side matched markers | |
| if (hideFilteredMarkers) { | |
| features = features.filter((f) => f.properties.matched === true); |
| const mappedFeatures = features.map((f) => ({ | ||
| ...f, | ||
| properties: { | ||
| ...f.properties, | ||
| [MARKER_CLIENT_EXCLUDED_KEY]: !recordIds.includes(f.properties.id), | ||
| }, | ||
| })); | ||
|
|
||
| return { | ||
| type: "FeatureCollection", | ||
| features: dataSourceMarkers.markers.map((f) => ({ | ||
| ...f, | ||
| properties: { | ||
| ...f.properties, | ||
| [MARKER_CLIENT_EXCLUDED_KEY]: !recordIds.includes(f.properties.id), | ||
| }, | ||
| })), | ||
| features: mappedFeatures, | ||
| }; |
There was a problem hiding this comment.
When hideFilteredMarkers is true, the code currently filters out server-side unmatched markers but still includes client-side filtered markers (marked with MARKER_CLIENT_EXCLUDED_KEY). Consider whether client-side filtered markers should also be removed from the map when this setting is enabled, rather than just marked as excluded. This would provide consistent behavior between server-side and client-side filtering.
| dataSourceMarkers={memberMarkers} | ||
| isMembers | ||
| mapConfig={mapConfig} | ||
| hideFilteredMarkers={viewConfig.hideFilteredMarkers} |
There was a problem hiding this comment.
When passing viewConfig.hideFilteredMarkers to the hideFilteredMarkers prop, consider using the nullish coalescing operator (?? false) or Boolean conversion to handle the case where the value is undefined for existing views that were created before this field was added. While the DataSourceMarkers component has a default parameter value of false, it's better to be explicit and consistent with how optional boolean config values are handled in the codebase.
| dataSourceMarkers={markers} | ||
| isMembers={false} | ||
| mapConfig={mapConfig} | ||
| hideFilteredMarkers={viewConfig.hideFilteredMarkers} |
There was a problem hiding this comment.
When passing viewConfig.hideFilteredMarkers to the hideFilteredMarkers prop, consider using the nullish coalescing operator (?? false) or Boolean conversion to handle the case where the value is undefined for existing views that were created before this field was added. While the DataSourceMarkers component has a default parameter value of false, it's better to be explicit and consistent with how optional boolean config values are handled in the codebase.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 participant
No description provided.