feat: filter trails by activity + backfill trail activities#456
feat: filter trails by activity + backfill trail activities#456fatherlinux wants to merge 1 commit into
Conversation
The Trails layer ignored activity selection, so picking "Biking" still showed hiking-only trails (Seneca, Sand Run Parkway, Ledges, etc.). And 144 of 179 trails had no primary_activities at all. Code (frontend): - iconUtils.js: add isActivityFilterActive() + trailPassesActivityFilter(). Untagged trails always render (a missing tag never hides a trail); when the legend is narrowed to specific activity types, a tagged trail shows only if its primary_activities match. Plain "Trails" browsing (all/none activities) is unchanged. - Map.jsx: gate trail rendering on showTrails && trailPassesActivityFilter in both MapBoundsTracker and the render path. Data (migration 075, idempotent): - 144 untagged trails -> 'Hiking'. - Drop 'Biking' from 11 mis-tagged footpaths (kept other tags). - Bikeable allowlist keeps Hiking+Biking: Towpath, Bike & Hike, Freedom, Gateway (West Creek paved connector), Cleveland/Solon/Portage/Mud Brook multi-use trails, Advanced drop section (MTB). 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, and updates the frontend map rendering to filter trails based on the active activity selection. The review feedback highlights a performance bottleneck in isActivityFilterActive, which is called repeatedly for every trail feature during rendering. Caching the filter status and adding defensive checks for visibleTypes and feature are recommended to optimize performance and prevent potential runtime errors.
| 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 render given the current activity | ||
| * narrowing. Untagged trails always show, so a missing tag never silently hides a | ||
| * trail. (Selecting "Biking" hides hiking-only trails.) | ||
| */ | ||
| 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 Bottleneck & Defensive Programming
- Performance Bottleneck:
isActivityFilterActiveis called insidetrailPassesActivityFilterfor every single trail feature (e.g., 179 trails) during map rendering and bounds tracking. SinceisActivityFilterActiveonly depends onvisibleTypesandiconConfig(which are constant for a given render pass), this results in redundant loops overiconConfighundreds of times per render. We can optimize this by caching the last computed result using reference equality checks onvisibleTypesandiconConfig. - Defensive Programming: Added defensive checks to ensure
visibleTypesis a valid Set (or has ahasmethod) andfeatureis non-null before accessing their properties.
let lastVisibleTypes = null;
let lastIconConfig = null;
let lastResult = false;
export function isActivityFilterActive(visibleTypes, iconConfig) {
if (visibleTypes === lastVisibleTypes && iconConfig === lastIconConfig) {
return lastResult;
}
lastVisibleTypes = visibleTypes;
lastIconConfig = iconConfig;
if (!iconConfig || iconConfig.length === 0) {
lastResult = false;
return false;
}
if (!visibleTypes || typeof visibleTypes.has !== 'function') {
lastResult = false;
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++;
}
lastResult = selected > 0 && selected < activityTypes;
return lastResult;
}
/**
* Whether a trail (linear feature) should render given the current activity
* narrowing. Untagged trails always show, so a missing tag never silently hides a
* trail. (Selecting "Biking" hides hiking-only trails.)
*/
export function trailPassesActivityFilter(feature, visibleTypes, iconConfig) {
if (!feature) return true;
if (!isActivityFilterActive(visibleTypes, iconConfig)) return true;
if (!(feature.primary_activities || '').trim()) return true;
return poiMatchesActivityForTypes(feature, visibleTypes, iconConfig);
}|
Superseded — reworking on current master (production already shipped a different trail filter in #453; this redo applies the stricter hide-non-matching behavior + data backfill on top). |
Summary
Selecting an activity like Biking in the map legend left hiking-only trails (Seneca, Sand Run Parkway, Ledges, etc.) on the map — the Trails layer was only gated by its on/off toggle and never honored activity selection. Compounding it, 144 of 179 trails had no
primary_activitiesat all, and a dozen footpaths were mis-tagged as bikeable.Code (frontend)
iconUtils.js:isActivityFilterActive()+trailPassesActivityFilter(). Untagged trails always render (a missing tag never silently hides a trail); when the legend is narrowed to specific activity types, a tagged trail shows only if its activities match. Plain "Trails" browsing (all/none activities selected) is unchanged.Map.jsx: trail rendering now gated onshowTrails && trailPassesActivityFilter(...)in bothMapBoundsTrackerand the render path.Data (
migration 075, idempotent)Hiking.Bikingfrom 11 mis-tagged footpaths (kept their other tags).Test plan
UPDATE 0 / UPDATE 0(idempotent).gourmand --full .passes (0 violations).🤖 Generated with Claude Code