From 9da3967dd972a02426beab7b4ac5ad310629e4be Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 19 Aug 2026 19:34:00 -0400 Subject: [PATCH] docs: record that destinations are configuration Beside the three navigation paragraphs, since it is the fourth thing the shell states about where the user can go. Two notes are measured facts rather than design: a default expressed as an *absent key* survives an existing seed, where one expressed as a zero value does not; and a spec can no longer assume a destination has a nav item. --- .pi/skills/yellowjacket-dev/SKILL.md | 6 +++ .planning/NOTES.md | 36 +++++++++++++++ CLAUDE.md | 65 ++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+) diff --git a/.pi/skills/yellowjacket-dev/SKILL.md b/.pi/skills/yellowjacket-dev/SKILL.md index c3b738f..16686c7 100644 --- a/.pi/skills/yellowjacket-dev/SKILL.md +++ b/.pi/skills/yellowjacket-dev/SKILL.md @@ -195,6 +195,12 @@ Two rules about climbing: - **Do not write an e2e spec first.** Drive the flow by hand, then promote it with `/e2e`. Specs written blind assert on selectors that do not exist. +- **Not every view has a nav item.** Since #25 the destinations are + configurable, Autotag is hidden by default and Downloads is absent + until a download client exists — so `getByTestId('nav-')` waits + 30 s for a locator that will never resolve. `navigateTo(page, view)` + (`e2e/support/fixtures.ts`) dispatches the app's own `navigate` event. + Click the nav item when the *nav* is what the spec is about. Before a commit, the gate is `make lint`, `make test`, `make ui-test`, `make bindings-check`, `make css-check` and — from `frontend/` — diff --git a/.planning/NOTES.md b/.planning/NOTES.md index 5230157..c7d3adf 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -3696,3 +3696,39 @@ as a tidy-up, a change nothing on a desktop renders differently. Related: a width-gated decision **is** testable at both tiers, which is why #61's phone mini player is a `matchMedia` stub in the component test and needs nothing special. + +## A default that is an *absent* key survives an existing seed (2026-08-19) + +The skill warns that a seed freezes every default it has already +persisted, so changing one in `backend/config` is invisible against an +existing `YJ_HOME` while CI, which seeds by running the app, tests the +new one. That warning is about defaults stored as *values*. + +#25's Autotag-hidden default is stored as the **absence of a key**: +`GeneralConfig.ViewVisibility` is a map, an id it does not mention takes +`backend/config.Views`' answer, and only what the user changed is ever +written. So a seed built before the feature existed showed the new +default immediately — verified against `.dev/seeds/default.tar`, whose +`config.toml` has no `[General.ViewVisibility]` table at all, and whose +sidebar came up without Autotag on the first launch of the new binary. +After toggling it on and off again the file carries exactly one line, +`autotag = false`. + +The general form is worth keeping: **a default expressed as a zero value +needs a re-seed to observe; a default expressed as an absent key does +not**, and it needs no migration for existing installs either. It is the +same property that makes removing a view later free (an unknown key is +dropped on load), which is what the `#25 → #27` ordering on #73 rests on. + +## A spec cannot assume a destination has a nav item (2026-08-19) + +Since #25, `getByTestId('nav-')` is not a reliable way to reach a +view: Autotag is hidden by default and Downloads is absent without a +download client, so four existing specs failed on a 30 s timeout waiting +for a locator that will never resolve. `navigateTo(page, view)` in +`e2e/support/fixtures.ts` dispatches the app's own `navigate` event +instead, which is what every nav item, card and detail view dispatches — +so it is the mechanism and not a test-only door. + +Use the nav item when the *nav* is the subject, and `navigateTo` when +the view is. diff --git a/CLAUDE.md b/CLAUDE.md index c0e7826..4664818 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1032,6 +1032,71 @@ whole way through — so it was green on the broken build. Same trap as `layout-overflow.spec.ts` and `page-header`: a spec named for the behaviour, measuring the plumbing. +**Which destinations exist is configuration, and hiding one takes away +the nav item and nothing else.** Eleven sidebar entries is more than +most libraries need (#25), so each is toggleable from Settings → +Navigation, Autotag is off until asked for, and Downloads is absent +until there is a client to download with — a destination for a feature +that cannot work is worse than none. `navigate` still resolves a hidden +view, which is not a nicety: detail views navigate into these and the +launch page is one of them. Nothing needed a special case for the +highlight either, because the paragraph above moved that onto +`active-view-store`: the sidebar asks `isActive(id)` per *rendered* +item, so a hidden view lights nothing exactly as a detail view does. + +Five things about it are load-bearing. + +**The stored shape is a map keyed by view id, and an absent key means +that view's own default** (`backend/config.Views`). That is what makes +this need no migration in either direction, and it is the polarity rule +`AllowMeteredCatalogDownload` states: the zero value is the intended +answer. A `HiddenViews []string` cannot express "Autotag off by +default" at all — its zero value is *hide nothing* — and a struct with +a boolean per view turns a view that later stops existing into stored +garbage. Here an unknown key is dropped on load and a view added later +gets its own default rather than being invisible or forcibly visible. +It is also what makes #73's `#25 → #27` order safe rather than +backwards: when Jobs folds into Settings, `jobs = true` in somebody's +config is a key nothing asks about. + +**Two states the user could not get out of are refused, in the config +and not in the checkbox.** Settings is never hideable and the launch +page is not hideable while it is the launch page. `config.toml` is +hand-editable, so a disabled checkbox is the affordance and +`SetViewVisible` is the rule — an app that can be locked out of its own +Settings by a typo in TOML is a support problem nobody can debug +remotely. On *load* the launch page is instead un-hidden rather than +refused: there is nobody to tell, and the honest reading of "my launch +page is Autotag" is that this user wants Autotag, not that their launch +page should be silently reset to something they did not choose. + +**Downloads is gated at the nav and not in the config**, on +`downloadStore.available`, so switching it on in Settings still means +what it says once a client exists and the tab appears without a restart +(#37's rule). `available` is false until the providers have loaded, +which makes the item *appear* on a fresh launch rather than appearing +and then vanishing. + +**The tab bar honours the toggles too, and the reason is local rather +than a general rule about phones.** `PHONE_COLUMN_IDS` is the precedent +for "what a phone shows is a different question", and it would apply — +except that `bottom-nav`'s "More" opens the *same* ``, +which filters, so an unfiltered bar would contradict its own drawer one +tap away. Which four tabs is still plan 016's committed subset; this +only removes from it, and "More" is never filtered because it is how +everything else stays reachable. + +**The list of destinations is `services/view-meta.ts`**, on +`shortcut-meta.ts`'s pattern, because #25 gave it a second reader: +Settings renders a toggle per view and needs the same labels in the +same order. Which views exist and what an unconfigured install shows is +Go's (`backend/config.Views`, which `DefaultPage`'s validation reads +too, so the launchable set is not a second list); how they are *drawn* +is the frontend's, beside the rest of the icon vocabulary. The binding +returns the **resolved** map for every view, so the frontend holds no +copy of the defaults — which would be the copy that shipped in the +binary rather than the one being edited. + **A primary view is cached, not unmounted.** `index.ts` keeps every primary view in the DOM and toggles a `.view-hidden` class, because that is what preserves `scrollTop` across navigation — so