Properly handle place removals
* Transition to OSM route or index instead of staying on ghost route/ID (closes sidebar if it was a custom place) * Ensure save button and lists are in the correct state
This commit is contained in:
@@ -1,4 +1,5 @@
|
|||||||
import Component from '@glimmer/component';
|
import Component from '@glimmer/component';
|
||||||
|
import { service } from '@ember/service';
|
||||||
import { on } from '@ember/modifier';
|
import { on } from '@ember/modifier';
|
||||||
import { htmlSafe } from '@ember/template';
|
import { htmlSafe } from '@ember/template';
|
||||||
import { humanizeOsmTag } from '../utils/format-text';
|
import { humanizeOsmTag } from '../utils/format-text';
|
||||||
@@ -13,9 +14,14 @@ import { tracked } from '@glimmer/tracking';
|
|||||||
import { action } from '@ember/object';
|
import { action } from '@ember/object';
|
||||||
|
|
||||||
export default class PlaceDetails extends Component {
|
export default class PlaceDetails extends Component {
|
||||||
|
@service storage;
|
||||||
@tracked isEditing = false;
|
@tracked isEditing = false;
|
||||||
@tracked showLists = false;
|
@tracked showLists = false;
|
||||||
|
|
||||||
|
get isSaved() {
|
||||||
|
return this.storage.isPlaceSaved(this.place.id || this.place.osmId);
|
||||||
|
}
|
||||||
|
|
||||||
get place() {
|
get place() {
|
||||||
return this.args.place || {};
|
return this.args.place || {};
|
||||||
}
|
}
|
||||||
@@ -38,7 +44,7 @@ export default class PlaceDetails extends Component {
|
|||||||
|
|
||||||
@action
|
@action
|
||||||
startEditing() {
|
startEditing() {
|
||||||
if (!this.place.createdAt) return; // Only allow editing saved places
|
if (!this.isSaved) return; // Only allow editing saved places
|
||||||
this.isEditing = true;
|
this.isEditing = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -276,7 +282,7 @@ export default class PlaceDetails extends Component {
|
|||||||
<button
|
<button
|
||||||
type="button"
|
type="button"
|
||||||
class={{if
|
class={{if
|
||||||
this.place.createdAt
|
this.isSaved
|
||||||
"btn btn-secondary"
|
"btn btn-secondary"
|
||||||
"btn btn-outline"
|
"btn btn-outline"
|
||||||
}}
|
}}
|
||||||
@@ -284,20 +290,21 @@ export default class PlaceDetails extends Component {
|
|||||||
>
|
>
|
||||||
<Icon
|
<Icon
|
||||||
@name="bookmark"
|
@name="bookmark"
|
||||||
@color={{if this.place.createdAt "currentColor" "#007bff"}}
|
@color={{if this.isSaved "currentColor" "#007bff"}}
|
||||||
/>
|
/>
|
||||||
{{if this.place.createdAt "Saved" "Save"}}
|
{{if this.isSaved "Saved" "Save"}}
|
||||||
</button>
|
</button>
|
||||||
|
|
||||||
{{#if this.showLists}}
|
{{#if this.showLists}}
|
||||||
<PlaceListsManager
|
<PlaceListsManager
|
||||||
@place={{this.saveablePlace}}
|
@place={{this.saveablePlace}}
|
||||||
@onClose={{this.closeLists}}
|
@onClose={{this.closeLists}}
|
||||||
|
@isSaved={{this.isSaved}}
|
||||||
/>
|
/>
|
||||||
{{/if}}
|
{{/if}}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{{#if this.place.createdAt}}
|
{{#if this.isSaved}}
|
||||||
<button
|
<button
|
||||||
type="button"
|
type="button"
|
||||||
class="btn btn-outline"
|
class="btn btn-outline"
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import Component from '@glimmer/component';
|
import Component from '@glimmer/component';
|
||||||
import { service } from '@ember/service';
|
import { service } from '@ember/service';
|
||||||
import { action } from '@ember/object';
|
import { action } from '@ember/object';
|
||||||
|
import { tracked } from '@glimmer/tracking';
|
||||||
import { on } from '@ember/modifier';
|
import { on } from '@ember/modifier';
|
||||||
import { fn } from '@ember/helper';
|
import { fn } from '@ember/helper';
|
||||||
import { htmlSafe } from '@ember/template';
|
import { htmlSafe } from '@ember/template';
|
||||||
@@ -8,12 +9,15 @@ import onClickOutside from '../modifiers/on-click-outside';
|
|||||||
|
|
||||||
export default class PlaceListsManager extends Component {
|
export default class PlaceListsManager extends Component {
|
||||||
@service storage;
|
@service storage;
|
||||||
|
@service router;
|
||||||
|
@tracked _forceClear = false;
|
||||||
|
|
||||||
get isSaved() {
|
get isSaved() {
|
||||||
return !!this.args.place.createdAt;
|
return this.args.isSaved;
|
||||||
}
|
}
|
||||||
|
|
||||||
get placeListIds() {
|
get placeListIds() {
|
||||||
|
if (this._forceClear) return [];
|
||||||
return this.args.place._listIds || [];
|
return this.args.place._listIds || [];
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -30,7 +34,35 @@ export default class PlaceListsManager extends Component {
|
|||||||
@action
|
@action
|
||||||
async toggleSaved() {
|
async toggleSaved() {
|
||||||
if (this.isSaved) {
|
if (this.isSaved) {
|
||||||
|
const { osmId, osmType } = this.args.place;
|
||||||
|
|
||||||
await this.storage.removePlace(this.args.place);
|
await this.storage.removePlace(this.args.place);
|
||||||
|
|
||||||
|
// Clean up the local object reference immediately to prevent UI flicker
|
||||||
|
// or stale state if the transition is delayed/cancelled.
|
||||||
|
if (this.args.place) {
|
||||||
|
this.args.place.id = null;
|
||||||
|
this.args.place.createdAt = null;
|
||||||
|
this.args.place._listIds = [];
|
||||||
|
this._forceClear = true;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Transition immediately to the canonical state
|
||||||
|
if (osmId && osmType) {
|
||||||
|
// Create a transient copy that looks like a fresh OSM result
|
||||||
|
const rawPlace = { ...this.args.place };
|
||||||
|
delete rawPlace.id;
|
||||||
|
delete rawPlace.createdAt;
|
||||||
|
delete rawPlace._listIds;
|
||||||
|
|
||||||
|
// Transition to the place route using the raw object
|
||||||
|
// This updates the URL to 'osm:...' and renders immediately
|
||||||
|
this.router.transitionTo('place', rawPlace);
|
||||||
|
} else {
|
||||||
|
// Custom place deleted -> go home
|
||||||
|
this.router.transitionTo('index');
|
||||||
|
}
|
||||||
|
|
||||||
if (this.args.onClose) this.args.onClose();
|
if (this.args.onClose) this.args.onClose();
|
||||||
} else {
|
} else {
|
||||||
await this.storage.storePlace(this.args.place);
|
await this.storage.storePlace(this.args.place);
|
||||||
|
|||||||
@@ -282,11 +282,22 @@ export default class StorageService extends Service {
|
|||||||
let place = this.savedPlaces.find((p) => p.id && String(p.id) === strId);
|
let place = this.savedPlaces.find((p) => p.id && String(p.id) === strId);
|
||||||
if (place) return place;
|
if (place) return place;
|
||||||
|
|
||||||
|
// Check placesInView as fallback
|
||||||
|
place = this.placesInView.find((p) => p.id && String(p.id) === strId);
|
||||||
|
if (place) return place;
|
||||||
|
|
||||||
// Then search by OSM ID
|
// Then search by OSM ID
|
||||||
place = this.savedPlaces.find((p) => p.osmId && String(p.osmId) === strId);
|
place = this.savedPlaces.find((p) => p.osmId && String(p.osmId) === strId);
|
||||||
|
if (place) return place;
|
||||||
|
|
||||||
|
place = this.placesInView.find((p) => p.osmId && String(p.osmId) === strId);
|
||||||
return place;
|
return place;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
isPlaceSaved(id) {
|
||||||
|
return !!this.findPlaceById(id);
|
||||||
|
}
|
||||||
|
|
||||||
async storePlace(placeData) {
|
async storePlace(placeData) {
|
||||||
const savedPlace = await this.places.store(placeData);
|
const savedPlace = await this.places.store(placeData);
|
||||||
|
|
||||||
|
|||||||
@@ -42,6 +42,9 @@ class MockStorageService extends Service {
|
|||||||
findPlaceById() {
|
findPlaceById() {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
isPlaceSaved() {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
loadPlacesInBounds() {
|
loadPlacesInBounds() {
|
||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -41,6 +41,9 @@ module('Acceptance | search', function (hooks) {
|
|||||||
findPlaceById() {
|
findPlaceById() {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
isPlaceSaved() {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
rs = {
|
rs = {
|
||||||
on: () => {},
|
on: () => {},
|
||||||
};
|
};
|
||||||
@@ -85,6 +88,9 @@ module('Acceptance | search', function (hooks) {
|
|||||||
findPlaceById() {
|
findPlaceById() {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
isPlaceSaved() {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
rs = {
|
rs = {
|
||||||
on: () => {},
|
on: () => {},
|
||||||
};
|
};
|
||||||
@@ -130,6 +136,9 @@ module('Acceptance | search', function (hooks) {
|
|||||||
if (id === '999') return this.savedPlaces[0];
|
if (id === '999') return this.savedPlaces[0];
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
isPlaceSaved(id) {
|
||||||
|
return !!this.findPlaceById(id);
|
||||||
|
}
|
||||||
rs = {
|
rs = {
|
||||||
on: () => {},
|
on: () => {},
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -13,6 +13,14 @@ module('Integration | Component | place-details', function (hooks) {
|
|||||||
{ id: 'to-do', title: 'To do', color: '#008000' },
|
{ id: 'to-do', title: 'To do', color: '#008000' },
|
||||||
];
|
];
|
||||||
|
|
||||||
|
isPlaceSaved(id) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
findPlaceById(id) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
async storePlace(place) {
|
async storePlace(place) {
|
||||||
return { ...place, id: '123', createdAt: new Date().toISOString() };
|
return { ...place, id: '123', createdAt: new Date().toISOString() };
|
||||||
}
|
}
|
||||||
@@ -28,6 +36,12 @@ module('Integration | Component | place-details', function (hooks) {
|
|||||||
|
|
||||||
hooks.beforeEach(function () {
|
hooks.beforeEach(function () {
|
||||||
this.owner.register('service:storage', StorageService);
|
this.owner.register('service:storage', StorageService);
|
||||||
|
|
||||||
|
// Mock Router for all tests
|
||||||
|
class MockRouter extends Service {
|
||||||
|
transitionTo() {}
|
||||||
|
}
|
||||||
|
this.owner.register('service:router', MockRouter);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('it formats coordinates correctly', async function (assert) {
|
test('it formats coordinates correctly', async function (assert) {
|
||||||
@@ -95,6 +109,12 @@ module('Integration | Component | place-details', function (hooks) {
|
|||||||
storedPlace = place;
|
storedPlace = place;
|
||||||
return { ...place, id: 'new-id', createdAt: new Date().toISOString() };
|
return { ...place, id: 'new-id', createdAt: new Date().toISOString() };
|
||||||
}
|
}
|
||||||
|
isPlaceSaved() {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
findPlaceById() {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
this.owner.register('service:storage', MockStorage);
|
this.owner.register('service:storage', MockStorage);
|
||||||
|
|
||||||
@@ -126,12 +146,15 @@ module('Integration | Component | place-details', function (hooks) {
|
|||||||
});
|
});
|
||||||
|
|
||||||
test('it handles removing a saved place via master toggle', async function (assert) {
|
test('it handles removing a saved place via master toggle', async function (assert) {
|
||||||
let removedPlace = null;
|
let removedPlaceId = null;
|
||||||
|
|
||||||
class MockStorage extends Service {
|
class MockStorage extends Service {
|
||||||
lists = [];
|
lists = [];
|
||||||
async removePlace(place) {
|
async removePlace(place) {
|
||||||
removedPlace = place;
|
removedPlaceId = place.id;
|
||||||
|
}
|
||||||
|
isPlaceSaved() {
|
||||||
|
return true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
this.owner.register('service:storage', MockStorage);
|
this.owner.register('service:storage', MockStorage);
|
||||||
@@ -160,7 +183,9 @@ module('Integration | Component | place-details', function (hooks) {
|
|||||||
// Click it to remove
|
// Click it to remove
|
||||||
await click(masterToggle);
|
await click(masterToggle);
|
||||||
|
|
||||||
assert.strictEqual(removedPlace.id, 'saved-id', 'removePlace was called');
|
assert.strictEqual(removedPlaceId, 'saved-id', 'removePlace was called');
|
||||||
|
|
||||||
|
assert.deepEqual(place._listIds, [], '_listIds was cleared on the object');
|
||||||
});
|
});
|
||||||
|
|
||||||
test('it adds place to a list', async function (assert) {
|
test('it adds place to a list', async function (assert) {
|
||||||
@@ -175,6 +200,9 @@ module('Integration | Component | place-details', function (hooks) {
|
|||||||
listId = id;
|
listId = id;
|
||||||
shouldAdd = add;
|
shouldAdd = add;
|
||||||
}
|
}
|
||||||
|
isPlaceSaved() {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
this.owner.register('service:storage', MockStorage);
|
this.owner.register('service:storage', MockStorage);
|
||||||
|
|
||||||
@@ -202,4 +230,29 @@ module('Integration | Component | place-details', function (hooks) {
|
|||||||
assert.strictEqual(placeArg.id, 'p1');
|
assert.strictEqual(placeArg.id, 'p1');
|
||||||
assert.true(shouldAdd);
|
assert.true(shouldAdd);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('it respects storage service state over stale place object', async function (assert) {
|
||||||
|
class MockStorage extends Service {
|
||||||
|
lists = [];
|
||||||
|
isPlaceSaved() {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
findPlaceById() {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
this.owner.register('service:storage', MockStorage);
|
||||||
|
|
||||||
|
const place = {
|
||||||
|
id: 'stale-id',
|
||||||
|
title: 'Stale Place',
|
||||||
|
createdAt: '2023-01-01', // Looks saved
|
||||||
|
};
|
||||||
|
|
||||||
|
await render(<template><PlaceDetails @place={{place}} /></template>);
|
||||||
|
|
||||||
|
// Button should say "Save", not "Saved" because isPlaceSaved returns false
|
||||||
|
assert.dom('.actions button').hasText('Save');
|
||||||
|
assert.dom('.actions button').doesNotHaveClass('btn-secondary');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user