Add coordinate modal UX by slifty · Pull Request #1161 · PermanentOrg/web-app

slifty · 2026-09-04T15:14:19Z

This PR adds a new modal for picking locations via map pins / coordinates. Similar to the uncertain location UX we recently added this is JUST about the user experience and the resulting selections are not saved. It's also behind the feature flag since it is ultimately part of the new location feature.

This PR also includes some componentization improvements which impact the locations modal we just added.

Resolves #1159

codecov · 2026-09-04T15:16:17Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.39%. Comparing base (42ff185) to head (6e1c973).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1161      +/-   ##
==========================================
+ Coverage   52.59%   53.39%   +0.79%     
==========================================
  Files         357      361       +4     
  Lines       12203    12397     +194     
  Branches     2208     2254      +46     
==========================================
+ Hits         6418     6619     +201     
+ Misses       5556     5554       -2     
+ Partials      229      224       -5

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

🟡 Changes recommended

A few type contracts in the new utilities/component don’t match actual runtime/usage (optional/null inputs and optional view child), which should be corrected for clarity and future safety.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR introduces a new coordinate-picking modal (map pin + free-form coordinate text) and wires it into the file-browser sidebar UX behind the existing uncertain-locations feature flag, without persisting changes.

Changes:

Added coordinate parsing/formatting utilities (with unit tests) for DMS + decimal-degree inputs.
Updated the sidebar location section to split “coordinates” vs “place/address” actions when uncertain-locations is enabled, including map-preview click routing.
Added a new standalone CoordinatePickerComponent modal (with tests) and exposed it via EditService.openCoordinateDialog.

File summaries

File	Description
src/app/shared/utilities/coordinates.ts	Adds coordinate format/parse helpers and `LocnVOData` → coordinates extraction.
src/app/shared/utilities/coordinates.spec.ts	Unit tests covering formatting and parsing behavior/edge cases.
src/app/file-browser/components/sidebar/sidebar.component.ts	Adds coordinate/address display logic and map preview routing behind feature flag.
src/app/file-browser/components/sidebar/sidebar.component.spec.ts	Extends sidebar tests for the new split location UX and dialog routing.
src/app/file-browser/components/sidebar/sidebar.component.scss	Adds styling for coordinate display and location buttons.
src/app/file-browser/components/sidebar/sidebar.component.html	Updates Location section UI to show separate coordinate/address actions when flagged.
src/app/file-browser/components/coordinate-picker/coordinate-picker.component.ts	Implements the new coordinate picker modal with map + text input synchronization.
src/app/file-browser/components/coordinate-picker/coordinate-picker.component.spec.ts	Tests modal initialization, map clicks, typing behavior, and save/cancel flows.
src/app/file-browser/components/coordinate-picker/coordinate-picker.component.scss	Styles the new modal layout and invalid-state presentation.
src/app/file-browser/components/coordinate-picker/coordinate-picker.component.html	Modal template including map, marker, coordinate input, and footer actions.
src/app/core/services/edit/edit.service.ts	Adds `openCoordinateDialog` to open the new coordinate picker modal.

Review details

Suppressed comments (1)

src/app/file-browser/components/coordinate-picker/coordinate-picker.component.ts:94

@ViewChild(GoogleMap) is undefined until after view init and can also be absent in tests; the code already treats it as optional via this.map?.googleMap. Marking the field optional aligns the type with actual usage.

	@ViewChild(GoogleMap) map: GoogleMap;

Files reviewed: 11/11 changed files
Comments generated: 2
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+	public item: ItemVO;
+	public profileItem: ProfileItemVOData;


