feat: hide non-matching trails when an activity is selected + backfill activities#457
Conversation
…l activities PR #453 made trails activity-aware with OR logic (selecting "Biking" ADDS bikeable trails on top), but with the Trails layer on it still showed hiking-only trails (Seneca, Sand Run Parkway, Ledges). And 144 of 180 trails have no primary_activities at all. Code (frontend) — builds on #453, does not replace it: - iconUtils.js: add isActivityFilterActive() + trailPassesActivityFilter(). Untagged trails always pass (a missing tag never hides a trail); a tagged trail passes only if it matches a selected activity. - Map.jsx: trail visibility is now (showTrails || matchesActivity) && trailPassesActivityFilter(...). This keeps #453's behavior (Trails-off + activity selected surfaces matching trails) AND hides non-matching trails once an activity filter is active. Plain "Trails" browsing (all/none activities) is unchanged. Data (migration 080, idempotent): - 144 untagged trails -> 'Hiking'. - Drop 'Biking' from 11 mis-tagged footpaths (kept other tags). - Bikeable allowlist (incl. Gateway, the West Creek paved connector) keeps Hiking+Biking. Result: trails tagged Biking 21 -> 10, untagged 144 -> 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a database migration to backfill and correct trail activities, alongside frontend updates to support filtering trails on the map by activity (e.g., hiding hiking-only trails when 'Biking' is selected). The review feedback highlights a critical bug in the SQL migration where removing 'Biking' from a trail with no other activities results in a NULL value, allowing it to incorrectly bypass the filter; a fallback to 'Hiking' is recommended. Additionally, performance and safety improvements are suggested for the frontend activity filtering logic to cache redundant computations and add defensive guards.
| SET primary_activities = ( | ||
| SELECT string_agg(trim(part), ', ') | ||
| FROM unnest(string_to_array(primary_activities, ',')) AS part | ||
| WHERE lower(trim(part)) <> 'biking' | ||
| ) |
There was a problem hiding this comment.
If a trail only has 'Biking' as its primary activity, removing 'Biking' will cause the subquery to return NULL. This would set primary_activities to NULL (or empty), which in the frontend is treated as an untagged trail. Since untagged trails always pass the activity filter, this trail would incorrectly continue to show up when the "Biking" filter is active.
To prevent this, we should fallback to 'Hiking' if removing 'Biking' leaves the activities list empty.
SET primary_activities = COALESCE(
NULLIF(
(
SELECT string_agg(trim(part), ', ')
FROM unnest(string_to_array(primary_activities, ',')) AS part
WHERE lower(trim(part)) <> 'biking'
),
''
),
'Hiking'
)| /** | ||
| * True when the legend is narrowed to a subset of activity-bearing types | ||
| * (e.g. just "Biking") rather than showing everything. The Trails layer is only | ||
| * refined by activity while this is true; with all (or no) activity types | ||
| * selected it stays all-or-nothing, preserving plain "show me the trails" browsing. | ||
| */ | ||
| export function isActivityFilterActive(visibleTypes, iconConfig) { | ||
| if (!iconConfig || iconConfig.length === 0) return false; | ||
| let activityTypes = 0, selected = 0; | ||
| for (const icon of iconConfig) { | ||
| if (icon.enabled === false || !icon.activity_fallbacks) continue; | ||
| activityTypes++; | ||
| if (visibleTypes.has(icon.name)) selected++; | ||
| } | ||
| return selected > 0 && selected < activityTypes; | ||
| } | ||
|
|
||
| /** | ||
| * Whether a trail (linear feature) should remain visible under the current | ||
| * activity narrowing. Untagged trails always pass, so a missing tag never | ||
| * silently hides a trail; a tagged trail passes only if it matches a selected | ||
| * activity. Selecting "Biking" thus hides hiking-only trails while the trail | ||
| * layer is on. | ||
| */ | ||
| export function trailPassesActivityFilter(feature, visibleTypes, iconConfig) { | ||
| if (!isActivityFilterActive(visibleTypes, iconConfig)) return true; | ||
| if (!(feature.primary_activities || '').trim()) return true; | ||
| return poiMatchesActivityForTypes(feature, visibleTypes, iconConfig); | ||
| } |
There was a problem hiding this comment.
Performance & Safety Improvements
- Redundant Computations:
isActivityFilterActiveis called for every single trail on every map move/zoom and render cycle. Since its result only depends onvisibleTypesandiconConfig(which do not change per trail), we can cache the last computed result using simple reference equality checks. This avoids O(N * M) redundant loops overiconConfigduring rendering. - Defensive Programming: Added safety guards to ensure
visibleTypesis a valid Set (or has a.hasmethod) andfeatureis not null/undefined before accessing their properties.
let lastVisibleTypes = null;
let lastIconConfig = null;
let lastResult = false;
/**
* True when the legend is narrowed to a subset of activity-bearing types
* (e.g. just "Biking") rather than showing everything. The Trails layer is only
* refined by activity while this is true; with all (or no) activity types
* selected it stays all-or-nothing, preserving plain "show me the trails" browsing.
*/
export function isActivityFilterActive(visibleTypes, iconConfig) {
if (!iconConfig || iconConfig.length === 0) return false;
if (!visibleTypes || typeof visibleTypes.has !== 'function') return false;
if (visibleTypes === lastVisibleTypes && iconConfig === lastIconConfig) {
return lastResult;
}
let activityTypes = 0, selected = 0;
for (const icon of iconConfig) {
if (icon.enabled === false || !icon.activity_fallbacks) continue;
activityTypes++;
if (visibleTypes.has(icon.name)) selected++;
}
lastVisibleTypes = visibleTypes;
lastIconConfig = iconConfig;
lastResult = selected > 0 && selected < activityTypes;
return lastResult;
}
/**
* Whether a trail (linear feature) should remain visible under the current
* activity narrowing. Untagged trails always pass, so a missing tag never
* silently hides a trail; a tagged trail passes only if it matches a selected
* activity. Selecting "Biking" thus hides hiking-only trails while the trail
* layer is on.
*/
export function trailPassesActivityFilter(feature, visibleTypes, iconConfig) {
if (!feature) return false;
if (!isActivityFilterActive(visibleTypes, iconConfig)) return true;
if (!(feature.primary_activities || '').trim()) return true;
return poiMatchesActivityForTypes(feature, visibleTypes, iconConfig);
}
Summary
Follow-up to #453. That PR made trails activity-aware with OR logic — selecting an activity adds matching trails on top — but with the Trails layer on, hiking-only trails (Seneca, Sand Run Parkway, Ledges) still showed under "Biking". Separately, 144 of 180 trails have no
primary_activitiesat all, and a dozen footpaths were mis-tagged as bikeable.This PR keeps #453's behavior and adds the missing piece: once an activity filter is active, non-matching trails are hidden.
Code (frontend) — extends #453, does not replace it
iconUtils.js:isActivityFilterActive()+trailPassesActivityFilter(). Untagged trails always pass (a missing tag never silently hides a trail); a tagged trail passes only if it matches a selected activity.Map.jsx: trail visibility is now(showTrails || matchesActivity) && trailPassesActivityFilter(...)at both render sites. Trails-off + activity-selected still surfaces matching trails (feat: add regional bikeways, fix trail hover rendering #453); an active activity filter now also hides non-matching trails. Plain "Trails" browsing (all/none activities) is unchanged.Data (
migration 080, idempotent)Hiking.Bikingfrom 11 mis-tagged footpaths (kept their other tags).Test plan
frontendproduction build passes;gourmand --full .= 0 violations.UPDATE 0 / UPDATE 0(idempotent).🤖 Generated with Claude Code