add field config on location by lucaslyl · Pull Request #3978 · cardstack/boxel

lucaslyl · 2026-02-11T10:12:17Z

linear: https://linear.app/cardstack/issue/ECO-436/location-field

Key changes:

GeoPointField: Extract edit UI into variant-based components with configuration support for standard and map-picker variants, plus optional addons (current location, quick locations)
GeoSearchPointField: Extract edit UI into composable components with configuration for search results display and recent searches
Field specs: Add playground specs (GeoPointFieldSpec, GeoSearchPointFieldSpec) with example configurations
Route: Update CoordinateField to use renamed GeoSearchPointEdit export

Demo

Geo Point with Field Config

Screen.Recording.2026-02-11.at.6.04.17.PM.mov

Geo Search Point with Field Config

Screen.Recording.2026-02-11.at.6.05.05.PM.mov

github-actions · 2026-02-11T10:32:21Z

Host Test Results

1 files ±0 1 suites ±0 1h 39m 9s ⏱️ +34s
1 837 tests ±0 1 824 ✅ ±0 13 💤 ±0 0 ❌ ±0
1 852 runs ±0 1 839 ✅ ±0 13 💤 ±0 0 ❌ ±0

Results for commit fc6efa9. ± Comparison against base commit c9b3642.

Copilot

Pull request overview

This PR refactors the GeoPointField and GeoSearchPointField to extract their edit UI into composable, configuration-driven components. The refactoring enables field-level configuration for various display modes and interactive features like current location detection, quick locations, search results, and recent searches.

Changes:

