-
Notifications
You must be signed in to change notification settings - Fork 1
fix: POI filter matches activities and unifies icon taxonomy (#410) #423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| -- Migration 065: Unify activities table and icon activity_fallbacks | ||
| -- Adds 4 new icon types (fishing, kayaking, scenic, art), updates nature fallbacks, | ||
| -- and syncs the activities table with all icon activity_fallbacks. | ||
|
|
||
| BEGIN; | ||
|
|
||
| -- 1. Add new icons with inline SVG content | ||
| -- Each icon follows the project style: 32x32 viewBox, colored circle, white art | ||
|
|
||
| INSERT INTO icons (name, label, svg_filename, title_keywords, activity_fallbacks, sort_order) | ||
| VALUES | ||
| ('fishing', 'Fishing', 'fishing.svg', 'fish,fishing,angler', 'Fishing', 15), | ||
| ('kayaking', 'Kayaking', 'kayaking.svg', 'kayak,canoe,paddle,paddling', 'Kayaking,Boat Rides', 16), | ||
| ('scenic', 'Scenic', 'scenic.svg', 'scenic,overlook,vista,viewpoint', 'Scenic Drives', 17), | ||
| ('art', 'Art & Culture', 'art.svg', 'art,gallery,studio', 'Art', 18) | ||
| ON CONFLICT (name) DO NOTHING; | ||
|
|
||
| -- 2. Update nature icon to include Bird Watching and Photography as fallbacks | ||
| UPDATE icons | ||
| SET activity_fallbacks = 'Nature Study,Wildlife Viewing,Bird Watching,Photography' | ||
| WHERE name = 'nature'; | ||
|
|
||
| -- 3. Rename visitor-center to Discovery, add library keyword | ||
| UPDATE icons | ||
| SET label = 'Discovery', | ||
| title_keywords = 'visitor center,info,information,museum,library' | ||
| WHERE name = 'visitor-center'; | ||
|
|
||
| -- 4. Remove museum from historic keywords (visitor-center already claims it at higher priority) | ||
| UPDATE icons | ||
| SET title_keywords = 'historic,history,house,mill,lock,farm,farms' | ||
| WHERE name = 'historic'; | ||
|
|
||
| -- 5. Fix specific POIs that fall to 'default' icon type | ||
| -- Gear Up Velo → biking (it's a bike shop) | ||
| UPDATE pois SET primary_activities = 'Biking' WHERE name = 'Gear Up Velo' AND primary_activities IS NULL; | ||
|
|
||
| -- John Brown Monument → historic (it's a historical monument) | ||
| UPDATE pois SET primary_activities = 'Historical Tours' WHERE name = 'John Brown Monument' AND primary_activities IS NULL; | ||
|
|
||
| -- Quaker Square → add Historical Tours (it's a historic building) | ||
| UPDATE pois SET primary_activities = 'Photography,Historical Tours' WHERE name = 'Quaker Square' AND primary_activities = 'Photography'; | ||
|
|
||
| -- Brecksville Reservation point POI → soft-delete (boundary version exists) | ||
| UPDATE pois SET deleted = true WHERE id = 5745 AND name = 'Brecksville Reservation' AND 'point' = ANY(poi_roles); | ||
|
|
||
| -- 6. Disable the 'default/Other' icon type — all point POIs now map to real types | ||
| UPDATE icons SET enabled = false WHERE name = 'default'; | ||
|
|
||
| -- 7. Sync activities table — add missing activities that exist as icon fallbacks | ||
| INSERT INTO activities (name, sort_order) VALUES | ||
| ('Music', (SELECT COALESCE(MAX(sort_order), 0) + 1 FROM activities)), | ||
| ('Art', (SELECT COALESCE(MAX(sort_order), 0) + 2 FROM activities)), | ||
| ('Boat Rides', (SELECT COALESCE(MAX(sort_order), 0) + 3 FROM activities)) | ||
| ON CONFLICT (name) DO NOTHING; | ||
|
|
||
| COMMIT; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ import { createPortal } from 'react-dom'; | |
| import { MapContainer, TileLayer, Marker, Tooltip, useMap, GeoJSON, useMapEvents, CircleMarker } from 'react-leaflet'; | ||
| import L from 'leaflet'; | ||
| import VirtualPoiCreator from './VirtualPoiCreator'; | ||
| import { getDestinationIconTypeFromConfig } from '../utils/iconUtils'; | ||
| import { getDestinationIconTypeFromConfig, poiMatchesActivityForTypes, matchesWholeWord } from '../utils/iconUtils'; | ||
| import { useTrip } from '../hooks/useTrip'; | ||
| import { useNavigate } from 'react-router-dom'; | ||
| import { generateSlug } from './sidebar/helpers'; | ||
|
|
@@ -238,7 +238,7 @@ function Legend({ | |
| <input | ||
| type="text" | ||
| className="search-input" | ||
| placeholder="Search destinations..." | ||
| placeholder="Search by name or activity..." | ||
| value={searchQuery || ''} | ||
| onChange={(e) => onSearchChange(e.target.value)} | ||
| /> | ||
|
|
@@ -589,7 +589,7 @@ function ZoomTooltipHider() { | |
| return null; | ||
| } | ||
|
|
||
| function MapBoundsTracker({ destinations, visibleTypes, getDestinationIconType, onVisiblePoisChange, onMapStateChange, linearFeatures, showTrails, showRivers, showWaterTaxis, visibleBoundaries, searchQuery }) { | ||
| function MapBoundsTracker({ destinations, visibleTypes, getDestinationIconType, onVisiblePoisChange, onMapStateChange, linearFeatures, showTrails, showRivers, showWaterTaxis, visibleBoundaries, searchQuery, iconConfig }) { | ||
| const map = useMap(); | ||
| const search = (searchQuery || '').toLowerCase(); | ||
|
|
||
|
|
@@ -607,7 +607,7 @@ function MapBoundsTracker({ destinations, visibleTypes, getDestinationIconType, | |
| // While searching, destinations are already title-filtered upstream — count | ||
| // them regardless of category; otherwise honor the category toggles. | ||
| const iconType = getDestinationIconType(dest); | ||
| if (!search && !visibleTypes.has(iconType)) { | ||
| if (!search && !visibleTypes.has(iconType) && !poiMatchesActivityForTypes(dest, visibleTypes, iconConfig)) { | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -672,7 +672,7 @@ function MapBoundsTracker({ destinations, visibleTypes, getDestinationIconType, | |
| } | ||
| } catch { | ||
| } | ||
| }, [map, destinations, visibleTypes, getDestinationIconType, onVisiblePoisChange, onMapStateChange, linearFeatures, showTrails, showRivers, showWaterTaxis, visibleBoundaries, search]); | ||
| }, [map, destinations, visibleTypes, getDestinationIconType, onVisiblePoisChange, onMapStateChange, linearFeatures, showTrails, showRivers, showWaterTaxis, visibleBoundaries, search, iconConfig]); | ||
|
|
||
| useMapEvents({ | ||
| moveend: updateVisiblePois, | ||
|
|
@@ -1160,26 +1160,37 @@ function Map({ destinations, selectedPoi, selectedIsLinear, onSelectPoi, isAdmin | |
| // doesn't re-scan every POI on each click. Stored as [minLat,minLng,maxLat,maxLng]. | ||
| // (PR #401 review) | ||
| const typeBoundsById = useMemo(() => { | ||
| // globalThis.Map: the bare name `Map` is this file's <Map> component, so | ||
| // `new Map()` would construct the component. (PR #401 review) | ||
| const byType = new globalThis.Map(); | ||
| for (const dest of destinations) { | ||
| if (!dest.latitude || !dest.longitude) continue; | ||
| const t = getDestinationIconType(dest); | ||
| const lat = parseFloat(dest.latitude); | ||
| const lng = parseFloat(dest.longitude); | ||
| const cur = byType.get(t); | ||
| const addToBounds = (type, lat, lng) => { | ||
| const cur = byType.get(type); | ||
| if (!cur) { | ||
| byType.set(t, [lat, lng, lat, lng]); | ||
| byType.set(type, [lat, lng, lat, lng]); | ||
| } else { | ||
| if (lat < cur[0]) cur[0] = lat; | ||
| if (lng < cur[1]) cur[1] = lng; | ||
| if (lat > cur[2]) cur[2] = lat; | ||
| if (lng > cur[3]) cur[3] = lng; | ||
| } | ||
| }; | ||
| for (const dest of destinations) { | ||
| if (!dest.latitude || !dest.longitude) continue; | ||
| const t = getDestinationIconType(dest); | ||
| const lat = parseFloat(dest.latitude); | ||
| const lng = parseFloat(dest.longitude); | ||
| addToBounds(t, lat, lng); | ||
| const poiActs = (dest.primary_activities || '').toLowerCase(); | ||
| if (poiActs && iconConfig) { | ||
| for (const icon of iconConfig) { | ||
| if (icon.enabled === false || !icon.activity_fallbacks || icon.name === t) continue; | ||
| const fbs = icon.activity_fallbacks.split(',').map(a => a.trim().toLowerCase()); | ||
| if (fbs.some(fb => fb && matchesWholeWord(poiActs, fb))) { | ||
| addToBounds(icon.name, lat, lng); | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+1181
to
+1190
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a correctness discrepancy between how const poiActs = (dest.primary_activities || '').toLowerCase();
if (poiActs && iconConfig) {
const tempSet = new Set();
for (const icon of iconConfig) {
if (icon.enabled === false || !icon.activity_fallbacks || icon.name === t) continue;
tempSet.clear();
tempSet.add(icon.name);
if (poiMatchesActivityForTypes(dest, tempSet, iconConfig)) {
addToBounds(icon.name, lat, lng);
}
}
} |
||
| } | ||
| return byType; | ||
| }, [destinations, getDestinationIconType]); | ||
| }, [destinations, getDestinationIconType, iconConfig]); | ||
|
|
||
| // Fit the map to all POIs of the given icon type(s), from cached per-type bounds. | ||
| const fitToTypes = useCallback((typeIds) => { | ||
|
|
@@ -1733,6 +1744,7 @@ function Map({ destinations, selectedPoi, selectedIsLinear, onSelectPoi, isAdmin | |
| showWaterTaxis={showWaterTaxis} | ||
| visibleBoundaries={visibleBoundaries} | ||
| searchQuery={searchQuery} | ||
| iconConfig={iconConfig} | ||
| /> | ||
| <MapMoveTracker onMapMove={() => setMapMoveCount(c => c + 1)} /> | ||
| <ZoomTooltipHider /> | ||
|
|
@@ -1774,7 +1786,7 @@ function Map({ destinations, selectedPoi, selectedIsLinear, onSelectPoi, isAdmin | |
| // When a title search is active, App has already narrowed destinations to | ||
| // name matches — show them regardless of the category toggles. | ||
| const iconType = getDestinationIconType(dest); | ||
| if (!searchQuery && !visibleTypes.has(iconType)) return null; | ||
| if (!searchQuery && !visibleTypes.has(iconType) && !poiMatchesActivityForTypes(dest, visibleTypes, iconConfig)) return null; | ||
|
|
||
| const isSelected = selectedDestination?.id === dest.id; | ||
| const icon = getDestinationIcon(dest); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| function matchesWholeWord(text, keyword) { | ||
| export function matchesWholeWord(text, keyword) { | ||
| const escaped = keyword.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | ||
| const regex = new RegExp(`\\b${escaped}\\b`, 'i'); | ||
| return regex.test(text); | ||
|
|
@@ -43,6 +43,24 @@ export function getDestinationIconTypeFromConfig(destination, iconConfig) { | |
| return 'default'; | ||
| } | ||
|
|
||
| export function poiMatchesActivityForTypes(poi, visibleTypes, iconConfig) { | ||
| if (!iconConfig || iconConfig.length === 0) return false; | ||
| const poiActivities = (poi.primary_activities || '').toLowerCase(); | ||
| if (!poiActivities) return false; | ||
|
|
||
| for (const icon of iconConfig) { | ||
| if (icon.enabled === false) continue; | ||
| if (!visibleTypes.has(icon.name)) continue; | ||
| if (!icon.activity_fallbacks) continue; | ||
|
|
||
| const fallbacks = icon.activity_fallbacks.split(',').map(a => a.trim().toLowerCase()); | ||
| for (const fb of fallbacks) { | ||
| if (fb && matchesWholeWord(poiActivities, fb)) return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
Comment on lines
+46
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The export function poiMatchesActivityForTypes(poi, visibleTypes, iconConfig) {
if (!iconConfig || iconConfig.length === 0) return false;
const poiActivities = (poi.primary_activities || '').toLowerCase();
if (!poiActivities) return false;
if (!globalThis._iconFallbackCache) {
globalThis._iconFallbackCache = new WeakMap();
}
const cache = globalThis._iconFallbackCache;
for (const icon of iconConfig) {
if (icon.enabled === false) continue;
if (!visibleTypes.has(icon.name)) continue;
if (!icon.activity_fallbacks) continue;
let fallbacks = cache.get(icon);
if (!fallbacks) {
fallbacks = icon.activity_fallbacks.split(',').map(a => a.trim().toLowerCase());
cache.set(icon, fallbacks);
}
for (const fb of fallbacks) {
if (fb && matchesWholeWord(poiActivities, fb)) return true;
}
}
return false;
} |
||
|
|
||
| export function getIconUrlForPOI(poi, iconConfig, poiType) { | ||
| if (poiType === 'trail') return '/icons/layers/trails.svg'; | ||
| if (poiType === 'river') return '/icons/layers/rivers.svg'; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoding specific database IDs (like
5745) in migrations is risky because IDs can differ across environments (development, staging, production) due to different insertion orders or manual edits. Since you are already filtering bynameandpoi_roles, you can safely remove the hardcodedidfilter to make the migration robust across all environments.