-
Notifications
You must be signed in to change notification settings - Fork 2
Add coordinate modal UX #1161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add coordinate modal UX #1161
Changes from all commits
ef108ea
847bb76
3af3f2b
d02de77
31af94d
6e1c973
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,7 @@ import { PublishComponent } from '@fileBrowser/components/publish/publish.compon | |
| import { EditTagsComponent } from '@fileBrowser/components/edit-tags/edit-tags.component'; | ||
| import { LocationPickerComponent } from '@fileBrowser/components/location-picker/location-picker.component'; | ||
| import { UncertainLocationPickerComponent } from '@fileBrowser/components/uncertain-location-picker/uncertain-location-picker.component'; | ||
| import { CoordinatePickerComponent } from '@fileBrowser/components/coordinate-picker/coordinate-picker.component'; | ||
| import { FeatureFlagService } from '@root/app/feature-flag/services/feature-flag.service'; | ||
| import { SharingDialogComponent } from '@fileBrowser/components/sharing-dialog/sharing-dialog.component'; | ||
| import { FolderPickerService } from '../folder-picker/folder-picker.service'; | ||
|
|
@@ -647,6 +648,20 @@ export class EditService { | |
| }); | ||
| } | ||
|
|
||
| /** | ||
| * 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. | ||
| */ | ||
| openCoordinateDialog(item: ItemVO): void { | ||
| this.dialog.open(CoordinatePickerComponent, { | ||
| data: { item }, | ||
| panelClass: 'dialog', | ||
| height: 'auto', | ||
| width: '640px', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a big fan of this hardcoded width. It's also duplicated on |
||
| }); | ||
| } | ||
|
|
||
| public async openFolderPicker( | ||
| items: ItemVO[], | ||
| operation: FolderPickerOperations, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| <pr-dialog-frame | ||
| heading="Choose GPS Coordinates" | ||
| [confirmDisabled]="!isValid()" | ||
| (cancelled)="cancel()" | ||
| (confirmed)="save()" | ||
| > | ||
| <pr-coordinate-map-input | ||
| [coordinates]="coordinates()" | ||
| (coordinatesChange)="onCoordinatesChange($event)" | ||
| (validityChange)="onValidityChange($event)" | ||
| /> | ||
| </pr-dialog-frame> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| :host { | ||
| display: block; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,170 @@ | ||
| import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; | ||
| import { ComponentFixture, TestBed } from '@angular/core/testing'; | ||
| import { DIALOG_DATA, DialogRef } from '@angular/cdk/dialog'; | ||
| import { GoogleMapsModule } from '@angular/google-maps'; | ||
| import { RecordVO } from '@models'; | ||
| import { ProfileItemVOData } from '@models/profile-item-vo'; | ||
| import { CoordinateMapInputComponent } from '@shared/components/coordinate-map-input/coordinate-map-input.component'; | ||
| import { | ||
| CoordinatePickerComponent, | ||
| CoordinatePickerData, | ||
| } from './coordinate-picker.component'; | ||
|
|
||
| const LISBON = { latitude: 38.70786, longitude: -9.400139 }; | ||
|
|
||
| describe('CoordinatePickerComponent', () => { | ||
| let fixture: ComponentFixture<CoordinatePickerComponent>; | ||
| let component: CoordinatePickerComponent; | ||
| let dialogRef: jasmine.SpyObj<DialogRef>; | ||
|
|
||
| const setUp = async (dialogData: CoordinatePickerData): Promise<void> => { | ||
| dialogRef = jasmine.createSpyObj('DialogRef', ['close']); | ||
|
|
||
| await TestBed.configureTestingModule({ | ||
| imports: [CoordinatePickerComponent], | ||
| providers: [ | ||
| { provide: DIALOG_DATA, useValue: dialogData }, | ||
| { provide: DialogRef, useValue: dialogRef }, | ||
| ], | ||
| }) | ||
| .overrideComponent(CoordinateMapInputComponent, { | ||
| remove: { imports: [GoogleMapsModule] }, | ||
| add: { schemas: [CUSTOM_ELEMENTS_SCHEMA] }, | ||
| }) | ||
| .compileComponents(); | ||
|
|
||
| fixture = TestBed.createComponent(CoordinatePickerComponent); | ||
| component = fixture.componentInstance; | ||
| fixture.detectChanges(); | ||
| }; | ||
|
|
||
| const query = <T extends HTMLElement>(selector: string): T => | ||
| fixture.nativeElement.querySelector(selector); | ||
|
|
||
| describe('with an item that has no location', () => { | ||
| const item = new RecordVO({ recordId: 123 }); | ||
|
|
||
| beforeEach(async () => { | ||
| await setUp({ item }); | ||
| }); | ||
|
|
||
| it('should read the item off the dialog data', () => { | ||
| expect(component.item).toBe(item); | ||
| }); | ||
|
|
||
| it('should render a titled dialog', () => { | ||
| expect(query('.pr-dialog-header h2').textContent.trim()).toBe( | ||
| 'Choose GPS Coordinates', | ||
| ); | ||
| }); | ||
|
|
||
| it('should hand the map input nothing to start from', () => { | ||
| expect(component.coordinates()).toBeNull(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('when the item already has coordinates', () => { | ||
| beforeEach(async () => { | ||
| await setUp({ | ||
| item: new RecordVO({ | ||
| recordId: 123, | ||
| LocnVO: { ...LISBON, city: 'Lisbon' }, | ||
| }), | ||
| }); | ||
| }); | ||
|
|
||
| it('should hand the stored pair to the map input', () => { | ||
| expect(component.coordinates()).toEqual(LISBON); | ||
| }); | ||
|
|
||
| it('should keep the address it was given when saving', () => { | ||
| component.save(); | ||
|
|
||
| expect(dialogRef.close).toHaveBeenCalledWith({ | ||
| location: { ...LISBON, city: 'Lisbon' }, | ||
| }); | ||
| }); | ||
|
|
||
| it('should clear the stored pair when the map input reports none', () => { | ||
| component.onCoordinatesChange(null); | ||
| component.save(); | ||
|
|
||
| expect(dialogRef.close).toHaveBeenCalledWith({ | ||
| location: { latitude: null, longitude: null, city: 'Lisbon' }, | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('when the item has an address but no coordinates', () => { | ||
| beforeEach(async () => { | ||
| await setUp({ | ||
| item: new RecordVO({ recordId: 123, LocnVO: { city: 'Lisbon' } }), | ||
| }); | ||
| }); | ||
|
|
||
| it('should still carry the address through a save', () => { | ||
| component.save(); | ||
|
|
||
| expect(dialogRef.close).toHaveBeenCalledWith({ | ||
| location: { city: 'Lisbon', latitude: null, longitude: null }, | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('when a profile item carries the location', () => { | ||
| it('should read the first of its locations', async () => { | ||
| await setUp({ | ||
| profileItem: { LocnVOs: [{ ...LISBON }] } as ProfileItemVOData, | ||
| }); | ||
|
|
||
| expect(component.coordinates()).toEqual(LISBON); | ||
| }); | ||
| }); | ||
|
|
||
| describe('when the map input reports unreadable text', () => { | ||
| beforeEach(async () => { | ||
| await setUp({ | ||
| item: new RecordVO({ recordId: 123, LocnVO: { ...LISBON } }), | ||
| }); | ||
| }); | ||
|
|
||
| it('should disable the confirm button', () => { | ||
| component.onValidityChange(false); | ||
| fixture.detectChanges(); | ||
|
|
||
| expect(query<HTMLButtonElement>('.pr-btn-confirm').disabled).toBeTrue(); | ||
| }); | ||
|
|
||
| it('should refuse to save the pair the field is no longer showing', () => { | ||
| component.onValidityChange(false); | ||
| component.save(); | ||
|
|
||
| expect(dialogRef.close).not.toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('leaving the dialog', () => { | ||
| beforeEach(async () => { | ||
| await setUp({ item: new RecordVO({ recordId: 123 }) }); | ||
| }); | ||
|
|
||
| it('should close with nothing when cancelled', () => { | ||
| query<HTMLButtonElement>('.pr-btn-cancel').click(); | ||
|
|
||
| expect(dialogRef.close).toHaveBeenCalledWith(); | ||
| }); | ||
|
|
||
| it('should close with nothing when dismissed from the header', () => { | ||
| query<HTMLButtonElement>('.pr-close-button').click(); | ||
|
|
||
| expect(dialogRef.close).toHaveBeenCalledWith(); | ||
| }); | ||
|
|
||
| it('should close with the pair when saved', () => { | ||
| component.onCoordinatesChange(LISBON); | ||
| query<HTMLButtonElement>('.pr-btn-confirm').click(); | ||
|
|
||
| expect(dialogRef.close).toHaveBeenCalledWith({ location: LISBON }); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| import { Component, Inject, OnInit, Optional, signal } from '@angular/core'; | ||
| import { CommonModule } from '@angular/common'; | ||
| import { DIALOG_DATA, DialogRef } from '@angular/cdk/dialog'; | ||
| import { ItemVO, LocnVOData } from '@models'; | ||
| import { ProfileItemVOData } from '@models/profile-item-vo'; | ||
| import { CoordinateMapInputComponent } from '@shared/components/coordinate-map-input/coordinate-map-input.component'; | ||
| import { DialogFrameComponent } from '@shared/components/dialog-frame/dialog-frame.component'; | ||
| import { | ||
| Coordinates, | ||
| coordinatesFromLocation, | ||
| } from '@shared/utilities/coordinates'; | ||
|
|
||
| export interface CoordinatePickerData { | ||
| item?: ItemVO; | ||
| profileItem?: ProfileItemVOData; | ||
| } | ||
|
|
||
| export interface CoordinatePickerResult { | ||
| location: LocnVOData; | ||
| } | ||
|
|
||
| @Component({ | ||
| selector: 'pr-coordinate-picker', | ||
| standalone: true, | ||
| imports: [CommonModule, CoordinateMapInputComponent, DialogFrameComponent], | ||
| templateUrl: './coordinate-picker.component.html', | ||
| styleUrls: ['./coordinate-picker.component.scss'], | ||
| }) | ||
| export class CoordinatePickerComponent implements OnInit { | ||
| public item?: ItemVO; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need the whole item? I see we're using |
||
| public profileItem?: ProfileItemVOData; | ||
|
|
||
| coordinates = signal<Coordinates | null>(null); | ||
| isValid = signal(true); | ||
|
|
||
| private locationBeingEdited: LocnVOData = {}; | ||
|
|
||
| constructor( | ||
| @Optional() | ||
| @Inject(DIALOG_DATA) | ||
| public dialogData?: CoordinatePickerData, | ||
| @Optional() private dialogRef?: DialogRef<CoordinatePickerResult>, | ||
| ) { | ||
| if (this.dialogData) { | ||
| this.item = this.dialogData.item; | ||
| this.profileItem = this.dialogData.profileItem; | ||
| } | ||
| } | ||
|
|
||
| ngOnInit(): void { | ||
| const existing = this.item?.LocnVO ?? this.profileItem?.LocnVOs?.[0]; | ||
| if (!existing) { | ||
| return; | ||
| } | ||
| this.locationBeingEdited = { ...existing }; | ||
| this.coordinates.set(coordinatesFromLocation(existing)); | ||
| } | ||
|
|
||
| public onCoordinatesChange(coordinates: Coordinates | null): void { | ||
| this.coordinates.set(coordinates); | ||
| } | ||
|
|
||
| public onValidityChange(isValid: boolean): void { | ||
| this.isValid.set(isValid); | ||
| } | ||
|
|
||
| public cancel(): void { | ||
| this.dialogRef?.close(); | ||
| } | ||
|
|
||
| public save(): void { | ||
| if (!this.isValid()) { | ||
| return; | ||
| } | ||
| const coordinates = this.coordinates(); | ||
| this.dialogRef?.close({ | ||
| location: { | ||
| ...this.locationBeingEdited, | ||
| latitude: coordinates?.latitude ?? null, | ||
| longitude: coordinates?.longitude ?? null, | ||
| }, | ||
| }); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we'll remove the comment once we get rid of the flag?