Keep a place, and come back to it

Restores the commit reverted off the routing branch, which is where it was
committed by mistake. The change is unmodified; only its branch is.

The map had no memory. Every visit started at the same overview, and a system
worth returning to had to be found again by name each time. A mark on the
readout and on a body's panel now keeps it, a Bookmarks tab lists what has been
kept, and choosing one goes there — a star by flying into its system, a body by
opening its page.

Local storage, not an account. This map asks nobody to sign in, and a list of
stars somebody liked is not worth a server. Every read of that store is
defensive, because it is a string a person can edit, another tab can write, and
a browser can refuse to hand over at all: a bad entry is skipped rather than
losing the rest, duplicates are collapsed since two entries for one place would
each toggle the other's control, the list is bounded so a hand-edited store
cannot decide how much this renders, and where storage is denied outright the
bookmarks still work for the visit — they just do not outlive it.

The name is stored alongside the id rather than looked up, so the list reads
before the catalogues have loaded, and a bookmark to something a later
catalogue no longer holds still says what it was instead of decaying into a
bare number.

The tab is offered even when it is empty, and says what the mark does: a tab
that appears only once you have already found the feature is a tab that never
taught anyone anything.

Choosing a kept place hands the panel back to the readout, the same move as
choosing a search result and for the same reason. That behaviour is what the
end-to-end spec caught missing — the readout it asserted on did not exist,
because the panel just used was still covering it.

Verified: build clean, 587/587 unit, 11/11 end-to-end, design detector clean,
screenshots at 1440x900 and 390x844.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016jxMkwA2rbicdGxHosecYi

src/app/shared/state/bookmarks.store.spec.ts

@@ -0,0 +1,109 @@
+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);
+  });
+});

src/app/shared/state/bookmarks.store.ts

@@ -0,0 +1,111 @@
+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 [];
+    }
+  }
+}

src/app/shared/ui/bookmark-icon.component.ts

@@ -0,0 +1,22 @@
+import { ChangeDetectionStrategy, Component, input } from '@angular/core';
+
+/**
+ * The mark on a place worth coming back to. Hollow when it is not kept, filled when it is —
+ * the same two states as the Display panel's layer ticks, which is where the eye has already
+ * learned what a filled mark means here.
+ *
+ * Size and colour come from the classes on the host; the svg fills it.
+ */
+@Component({
+  selector: 'app-bookmark-icon',
+  changeDetection: ChangeDetectionStrategy.OnPush,
+  host: { class: 'block', 'aria-hidden': 'true' },
+  template: `
+    <svg class="h-full w-full" viewBox="0 0 24 24" [attr.fill]="kept() ? 'currentColor' : 'none'" stroke="currentColor" stroke-width="1.5" stroke-linejoin="round">
+      <path d="M7 4h10v16l-5-4-5 4z" />
+    </svg>
+  `
+})
+export class BookmarkIconComponent {
+  readonly kept = input(false);
+}