Configurable sidebar destinations, Downloads gated on a client, Autotag off by default #145
@@ -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-<view>')` 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/` —
|
||||
|
||||
@@ -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-<view>')` 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.
|
||||
|
||||
@@ -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* `<app-sidebar>`,
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user