Observation Only Zones

[kevinherdez]

This takes the overall changes from #904, adds them to the latest commit on main, and iterates on it. The main idea of the PR as well as the useAlternateObservationZones hook are largely the same. However, there are a few key differences.

PR Differences

Merged Map Layer Removal

I removed useMergedMapLayer which called useAlternateObservationZones behind the scenes. The MergedMapLayer was being passed to the matchesZones helper function to determine if an observation was either in a forecast zone or an observation zone. Since this PR has been out, matchesZones no longer accepts the full MapLayer objects. Instead, it takes the array of MapFeatures. I decided it would be better to use the ObservationZoneFeatures array directly.

Pass zone name into ObservationDetailView

To avoid calling matchesZone twice (once in the obs list view and once in the obs detail view), I decided to pass the zone name directly into the the ObservationDetailView. This removed the need to call useAllMapLayers, useAlternateObservationZones, and useAvalancheCenterMetadata from the detail view

Changes to ObservationDetailModalView

Because this view is shown from a deep link, there's no guarantee that the centerId set in the user preferences matches the centerId from the deep linked zone. This means we need to fetch all of the necessary information to show the correct zone name in the modal. To guarantee that we have the correct centerId, I broke the component up into two parts so that we can make sure that everything is correctly fetched before showing content.

Open issues from the original one

It looks like we never closed on if we should show the alternate zones in the ObservationFilterForm. I hooked it up so we can easily add it, if need be, but I'm not doing anything with it.

Once this PR is checked in, I'll close the original one and close the associated issue

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[yuliadub]

claude flagged this, not sure if it

The lockfile pulls in a very unusual dependency graph for what should be a well-known package:
fast-xml-parser@5.10.1
├── @nodable/entities@^3.0.0
├── fast-xml-builder@^1.2.0
│ ├── path-expression-matcher@^1.6.2
│ └── xml-naming@^0.3.0
├── is-unsafe@^2.0.0
├── path-expression-matcher@^1.6.2
├── strnum@^2.4.1 (→ anynum@^1.0.1)
└── xml-naming@^0.3.0

The legitimate fast-xml-parser (NaturalIntelligence) has essentially only strnum as a runtime dep. @nodable/entities, fast-xml-builder, xml-naming, path-expression-matcher, anynum, is-unsafe are unknown/obscure packages. Verify the publisher, the tarball checksum, and the maintainer before merging — this pattern is consistent with a typosquat / hijack.

comparing to what was there before (https://github.com/NWACus/avy/pull/904/changes#diff-51e4f558fae534656963876761c95b83b6ef5da5103c4adef6768219ed76c2deR6131) does look different

[yuliadub]

should be &?

[yuliadub]

we used to do toUpperCase for center id and no we dont, why?

[yuliadub]

looks like this was there before but !capabilities appears twice

[yuliadub]

broken test title

[yuliadub]

I am not sure if I love zoneNameas a required navigation param, but totally open to arguments against me though. Generated the below with Claude since I gotta run to pick up my kid but wanted to capture the concerns. Let me know your thoughts!

Making zoneName a required field on the observation route couples display text to the routing contract

Why it's a problem

TypeScript enforces the new param at call sites inside the repo (list view, dev menu, etc.), but React Navigation doesn't enforce param shapes at runtime. Any entry point that isn't type-checked can hand zoneName === undefined to the screen and render "undefined Observation" in the header, because ObservationDetailView now unconditionally does:

React.useEffect(() => {
  navigation.setOptions({title: `${zoneName} Observation`});
}, [navigation, zoneName]);

Entry points that bypass TS today or in the future:

• Deep links (linking.config templates almost certainly won't carry a display string).
• Push notifications.
• Persisted navigation state restored from an older build.
• New call sites added later that forget to pass it.

Even setting the regression aside, treating a display label as routing data means it ends up in persisted nav state and analytics/breadcrumb params, and it invites someone to later promote it to a URL path segment "because it's already a param" — at which point zone names with &, spaces, etc. become an encoding problem too.

Suggested fix — hint + fallback

Keep the perf win (skip the extra fetches when the caller already knows the zone), but don't rely on it for correctness:

// routes.ts
observation: {
  id: string;
  zoneName?: string; // display hint from the list view; not required
};

// ObservationDetailView.tsx
const {id, zoneName} = route.params;
const observation = useNACObservation(id).data;
const derivedZone = useObservationZoneName(observation); // undefined until queries resolve
const effectiveZone = zoneName ?? derivedZone;

React.useEffect(() => {
  navigation.setOptions({title: effectiveZone ? `${effectiveZone} Observation` : 'Observation'});
}, [navigation, effectiveZone]);

useObservationZoneName would wrap the same logic ObservationDetailModalContent already uses (useAvalancheCenterMetadata → useAlternateObservationZones → useAllMapLayers → matchesZone). Since the list view and prefetchAllActiveForecasts typically populate those caches, navigations from the list stay cheap; deep links pay one round-trip but render a correct title instead of undefined Observation.

Bonus: extracting useObservationZoneName lets both the modal and the detail view share one implementation instead of duplicating the fetch-and-match dance.

[kevinherdez]

I had a longer, more detailed response but Github erased it 😢

I made it required mainly for the perf cost since the ObservationListView already determines the zone name for each observation, but also so to clearly point out that the zone name really is a concept that's external to the observation itself. Doing this also fixed a bug where the navigation title would just show "Observation" for a split second before showing "<Zone name> Observation"

To Claude's points, for deep linking we use the ObservationDetailViewModal which needs to determine the zone name itself since there's no way to pass that information to the modal. I imagine this flow would be the same for push notifications, and future changes here to use ObservationDetailView would be a bigger change in the overall deep linking experience.

I like the useObservationZoneName hook as it neatly wraps the hooks/logic required to figure out the zone name, but I don't really think that it should be used in ObservationDetailView because we would still be duplicating calls to hooks and functions that's already being done in ObservationListView.

To fallback better, we could make the type for zoneName be string | undefined. We could also change the name to something clearer like zoneDisplayName or zoneNameForTitle

I'm not entirely sure what Claude is getting at here with this comment. It seems like something that would be an issue only on web.

Even setting the regression aside, treating a display label as routing data means it ends up in persisted nav state and analytics/breadcrumb params, and it invites someone to later promote it to a URL path segment "because it's already a param" — at which point zone names with &, spaces, etc. become an encoding problem too.

There could be better naming with the routes too. observation and observationModal are not super clear in their intended destination.