+const isInRange = ({ latitude, longitude }: Coordinates): boolean =>
+	Math.abs(latitude) <= MAX_LATITUDE && Math.abs(longitude) <= MAX_LONGITUDE;
+
+export const parseCoordinates = (text: string): Coordinates | null => {


Copilot

🟡 Changes recommended

There are a few concrete correctness/contract issues (notably parseCoordinates typing vs usage and invalid styling being overridden on focus) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

src/app/shared/utilities/coordinates.ts:109

parseCoordinates is typed to accept only string, but it is implemented defensively (text ?? '') and the spec calls it with null. Updating the parameter type will make the contract consistent and avoid type errors in stricter TS settings.

export const parseCoordinates = (text: string): Coordinates | null => {

src/app/shared/components/icon-text-input/icon-text-input.component.scss:24

When the field is both invalid and focused, the :focus-within rule sets the border back to blue, which hides the invalid state during editing. Add an override so invalid styling wins while focused.

	&:focus-within {
		@include input-focus-state;
		border-color: $PR-blue-100;

Files reviewed: 27/27 changed files
Comments generated: 1
Review effort level: Lite

+	 * leaves alone. Only reachable behind `uncertain-locations`, which is what
+	 * splits a location into an address and a pair in the first place.
+	 */
+	async openCoordinateDialog(item: ItemVO) {


Prior to this change the icon was passed to the component, but by having the component render the icon with a gradient the icons get distinct IDs. Issue #1159 Implement UX for location coordinate entry Claude-Session: https://claude.ai/code/session_01HM1anu1T97zKvtb1TKTcfc

This frame is going to be used by other location picker modals. Issue #1159 Implement UX for location coordinate entry Claude-Session: https://claude.ai/code/session_01HM1anu1T97zKvtb1TKTcfc

This utility is going to be used for the upcoming location picker UX. There are a few ways to note GPS coordinates and we're standardizing on DMS for rendering but will accept degree format for now. If the string doesn't parse we return null because that's actually something a caller can handler and understand. Issue #1159 Implement UX for location coordinate entry Claude-Session: https://claude.ai/code/session_01HM1anu1T97zKvtb1TKTcfc

The coordinate map input only has a single intended use right now but I think there is benefit in having well-scoped components from a testability and consolidation of complexity perspective. Issue #1159 Implement UX for location coordinate entry Claude-Session: https://claude.ai/code/session_01HM1anu1T97zKvtb1TKTcfc

This modal will allow users to pick GPS by dropping a pin on a map *or* by pasting in text. Unparsable text will result in an inability to click save, since that's something the user should resolve and simply falling back might be confusing. Issue #1159 Implement UX for location coordinate entry Claude-Session: https://claude.ai/code/session_01HM1anu1T97zKvtb1TKTcfc

The final UX for the sidebar will look quite different from this, but we're making this change to allow interaction with both types of location modal as we develop. Issue #1159 Implement UX for location coordinate entry Claude-Session: https://claude.ai/code/session_01HM1anu1T97zKvtb1TKTcfc

aasandei-vsp

This PR is quite big and there are certain commits that could have been their own PRs or be grouped in smaller PRs.

aasandei-vsp · 2026-09-07T11:59:56Z

+
+@Component({
+	selector: 'pr-dialog-frame',
+	standalone: true,


That is great! Defining it as standalone moves us away from the module chaos and it's the way to go!

aasandei-vsp · 2026-09-07T12:07:14Z

@@ -0,0 +1,159 @@
+import { LocnVOData } from '@models';


I was wondering if we could swap this for a library, so I checked. Nope :))) There is no worthy candidate, from what I have found.

aasandei-vsp · 2026-09-07T12:12:11Z

+		<path fill="#131B4A" d="M14 0C6.268 0 0 6.268 0 14c0 9.5 12.2 20.86 12.72 21.34a1.87 1.87 0 0 0 2.56 0C15.8 34.86 28 23.5 28 14 28 6.268 21.732 0 14 0Z"/>
+		<circle cx="14" cy="14" r="5" fill="#FFFFFF"/>
+	</svg>`,
+)}`;


How did we established the values for these const? They look more like configuration than simply parts of a component.

aasandei-vsp · 2026-09-07T12:14:02Z

+} from '@shared/utilities/coordinates';
+import { faLocationCrosshairs } from '@fortawesome/pro-regular-svg-icons';
+
+const CONTINENTAL_US_CENTER: google.maps.LatLngLiteral = {


Is there any angular google map type that we could use here? Just for cosmetic purposes....

aasandei-vsp · 2026-09-07T12:14:07Z

+
+const toLatLngLiteral = (
+	coordinates: Coordinates,
+): google.maps.LatLngLiteral => ({


Is there any angular google map type that we could use here? Just for cosmetic purposes....

aasandei-vsp · 2026-09-07T12:14:23Z

+		icon: { url: BRAND_PIN_ICON_URL },
+	};
+
+	mapOptions: google.maps.MapOptions = {


Is there any angular google map type that we could use here? Just for cosmetic purposes....

aasandei-vsp · 2026-09-07T12:20:48Z

+	styleUrls: ['./coordinate-picker.component.scss'],
+})
+export class CoordinatePickerComponent implements OnInit {
+	public item?: ItemVO;


Do we need the whole item? I see we're using this.item?.LocnVO. Is there something I'm missing?

aasandei-vsp · 2026-09-07T12:24:10Z

+	 * The coordinate half of a location, which the uncertain address modal
+	 * leaves alone. Only reachable behind `uncertain-locations`, which is what
+	 * splits a location into an address and a pair in the first place.
+	 */


I guess we'll remove the comment once we get rid of the flag?

aasandei-vsp · 2026-09-07T12:30:11Z

+			data: { item },
+			panelClass: 'dialog',
+			height: 'auto',
+			width: '640px',


Not a big fan of this hardcoded width. It's also duplicated on openLocationDialog. The edit service, that is heavy on business logic, should not be responsible for the width of a dialog. The component should actually own it.

aasandei-vsp · 2026-09-07T12:33:34Z

-					@if (location.name) {
-						<strong>{{ location.name }}</strong>
-						<br />
+			@if (showUncertainLocations) {


this could also be its own component. Extracting it would make the sidebar much cleaner.

Copilot AI lite review requested due to automatic review settings September 4, 2026 15:14

Copilot started reviewing on behalf of slifty September 4, 2026 15:14 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

slifty force-pushed the 1159-coordinate-modal branch from 760922b to 46f768b Compare September 4, 2026 16:40

slifty requested a lite review from Copilot September 4, 2026 16:52

Copilot started reviewing on behalf of slifty September 4, 2026 16:53 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread src/app/core/services/edit/edit.service.ts Outdated

* leaves alone. Only reachable behind `uncertain-locations`, which is what

* splits a location into an address and a pair in the first place.

*/

async openCoordinateDialog(item: ItemVO) {

slifty force-pushed the 1159-coordinate-modal branch 2 times, most recently from 9dc5861 to 5222145 Compare September 4, 2026 18:30

slifty added 6 commits September 4, 2026 17:33

Extract a reusable dialog frame

847bb76

This frame is going to be used by other location picker modals. Issue #1159 Implement UX for location coordinate entry Claude-Session: https://claude.ai/code/session_01HM1anu1T97zKvtb1TKTcfc

slifty force-pushed the 1159-coordinate-modal branch from 5222145 to 6e1c973 Compare September 4, 2026 21:33

aasandei-vsp requested changes Sep 7, 2026

View reviewed changes

Uh oh!

Conversation

slifty commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

aasandei-vsp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

slifty commented Sep 4, 2026 •

edited

Loading

codecov Bot commented Sep 4, 2026 •

edited

Loading