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