explorer: facet selection drives the map via point-mode promotion (#267)#270
Open
rdhyee wants to merge 3 commits into
Open
explorer: facet selection drives the map via point-mode promotion (#267)#270rdhyee wants to merge 3 commits into
rdhyee wants to merge 3 commits into
Conversation
…amplesorg#267) Andrea (isamplesorg#267) reported that clicking material/sampled-feature/object-type facets didn't update the map. Root cause: in cluster mode the map is drawn from pre-aggregated H3 summaries that only carry dominant_source, so they structurally cannot honor those facets. Counts already refresh; only the map layer was unresponsive. Mirror the committed-search path (applySearchFilterChange / C3): when any such facet is active, force point mode so the map shows the actual filtered dots (which already apply facetFilterSQL()). When facets clear, revert to the altitude-appropriate mode unless a text search is still latching point. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…links (isamplesorg#267) Codex review of the initial isamplesorg#267 fix found two gaps: 1) Latch bug: handleFacetFilterChange forced point mode on the click, but camera.changed/moveEnd only latched point mode for searchIsActive(), so panning/zooming at altitude reverted to unfiltered clusters while a facet stayed checked. Extend every point-mode latch (targetMode, the enter-point-directly branch, the moveEnd exit guard, and the hash-restore wantsPoint) to also honor hasFacetFilters(). 2) Deep-link miss: a shared ?material=/?context=/?object_type= URL booted into cluster mode. Force point mode at boot when the URL carries facet params (read from the URL, so it's independent of facet-checkbox hydration timing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nge (isamplesorg#267) Codex round-2 found the symmetric gap to the round-1 fix: clearing the search while a facet is still checked reverted to unfiltered clusters at altitude, because applySearchFilterChange's clear branch only kept point mode for searchIsActive(). Use forcePoint = searchIsActive() || hasFacetFilters() so neither filter being cleared drops the other's point-mode view. Mirrors the !searchIsActive() guard already in handleFacetFilterChange. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #267.
Root cause: clicking material / sampled-feature / object-type facets didn't update the map in cluster mode, because cluster dots come from pre-aggregated H3 summaries that carry only
dominant_source— they structurally can't honor those facets. (Counts already refresh; only the map layer was unresponsive. This is not the #253 count race, which is fixed.)Fix: mirror the committed-search path (
applySearchFilterChange/ C3) inhandleFacetFilterChange: when any such facet is active, force point mode so the map shows the actual filtered dots (which already applyfacetFilterSQL()). When facets clear, revert to the altitude-appropriate mode unless a text search is still latching point mode.Tradeoff to review: a broad facet (e.g. a million-sample material) will now drop to point mode and hit
POINT_BUDGET, showing capped dots with the existing 'showing N — zoom in' note — exactly as committed search already behaves. Worth a manual check across world + area scope.— 🤖 rbotyee (RY's bot). RY skimmed; the bot did the work. Ping @rdhyee.