Add geospatial filter modal to query builder - #88
Conversation
The theme mapped every colour token except --color-background, so the bg-background class of Dialog.Content produced nothing and the panel stayed transparent. Both containers also had no height cap, so a tall modal grew past the window and took its footer with it. They now stop at 90% of the window in both directions, and the content scrolls. Modal registered its Escape listener once, and only when canCloseModal was true at that moment. A later change never reached it.
A query could only get an area filter after a run on the map viewer. The modal draws one in the builder, before the first run. The area now carries the two columns it tests. The compile resolved that pair by name detection alone, so a table with x/y columns could not take an area. The user picks the pair in the modal, and detection stays the fallback.
There was a problem hiding this comment.
🟡 Changes recommended
It introduces a validated-column flow but still allows an invalid same-column pair in selection resolution and has formatting issues that can break tooling.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This pull request adds a geospatial filter modal to the query builder so users can draw an area filter before the first map run. It also updates spatial-filter compilation and hydration so the filter uses the user-selected coordinate column pair, with name-based detection as a fallback.
Changes:
- Add a builder modal that renders a basemap and draw tools to create or edit a spatial filter.
- Persist the chosen coordinate column pair on the spatial selection and resolve it via
selectionColumns()during compile and map edits. - Improve dialog/modal sizing and scrolling behavior to keep modal content usable on smaller viewports.
File summaries
| File | Description |
|---|---|
| src/tailwind.css | Define --color-background so bg-background works correctly in dialogs. |
| src/routes/visualisations/map-viewer/+page.svelte | Preserve and restamp selected coordinate columns when applying a redrawn area. |
| src/lib/query/seed-hydration.ts | Match derived bbox filters using the exact stored coordinate column names. |
| src/lib/query/draft.ts | Compile spatial filters using selectionColumns() instead of coordinate detection alone. |
| src/lib/geo/spatial-selection.ts | Store selected coordinate columns on selections and add helpers to resolve and validate them. |
| src/lib/components/visualisation/SpatialFilterMap.svelte | New lightweight MapLibre basemap component for drawing areas without data layers. |
| src/lib/components/visualisation/MapDrawTools.svelte | Add optional “Apply” button rendering and harden cleanup on destroy. |
| src/lib/components/ui/dialog/dialog-content.svelte | Constrain dialog size to viewport and enable scrolling for overflow content. |
| src/lib/components/query-builder/QueryWorkspace.svelte.ts | Update spatial filter patching for blocks without drafts, using stored column pairs. |
| src/lib/components/query-builder/QueryBuilderParameterBlock.svelte | Add geospatial filter UI entry points and wire the new modal into the builder. |
| src/lib/components/query-builder/GeospatialFilterModal.svelte | New modal to select coordinate columns and draw/apply/remove an area filter. |
| src/lib/components/modals/Modal.svelte | Make Escape-close respect live canCloseModal and add scrolling layout constraints. |
| AGENTS.md | Document the new “selection carries coordinate columns” rule and remove an outdated note. |
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const latitude = selection?.latitudeColumn; | ||
| const longitude = selection?.longitudeColumn; | ||
|
|
||
| if ( | ||
| latitude && | ||
| longitude && | ||
| availableNames.includes(latitude) && | ||
| availableNames.includes(longitude) | ||
| ) { | ||
| return { latitude, longitude }; | ||
| } |
| <span class="search-columns-item-name">{field.name}</span> | ||
| <span class="search-columns-item-details">{Utils.dataTypeToString(field.type)}</span> | ||
| <span class="search-columns-item-details">{Utils.dataTypeToString(field.type)}</span | ||
| > |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The functional changes are cohesive and low-risk, with only minor user-facing text and spelling nits noted.
Review details
Suppressed comments (4)
Previously missed (2) — in code that hasn't changed since the last review.
src/lib/components/query-builder/GeospatialFilterModal.svelte:100
- Update this message. The check uses the count of number columns, not the column names.
This issue also appears on line 149 of the same file.
src/lib/components/query-builder/QueryBuilderParameterBlock.svelte:3
- Fix spelling in the file header comment. The filename is misspelled as "QueryBuilderParametrBlock.svelte".
This issue also appears on line 131 of the same file.
src/lib/components/query-builder/QueryBuilderParameterBlock.svelte:134
- Update this hint text. The code checks for two number columns, not only columns named latitude and longitude.
const areaBlockReason = $derived.by(() => {
if (numericColumnCount < 2) {
return 'Select a latitude and a longitude column first.';
}
src/lib/components/query-builder/GeospatialFilterModal.svelte:150
- Update this warning text. It mentions latitude and longitude names, but the constraint is only two number columns.
This query selects less than two number columns. Add a latitude and a longitude column
first.
- Files reviewed: 13/13 changed files
- Comments generated: 0 new
- Review effort level: Lite
A query could only get an area filter after a run on the map viewer.
The modal draws one in the builder, before the first run.
The area now carries the two columns it tests. The compile resolved
that pair by name detection alone, so a table with x/y columns could
not take an area. The user picks the pair in the modal, and detection
stays the fallback.
Addresses issue #82