Revert "Keep a place, and come back to it"

This reverts commit bd3a5a4. The bookmarks work was committed onto this branch
by mistake — it belongs to its own pull request, and it has one, branched from
this branch's own tip. Reverting rather than rewinding because the branch is
published and a pull request is open against it: the diff this pull request
shows is what matters, and after this it shows the routing change alone.
This commit is contained in:
2026-08-20 19:10:24 +02:00
parent bd3a5a48c9
commit 9cdd8f9388
10 changed files with 18 additions and 513 deletions
@@ -1,109 +0,0 @@
import { TestBed } from '@angular/core/testing';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { Bookmark, BookmarksStore } from './bookmarks.store';
const KEY = 'star-map.bookmarks';
const SIRIUS: Bookmark = { kind: 'star', id: 32349, name: 'Sirius' };
const EARTH: Bookmark = { kind: 'body', id: 'earth', name: 'Earth' };
function store(): BookmarksStore {
// Constructed per test, because the list is read once on construction — which is the
// behaviour being tested for anything that seeds storage first.
TestBed.resetTestingModule();
return TestBed.inject(BookmarksStore);
}
describe('BookmarksStore', () => {
beforeEach(() => {
localStorage.clear();
vi.restoreAllMocks();
});
it('starts empty, and keeps what it is given', () => {
const bookmarks = store();
expect(bookmarks.bookmarks()).toEqual([]);
expect(bookmarks.toggle(SIRIUS)).toBe(true);
expect(bookmarks.bookmarks()).toEqual([SIRIUS]);
expect(bookmarks.has('star', 32349)).toBe(true);
});
it('drops what it already had, when told the same thing twice', () => {
const bookmarks = store();
bookmarks.toggle(SIRIUS);
expect(bookmarks.toggle(SIRIUS)).toBe(false);
expect(bookmarks.bookmarks()).toEqual([]);
expect(bookmarks.has('star', 32349)).toBe(false);
});
it('tells a star from a body that happen to share an id', () => {
const bookmarks = store();
bookmarks.toggle({ kind: 'star', id: 1, name: 'A star' });
bookmarks.toggle({ kind: 'body', id: 1, name: 'A body' });
expect(bookmarks.bookmarks()).toHaveLength(2);
bookmarks.remove('star', 1);
expect(bookmarks.bookmarks()).toEqual([{ kind: 'body', id: 1, name: 'A body' }]);
});
it('puts the newest first, since that is the one being come back to', () => {
const bookmarks = store();
bookmarks.toggle(SIRIUS);
bookmarks.toggle(EARTH);
expect(bookmarks.bookmarks().map((bookmark) => bookmark.name)).toEqual(['Earth', 'Sirius']);
});
it('survives the visit it was kept in', () => {
store().toggle(SIRIUS);
expect(store().bookmarks()).toEqual([SIRIUS]);
});
it('reads past whatever else is in there, rather than losing the lot', () => {
localStorage.setItem(KEY, JSON.stringify([SIRIUS, { kind: 'moon', id: 1, name: 'No such kind' }, { id: 'no-kind' }, null, 42, EARTH]));
expect(store().bookmarks()).toEqual([SIRIUS, EARTH]);
});
it('keeps one entry per place, however many the stored list holds', () => {
// Two entries for one place would each toggle the other's control on and off.
localStorage.setItem(KEY, JSON.stringify([SIRIUS, { ...SIRIUS, name: 'Sirius (again)' }]));
expect(store().bookmarks()).toEqual([SIRIUS]);
});
it('treats a store that is not a list, or not JSON at all, as no bookmarks', () => {
localStorage.setItem(KEY, '{"not":"a list"}');
expect(store().bookmarks()).toEqual([]);
localStorage.setItem(KEY, 'nonsense{');
expect(store().bookmarks()).toEqual([]);
});
it('goes on working where the browser will not store anything at all', () => {
// Private mode, a full quota, storage disabled by policy: reading throws, writing throws.
vi.spyOn(Storage.prototype, 'getItem').mockImplementation(() => {
throw new DOMException('denied');
});
vi.spyOn(Storage.prototype, 'setItem').mockImplementation(() => {
throw new DOMException('denied');
});
const bookmarks = store();
expect(bookmarks.bookmarks()).toEqual([]);
expect(() => bookmarks.toggle(SIRIUS)).not.toThrow();
// Kept for this visit, even though nothing will outlive it.
expect(bookmarks.bookmarks()).toEqual([SIRIUS]);
});
it('bounds what a hand-edited store can make it hold', () => {
const many = Array.from({ length: 500 }, (_, i) => ({ kind: 'star' as const, id: i, name: `Star ${i}` }));
localStorage.setItem(KEY, JSON.stringify(many));
expect(store().bookmarks()).toHaveLength(200);
});
});
-111
View File
@@ -1,111 +0,0 @@
import { Injectable, signal } from '@angular/core';
/**
* A place someone chose to keep. Either a star, which is a system to fly into, or a body, which
* is a page to open — the two things the map lets you arrive at.
*/
export interface Bookmark {
readonly kind: 'star' | 'body';
/** A HYG star id, or a `bodies.json`/`exoplanets.json` id. */
readonly id: number | string;
/**
* The name as it read when it was kept. Stored rather than looked up, so the list can be
* shown before the catalogues have loaded — and so a bookmark to something a later catalogue
* no longer holds still says what it was rather than becoming a bare id.
*/
readonly name: string;
}
const STORAGE_KEY = 'star-map.bookmarks';
/**
* How many are kept. Not a limit anyone will reach by hand — it is a bound on what a corrupted
* or hand-edited store can make the app render, and on what is written back.
*/
const MAX_BOOKMARKS = 200;
/** `${kind}:${id}`, since a star id and a body id are different kinds of thing. */
function keyOf(kind: Bookmark['kind'], id: number | string): string {
return `${kind}:${id}`;
}
function isBookmark(value: unknown): value is Bookmark {
if (typeof value !== 'object' || value === null) {
return false;
}
const candidate = value as Partial<Bookmark>;
return (
(candidate.kind === 'star' || candidate.kind === 'body') &&
(typeof candidate.id === 'number' || typeof candidate.id === 'string') &&
typeof candidate.name === 'string'
);
}
/**
* The places kept between visits, in this browser and nowhere else.
*
* Local storage rather than an account: the map asks nobody to sign in, and a list of stars
* somebody liked is not worth a server. Every read of it is defensive — the store is a string
* a user can edit, another tab can write, and a browser can refuse to give at all — and a
* failure to read or write one is never allowed to take the map down with it.
*/
@Injectable({ providedIn: 'root' })
export class BookmarksStore {
private readonly kept = signal<readonly Bookmark[]>(this.read());
/** Most recently kept first, which is the order they are useful in. */
readonly bookmarks = this.kept.asReadonly();
has(kind: Bookmark['kind'], id: number | string): boolean {
return this.kept().some((bookmark) => bookmark.kind === kind && bookmark.id === id);
}
/** Keeps a place, or drops it if it was already kept. Returns whether it is kept now. */
toggle(bookmark: Bookmark): boolean {
const kept = this.has(bookmark.kind, bookmark.id);
this.write(kept ? this.kept().filter((other) => !(other.kind === bookmark.kind && other.id === bookmark.id)) : [bookmark, ...this.kept()].slice(0, MAX_BOOKMARKS));
return !kept;
}
remove(kind: Bookmark['kind'], id: number | string): void {
this.write(this.kept().filter((bookmark) => !(bookmark.kind === kind && bookmark.id === id)));
}
private write(bookmarks: readonly Bookmark[]): void {
this.kept.set(bookmarks);
try {
localStorage.setItem(STORAGE_KEY, JSON.stringify(bookmarks));
} catch {
// Full, disabled, or private-mode storage. The list still works for this visit; it just
// will not outlive it, which is a smaller loss than the alternative of failing here.
}
}
private read(): Bookmark[] {
let raw: string | null = null;
try {
raw = localStorage.getItem(STORAGE_KEY);
} catch {
return [];
}
if (!raw) {
return [];
}
try {
const parsed: unknown = JSON.parse(raw);
if (!Array.isArray(parsed)) {
return [];
}
// Filtered rather than rejected wholesale: one bad entry should not lose the others, and
// deduplicated because two entries for one place would each toggle the other's control.
const seen = new Set<string>();
return parsed
.filter(isBookmark)
.filter((bookmark) => !seen.has(keyOf(bookmark.kind, bookmark.id)) && seen.add(keyOf(bookmark.kind, bookmark.id)))
.slice(0, MAX_BOOKMARKS)
.map(({ kind, id, name }) => ({ kind, id, name }));
} catch {
return [];
}
}
}