Extracted GeoPointField and GeoSearchPointField edit UIs into modular, reusable components with configuration support
Added field configuration types to enable variant-based rendering (standard vs map-picker for GeoPoint) and optional addons
Created playground field specs (GeoPointFieldSpec, GeoSearchPointFieldSpec) with example configurations and JSON data
Updated CoordinateField in route.gts to use the renamed GeoSearchPointEdit export
Modified map-render component to use aspect-ratio instead of min-height and improved popup content fallback logic

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
packages/catalog-realm/route/route.gts	Renamed import from GeoSearchPointEditTemplate to GeoSearchPointEdit
packages/catalog-realm/fields/geo-search-point.gts	Refactored into configuration-based dispatcher templates with type-safe options
packages/catalog-realm/fields/geo-search-point/util/index.gts	Extracted shared utilities for geocoding and search
packages/catalog-realm/fields/geo-search-point/components/*.gts	Created modular components for address input, search results, and recent searches
packages/catalog-realm/fields/geo-point.gts	Refactored with variant-based configuration (standard vs map-picker)
packages/catalog-realm/fields/geo-point/util/index.gts	Extracted shared coordinate validation and formatting utilities
packages/catalog-realm/fields/geo-point/components/*.gts	Created modular components for coordinate input, map picker, and location addons
packages/catalog-realm/field-spec/geo-point-spec.gts	Added playground spec with 8 configuration examples
packages/catalog-realm/field-spec/geo-search-point-spec.gts	Added playground spec with 4 configuration examples
packages/catalog-realm/field-spec/GeoPointFieldSpec/*.json	Configuration data for GeoPoint field examples
packages/catalog-realm/field-spec/GeoSearchPointFieldSpec/*.json	Configuration data for GeoSearchPoint field examples
packages/catalog-realm/components/map-render.gts	Changed min-height to aspect-ratio and improved popup fallback logic

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-13T20:12:28Z

packages/catalog-realm/fields/geo-point/components/geo-point-coordinate-input.gts

+  get lonState(): 'valid' | 'invalid' | 'none' {
+    const lon = this.args.model.lon;
+    if (lon == null) return 'none';
+    return lon >= -180 && lon <= 180 ? 'valid' : 'invalid';


The validation states don't check for NaN values. If the model has a NaN value (which could happen from the setLon action with invalid input), the validation will incorrectly show it as 'valid' since NaN >= -180 and NaN <= 180 both evaluate to false, making the ternary return 'valid'. Add a NaN check: if (lon == null || isNaN(lon)) return 'none'; or treat NaN as 'invalid'.

Copilot · 2026-02-13T20:12:28Z

packages/catalog-realm/fields/geo-search-point/components/geo-search-point-edit-field.gts

+  private debouncedSearch = debounce((query: string) => {
+    this.performSearchTask.perform(query);
+  }, 1000);
+


The debounced search function is not cancelled when the component is destroyed. If the component is destroyed while a debounced search is pending, the callback will still execute and may attempt to access this.performSearchTask on a destroyed component. Consider adding a willDestroy lifecycle hook to cancel the pending debounce: willDestroy() { this.debouncedSearch.cancel(); }.

Suggested change

willDestroy(): void {

this.debouncedSearch.cancel();

super.willDestroy?.();

}

Copilot · 2026-02-13T20:12:28Z

packages/catalog-realm/fields/geo-point/components/geo-point-coordinate-input.gts

+  get latState(): 'valid' | 'invalid' | 'none' {
+    const lat = this.args.model.lat;
+    if (lat == null) return 'none';
+    return lat >= -90 && lat <= 90 ? 'valid' : 'invalid';


The validation state doesn't check for NaN values. If the model has a NaN value (which could happen from the setLat action with invalid input), the comparison NaN >= -90 && NaN <= 90 evaluates to false, making the ternary incorrectly return 'valid'. Add a NaN check: if (lat == null || isNaN(lat)) return 'none'; or treat NaN as 'invalid'.

Copilot · 2026-02-13T20:12:29Z

packages/catalog-realm/fields/geo-point/components/geo-point-coordinate-input.gts

+      this.args.model.lat = val === '' ? null : Number(val);
+    }
+  }
+
+  @action
+  setLon(val: string) {
+    if (this.args.model) {
+      this.args.model.lon = val === '' ? null : Number(val);


The setLon action converts input to Number without validating for NaN. If a user enters invalid text like 'xyz', Number('xyz') returns NaN which will pass the null check but fail validation silently. Consider adding NaN validation similar to the suggestion for setLat.

Suggested change

this.args.model.lat = val === '' ? null : Number(val);

}

}

@action

setLon(val: string) {

if (this.args.model) {

this.args.model.lon = val === '' ? null : Number(val);

if (val === '') {

this.args.model.lat = null;

} else {

const num = Number(val);

this.args.model.lat = Number.isNaN(num) ? null : num;

}

}

}

@action

setLon(val: string) {

if (this.args.model) {

if (val === '') {

this.args.model.lon = null;

} else {

const num = Number(val);

this.args.model.lon = Number.isNaN(num) ? null : num;

}

Copilot · 2026-02-13T20:12:29Z

packages/catalog-realm/fields/geo-point/util/index.gts

+    !isNaN(lat) &&
+    !isNaN(lon)


Inconsistent use of isNaN vs Number.isNaN for coordinate validation. The geo-search-point util uses Number.isNaN (lines 37-38) while this file uses the global isNaN function (lines 15-16 and 50-51). Number.isNaN is more strict and doesn't coerce values to numbers, making it the preferred choice for type-safe validation. Consider using Number.isNaN consistently across both files.

Copilot · 2026-02-13T20:12:30Z

packages/catalog-realm/components/map-render.gts

        width: 100%;
        height: 100%;
-        min-height: 300px;
+        aspect-ratio: 16/9;


Setting both height: 100% and aspect-ratio: 16/9 on the same element can lead to conflicting layout behavior. If the parent container has a defined height, height: 100% will take precedence and the aspect-ratio will be ignored. If the parent doesn't have a defined height, the aspect-ratio will control the height based on width. This could lead to inconsistent sizing behavior depending on the parent container. Consider removing one of these properties or documenting the intended layout behavior more clearly.

Suggested change

aspect-ratio: 16/9;

Copilot · 2026-02-13T20:12:30Z

packages/catalog-realm/fields/geo-point.gts

+    const opts = this.config.options;
+    if (!opts) return undefined;
+    return {
+      mapHeight: (opts as GeoPointMapOptions).mapHeight,
+      tileserverUrl: (opts as GeoPointMapOptions).tileserverUrl,


The type cast (opts as GeoPointMapOptions) is unsafe here because opts could be of type GeoPointBaseOptions when variant is 'standard'. This would access potentially undefined properties. Consider checking the variant or using optional chaining: (opts as GeoPointBaseOptions & GeoPointMapOptions)?.mapHeight or narrowing the type based on this.config.variant.

Suggested change

const opts = this.config.options;

if (!opts) return undefined;

return {

mapHeight: (opts as GeoPointMapOptions).mapHeight,

tileserverUrl: (opts as GeoPointMapOptions).tileserverUrl,

const { options, variant } = this.config;

if (variant !== 'map-picker' || !options) {

return undefined;

}

const mapOpts = options as GeoPointBaseOptions & GeoPointMapOptions;

return {

mapHeight: mapOpts.mapHeight,

tileserverUrl: mapOpts.tileserverUrl,

Copilot · 2026-02-13T20:12:30Z

packages/catalog-realm/fields/geo-point/components/geo-point-coordinate-input.gts

+      this.args.model.lat = val === '' ? null : Number(val);
+    }
+  }
+
+  @action
+  setLon(val: string) {
+    if (this.args.model) {
+      this.args.model.lon = val === '' ? null : Number(val);


The setLat and setLon actions convert input to Number without validating for NaN. If a user enters invalid text like 'abc', Number('abc') returns NaN which will pass the null check but fail validation silently. The validation states only check range, not NaN. Consider adding NaN validation: const num = Number(val); this.args.model.lat = val === '' ? null : (isNaN(num) ? null : num); or show an error state for NaN values.

Suggested change

this.args.model.lat = val === '' ? null : Number(val);

}

}

@action

setLon(val: string) {

if (this.args.model) {

this.args.model.lon = val === '' ? null : Number(val);

if (val === '') {

this.args.model.lat = null;

} else {

const num = Number(val);

this.args.model.lat = Number.isNaN(num) ? null : num;

}

}

}

@action

setLon(val: string) {

if (this.args.model) {

if (val === '') {

this.args.model.lon = null;

} else {

const num = Number(val);

this.args.model.lon = Number.isNaN(num) ? null : num;

}

Copilot · 2026-02-13T20:12:30Z

packages/catalog-realm/components/map-render.gts

+      const popupContent =
+        c.address?.trim() || `${c.lat.toFixed(6)}, ${c.lng.toFixed(6)}`;


The fallback for popup content uses c.address?.trim() which returns an empty string for whitespace-only addresses. Empty strings are falsy, so the logic correctly falls back to coordinates. However, for better clarity and to handle edge cases explicitly, consider using c.address?.trim() || undefined to make the intent clearer.

Suggested change

const popupContent =

c.address?.trim() || `${c.lat.toFixed(6)}, ${c.lng.toFixed(6)}`;

const trimmedAddress = c.address?.trim() || undefined;

const popupContent =

trimmedAddress ?? `${c.lat.toFixed(6)}, ${c.lng.toFixed(6)}`;

Copilot · 2026-02-13T20:12:31Z

packages/catalog-realm/fields/geo-point.gts

+  <template>
+    <GeoPointMapPicker
+      @model={{@model}}
+      @options={{this.mapOptions}}
+      @canEdit={{false}}
+    />
+  </template>


The embedded template always displays GeoPointMapPicker regardless of the configured variant. This means that even when variant is 'standard', the embedded view will show a map instead of potentially showing coordinates in a simpler format. Consider checking the variant configuration and rendering different content based on whether it's 'standard' or 'map-picker', similar to how the edit template handles different variants.

add field config on location

fc6efa9

lucaslyl self-assigned this Feb 11, 2026

lucaslyl requested a review from a team February 11, 2026 10:12

habdelra requested a review from Copilot February 13, 2026 20:02

Copilot started reviewing on behalf of habdelra February 13, 2026 20:03 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add field config on location#3978

add field config on location#3978
lucaslyl wants to merge 1 commit intomainfrom
ECO-436-location-field

lucaslyl commented Feb 11, 2026

Uh oh!

github-actions bot commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

+  willDestroy(): void {
+    this.debouncedSearch.cancel();
+    super.willDestroy?.();
+  }

-      this.args.model.lat = val === '' ? null : Number(val);
-    }
-  }
-  @action
-  setLon(val: string) {
-    if (this.args.model) {
-      this.args.model.lon = val === '' ? null : Number(val);
+      if (val === '') {
+        this.args.model.lat = null;
+      } else {
+        const num = Number(val);
+        this.args.model.lat = Number.isNaN(num) ? null : num;
+      }
+    }
+  }
+  @action
+  setLon(val: string) {
+    if (this.args.model) {
+      if (val === '') {
+        this.args.model.lon = null;
+      } else {
+        const num = Number(val);
+        this.args.model.lon = Number.isNaN(num) ? null : num;
+      }

		const popupContent =
		c.address?.trim() \|\| `${c.lat.toFixed(6)}, ${c.lng.toFixed(6)}`;

Conversation

lucaslyl commented Feb 11, 2026

Key changes:

Demo

Uh oh!

github-actions bot commented Feb 11, 2026

Host Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant