One connection-settings store for every application - #286
Conversation
Six sites ship a "point this app at a different backend" page, and each wrote its own: pays.online, honey.id, nofilter.io, web3.trading, support.cafe and 24x.ai, in three different file layouts, with 58 of honey.id's 164 store lines identical to pays.online's. They had drifted apart in behaviour, not only in shape, which is how pays.online ended up with a toggle that flips a checkbox and reveals nothing. The storage model is per-endpoint rather than global. Four sites had one `useCustomUrl` covering every backend at once; support.cafe and 24x.ai had already grown a flag per backend, because pointing the API at a local instance while leaving auth on production is the ordinary case. Per-endpoint covers both, and `setUseCustom` flips them together for a page that wants one switch. Three behaviours are kept from the copies that had them, because each was learned the hard way: - A stored value is data from outside the program. Every field is validated on read and anything unrecognised falls back, so a settings page cannot be locked shut by a value someone typed by hand. - Nothing customised clears the stored copy rather than writing the defaults into it. Otherwise a later change to a default is masked by a stale one. - `isApplying` covers the awaited `onApply`, not just the write, so a page can disable its form for the whole reconnect. Seven tests, run under `--conditions=browser` because the server build of Solid never propagates a signal and every assertion would otherwise pass while testing nothing. The one asserting `isApplying` holds the callback open deliberately: a callback that returns inside the same batch flips the flag back before anything can observe it.
This repository does not hand-write tests. `components.ts` declares a component, `qa:checks` generates its .ron checks, `qa:entries` generates its entry bundle, and CI drives every page in the native renderer, failing if the generated files were not regenerated and committed. The generator says so in its own header: nobody writes a check by hand. A hand-written bun test restates the implementation in assertions, which is the code written twice, and it proves nothing about the renderer the components actually run in. The store gets its coverage through the panel that consumes it, registered like every other component.
…places Six sites hand-wrote this page and one of them, pays.online, ships it broken: the switch flips, the tree reports it on, and the URL fields never appear, because the page reads the flag once outside a tracked scope. Typecheck, lint and build all pass on it. The component owns the switch state in a signal of its own rather than reading it back out of the store, which is the shape that cannot be read untracked by accident, and the revealed region is mounted with `Show` rather than hidden with CSS, so its absence is observable rather than merely invisible. `children` renders inside that region. Every site has one field the others do not -- an app id, a second backend, a network selector -- and this keeps them inside the same reveal instead of forcing a second copy of the panel. It is also what lets the fixture name the callback result the `action` kind asserts on. Registered in components.ts, so the checks and the entry are generated. Two things the sweep found that no other gate would have: - The subject role is `switch`, not `checkbox`. Declaring the wrong one failed both interaction checks against a control that was painting correctly. - Blitz dispatches a checkbox's click but not its change event, so the switch was wired to `onChange` and revealed nothing -- the same symptom as the defect this replaces, from a different cause. `onClick` reads `checked` and works under both. 4/4 under `tests/qa-harness/run-all.sh connection-settings`.
…r Blitz Blitz dispatches a checkbox's click and no change event, so all three were inert in the renderer this library is tested with: the tree reported the new state because the renderer flips `checked` itself, while `onChange` never fired. A panel that used Switch to reveal its fields revealed nothing. Nothing caught it, and the check that should have is the reason. `-toggles` compares the tree's `selected` before and after, which the renderer satisfies on its own. A controlled toggle whose callback never runs passes it while doing nothing at all. So the `toggle` kind now also generates `-reports`, against a fixture marker only the callback can raise. Getting that check right took three attempts, each worth recording: - Asserting the current state fails on ordering. Checks in a group share one host, so `-toggles` has already pressed the control and "on" is only correct when the number of presses is odd. The marker latches instead. - `heading:` with no name matches the page title, not the fixture's. - The fixture bound `onChange` and `onInput` both, to cover components that disagreed about which they took. Once the callback actually fired that became two flips per click, netting to no change, which reads exactly like the dead component it was meant to detect. The handlers dedupe per interaction rather than per value, because a controlled input whose DOM `checked` does not move again makes every click after the first look like a repeat.
Switch, Checkbox and Radio each bound `onChange` and `onClick` to one handler, deduped with a `queueMicrotask` flag, to cover a renderer that was thought to send only one of the two. The premise was wrong on both halves. The renderer does dispatch `change`; what it got wrong was the order of the click and three other things, all fixed in the engine. And the dedupe never deduped: a microtask checkpoint runs between the click and the change, so the flag was already cleared by the time `change` arrived and the handler ran twice per press. Two flips net to none, which is why the connection-settings panel would not open -- the fallback added to make a control respond was the reason it did not. The controls take `change`, which is what the platform provides.
|
Pushed The premise for that fallback was wrong on both halves. Blitz does dispatch
With the host built against the fixed engine, this branch is 17/17 across switch, checkbox, radio and connection-settings, and 73/74 over the whole library. The one failure is Two follow-ups this turned up, neither in this PR:
|
The switch wrote the override flags the moment it was flipped, and the store persists on every change, so opening the panel and navigating away left the application overridden while the URL fields beside it were still unsaved drafts. The one control that was not a draft was the one deciding whether any of the others counted. It is local state now, and Save commits it with the rest. Save also derives the flags per endpoint rather than calling `setUseCustom`. That helper flips every endpoint the store was configured with, including any the panel does not render, and marks an endpoint overridden even when its value is the fallback -- so "point the API somewhere else, leave auth on production", the case the per-endpoint model exists for, could not be expressed, and an endpoint pinned to a value that merely equalled today's default stayed pinned when that default moved. Addresses are checked before they are saved, by `validate` on the endpoint. Unchecked, a bad address is persisted, fails to connect, and is still there on the next launch with nothing on screen explaining it. The default check is a regex and deliberately not `new URL()`: the native renderer this library is tested against has no `URL` global, so a validator built on it throws, is caught, and refuses every address -- while passing every test written in a browser. It cost a red `-commits` to find that. `onApply` now receives the whole applied state. A site storing an app id here reconfigures its identity from the same call instead of reaching back into the store it had just handed over. `style` and `dataTheme` reach the root, which `UIBaseProps` promises and this component dropped. `urls` is resolved once for the list rather than per row. Reported in review of #286.
The root barrel exports the hook, the panel and their types, and this file is the source of truth every consuming application is pointed at. Six sites are meant to migrate onto it, so leaving it undescribed left them without a contract to migrate to.
The file's own rule is that fixtures import from component modules rather than the root, because a root import puts every component back into every page's graph, which is the failure the per-component entries exist to remove. This one import went to the root.
|
Folded in. The panel findings were right, and chasing one of them turned up a bug worth more than the finding. The switch committed before Save (7) and it flipped every endpoint (8). Both correct. The switch is local state now and Save commits it with everything else, so opening the panel and walking away changes nothing. Save also derives the flags per endpoint rather than calling Validation was only non-empty (9). Addresses are now checked before they are saved, by The default check is a regex and deliberately not
Design-system actions (17): the actions are the library's
Verified with a host built against the fixed engine: 17/17 across switch, checkbox, radio and connection-settings, and 73/74 over the whole library. The one failure is Two engine gaps this turned up, neither fixed here: no |
Two things, both consequences of `qa-inspect-host` no longer needing a window. `--locked` is gone. A lockfile is frozen at publication, so the host was pinned to whatever engine existed the day it was released, and a renderer fix could never reach this gate without a host release. Not hypothetical: `ps-blitz` 0.4.3 fixes three defects in checkbox activation, and the host published before it kept installing 0.4.2, so `switch`, `checkbox` and `radio` went on failing here against an engine that had already been fixed. The version mismatch `--locked` was added for is gone with it, since the host and its runtime now both require `^0.4` and resolve to one copy. And the job moves to Linux. It ran on macOS because the host could not be built anywhere else: it reached the engine through `tauri-runtime-blitz`, which brought the Tauri runtime, which does not compile on Linux at all. The host opens no window and now does not compile one, so there is nothing left to want a window system. macOS is for building packages, not for a component sweep.
The host does not read a font catalogue. `system-fonts` was the only thing pulling fontconfig into it, and a headless host asserting semantic outcomes has no reason to enumerate the machine's fonts. Dropped there; nothing to install here.
This PR moves the job from `macos-14` to `ubicloud-standard-2`, and zsh ships
with macOS but not with the Linux image. The sweep therefore died at
`zsh: command not found`, exit 127, after the bundle had already built and both
QA binaries had installed: everything the job exists to check had passed.
Nothing in the script needs zsh. Every conditional is `[[ ... ]]`, which bash
has; the array is only `ids+=("$line")` and `"${ids[@]}"`, with no numeric
indexing, so zsh's 1-based arrays never came into it; and `< <(...)` is a bash
construct to begin with. `bash -n` parses it clean.
Installing zsh on the runner was the other option and is the wrong one. The
shell is not what this job is testing.
Same correction ps-observability made when its host job moved off macOS:
`shell: zsh {0}` became `shell: bash {0}`.
The previous commit changed the shebang and the call site and claimed the
script was already bash-compatible. It was not, and the evidence used was too
weak: `bash -n` and a grep for `[[`, arrays and `=~`.
`bash -n` is a syntax check. These two lines parse fine under bash and mean
something else entirely:
readonly HERE="${0:A:h}"
readonly ROOT="${HERE:h:h}"
`:A` and `:h` are zsh parameter-expansion modifiers, the resolved absolute path
and dirname. Bash reads `${0:A:h}` as a substring expansion whose offset is the
variable `A`, so with `set -u` the script died on its second statement:
`run-all.sh: line 22: A: unbound variable`.
Replaced with `cd -- "$(dirname -- "$0")" && pwd -P`, and the two heads with
`$HERE/../..`. Checked by running both forms and comparing, rather than by
reading them:
bash: HERE=.../tests/qa-harness ROOT=.../UI
zsh: HERE=.../tests/qa-harness ROOT=.../UI
The script then runs under bash and drives components, which is further than it
reached before. Those were the only zsh-specific expansions in the file; the
arrays really are `ids+=(...)` and `"${ids[@]}"` with no numeric indexing.
Eight defects from review, verified against the pinned runtime rather than
reasoned about. One of them is why the panel could not work.
**Save and Reset applied the previous settings.** On `solid-js@2.0.0-rc.4` a
signal read straight after a write returns the old value until the microtask
flush:
setS({ n: 1 });
s().n // 0
await Promise.resolve();
s().n // 1
Only in the browser build. The server build answers `1` immediately, so a probe
run without an export condition disproves it, which is presumably how it
survived. The panel sets overrides, URLs and the app id and then calls `apply()`
in the same tick, so every read inside `apply` saw what the user had replaced:
`onApply` reconnected to the old addresses, the write persisted them, and the
screen updated a microtask later looking correct. Reset had the same shape,
reconnecting to the overrides it had just dropped.
The setters now advance a committed value eagerly and the signal follows for
anything tracking. Everything that has to agree -- what is persisted, what
`onApply` receives, what `isAtDefaults` reports -- reads from it. Measured under
the browser condition: the applied URL and app id are the ones just typed, and
reset applies the fallback.
**Saving the fallback resurrected an old custom URL.** The panel wrote the field
only while the override stayed on, so replacing a saved address with the
fallback disabled the override but left the old value in the store. The field
repopulated with it, and a second Save turned it back on.
**Disabled storage could stop the application starting.** `typeof localStorage`
sat outside the `try` in both read and write. It is a getter, `typeof` evaluates
it, and it throws `SecurityError` on an opaque origin -- so the availability
check threw before the access it guarded, at module scope, before anything
rendered.
**Enter did not save.** Implicit submission needs a submit button, or a single
field where there is none. Both actions are `type="button"` deliberately, and
the ordinary panel shows two URLs or a URL and an app id, so the `<form>`
carrying `onSubmit` never fired for the case its comment described. An explicit
handler on the fields, skipping an Enter that is committing an IME composition.
**Concurrent applies cleared the busy flag early.** Two applies -- a second
panel, Reset during a Save, a direct caller -- and whichever finished first said
idle while the other reconnect ran. Counted now.
**An empty app id changed meaning across a reload.** The setter and the write
accepted `""`; loading rejected it and substituted the configured default. Only
a non-string falls back now.
**The default validator matched a prefix.** Unanchored, `wss://host other`
passed because the pattern stopped at the space, and `wss://host:abc` passed
because the host run swallowed the colon. Anchored, with a digits-only port.
Still a regex and still not `new URL()`: the native renderer has no `URL`
global, which is the whole reason. Anchoring first broke `http://[::1]:80`, so
the host alternates a bracketed IPv6 literal -- caught by testing the cases, not
by reading the pattern.
**Two documented exports were unreachable.** `isAbsoluteUrl` is described as the
thing a caller composes its own `validate` from and `ConnectionSettingsApplied`
is what `onApply` receives; neither was exported from the barrel or the root.
Also: `docs/ui-usage.md` claimed `urls` is memoised while the code said
"deliberately not a memo". The code is right and the reason is good, so the doc
now says so, and the panel's own `resolvedUrls` is a real memo -- as a plain
function it was a full endpoint traversal per rendered row.
Not changed: `read()` accepting an unvalidated stored URL, and `setUrl`
bypassing validation. Both are deliberate -- the store is a container and the
panel is the gate, which the `validate` docstring states. Validating on load
would silently discard a working address someone saved.
Build passes and the fixes reach `ConnectionSettings.generated.tsx`. The 138
biome errors and 2 warnings are pre-existing: identical counts on the base.
`docs/ui-usage.md` already said event callbacks pass values, not events. Three
controls did not, so the rule was documentation rather than fact.
Switch onChange(Event) -> onChange(checked: boolean)
Checkbox onChange(Event) -> onChange(checked: boolean)
PasswordField onInput(value) -> onChange(value: string)
Slider, RadioGroup and CheckboxGroup were already `onChange(value)`, which is
what made the other three dangerous: one name meant two different things
depending on which field you reached for, so swapping a Switch for a Slider
silently changed what the handler received.
The native event is not gone. `Switch` and `Checkbox` take `onNativeChange`,
still ordered first, and `preventDefault()` there still vetoes the toggle -- and
now also suppresses `onChange`. That veto was load-bearing and a plain rename
would have destroyed it.
Found while doing it: a `Checkbox` inside a `CheckboxGroup` took an early return
after notifying the group, so a per-box handler never ran at all. It fires now;
the group still reports the whole selection separately.
`Select` refuses `value` together with `selectedKeys`, and `defaultValue`
together with `defaultSelectedKeys`. It preferred `selectedKeys` and ignored the
other, so a stale prop could win with nothing indicating which had been dropped.
There is no correct guess between two disagreeing sources.
`Card.state` was typed as the shared `State` -- default, loading, error,
invalid, disabled, hidden -- while the recipe implements info, success, warning,
danger. No member in common: every value the type allowed resolved to no class,
and every value the recipe implements was a type error. Now `CardState`. A card
is not a form control, which is why the shared vocabulary never fitted; the
reviewer's related point about ungated keyboard activation dissolves with it,
because a card has no disabled state to gate on.
Four more, each an accepted parameter that did nothing:
`Grid` computed its class string once, at initialisation, outside any tracked
scope, so changing `cols`, `gap` or `flow` afterwards did nothing and the only
way to see a new value was to remount. `Flex` does the same work in a memo and
is reactive. Now it matches.
`Input`'s label pointed at a different element than its input: the field takes
`<generated>-input` while the label fell back to the context id. The one
component whose job is associating a label with an input emitted a label
associated with nothing.
`Input` computed `isInvalid()` and passed `"disabled" | undefined`, so an input
marked invalid coloured its helper text while the field itself was never told.
`PasswordField` forwarded `isDisabled` / `isInvalid`, which are names on
`Input`'s *context*, not its props. The toggle button greyed out while the input
beside it stayed editable.
Docs: `layouts.md` told consumers to configure `pluginSolid()`, which the
library's own QA harness deliberately excludes -- solid-refresh's `$component`
wrapper calls `createSignal(component)`, and Solid 2 reads a function
initialiser as a derivation. The documented setup was one this repository had
already found not to work. It now mirrors the harness config.
Not addressed, deliberately: shared overlay ownership across Dialog, Drawer and
Popover; the recipe migration; and central spacing tokens. Each is a design
project rather than a fix.
Worth recording: `check:api` passes at 186 components across all of this,
because it compares parameter *names*. It cannot see that `onChange`'s payload
changed underneath it.
Build, typecheck and `check:api` pass. biome is unchanged against the base at
138 errors and 219 warnings.
Continues the audit. Everything here was verified by running it; two of the
findings turned out to be worse than reported and one rule was wrong rather
than the components.
**Overlays now have a single owner.** `lib/overlay.ts` holds one Escape stack
and one body-scroll counter for Dialog, Drawer and Popover.
Escape reached every layer, because each component bound its own `keydown` on
`document` and every listener that saw the key acted. A popover opened inside a
dialog closed both. The stack offers the key to the topmost *visible* overlay
only, and a visible overlay that refuses Escape still owns it, so nothing behind
it closes instead.
The scroll lock was worse than "restores the wrong value". Dialog and Drawer had
*separate* module-scope counters, each saving `body.style.overflow` on its first
lock. Dialog opens and saves ""; Drawer opens and saves "hidden", which is
Dialog's; Dialog closes and restores ""; Drawer closes and restores "hidden".
The page is left permanently unscrollable.
Registration is component-lifetime because all three bind once and gate inside
the handler, so entries carry `active()`. Without it a *closed* popover would
sit on top of the stack and swallow the dialog's Escape -- trading "closes
everything" for "closes nothing".
Verified: nested Escape closes one layer then the next; a closed overlay does
not swallow; a refusing overlay lets nothing behind it close; interleaved locks
stay locked and restore the original `overflow: auto`; stack and listeners drain
to zero.
**`check-contracts` was checking nothing.** It looked for `PascalCase.tsx`,
which has not existed since the layout migration, and skipped silently -- so it
printed "All 95 components pass" having source-checked 0 and skipped 93. It
verified that `index.ts` exists.
Fixing the lookup surfaced 38 failures across 22 components, and almost all of
them were the rules being stale:
- `omit()`/`twMerge()` demanded of components using `{...slot.root}`, where the
compiler does both.
- The same rule applied to components with no pass-through at all, where there
is nothing to separate.
- `style={{ "border-color": item.hex }}` called static; the dynamic-value regex
knew about calls and templates but not property access.
- `_shared` (helper modules) and `status` (a plain `.ts`) treated as components.
- `live-chat` and `table` hold several components each, which one-file-per-folder
could never find.
One real finding survived: `Slider` declares `JSX.InputHTMLAttributes` and
forwards none of it. `id` and `aria-label` were read by hand; `data-*`, `title`
and focus handlers were accepted by the type and dropped. Forwarded now -- and
the type is `HTMLAttributes<HTMLDivElement>`, because the component renders no
`<input>` at all: its semantic element is a div with `role="slider"`, so the
declared surface offered `min`, `max`, `step`, `checked` and `form` with
nowhere to go.
**`warningsAsErrors` is on.** The two unbaselined warnings in the repository
were both in `ConnectionSettings`, added on this branch, so they are fixed
rather than baselined: presentation moved into the recipe and the layout uses
`{...slot.*}`. That surfaced a class no rule matched (`--open`, deleted) and a
recipe prop that could never be driven, since `open` and `applying` are internal
state rather than props -- they are `data-open`/`data-applying` now, which is
the convention the docs already describe.
**One control-height scale.** `Button` and `Input` both took `size="sm"` and
disagreed: 2.25rem against 2rem, while `md` and `lg` already matched. A row of
the two was aligned at two sizes out of three. `--control-h-*` in the base
layer, not a theme, because height is not a theme decision.
**Docs that taught an API this library does not have.** The quick-start example
read `variant="primary" isPending={saving()}`: `variant` is
`solid | soft | outline | ghost | plain`, so primary is a *flavor*; pending is
`state="loading"`; the overlay triple is `open/defaultOpen/onOpenChange`, not
`isOpen`; and the same page said there is no polymorphic `as` while Flex, Grid
and Navbar ship one. The shortest documented path did not compile.
**A correction to the previous commit.** It said `check:api` passes with
`onChange`'s payload changed underneath it. That was measured against a stale
build: the check *does* catch names appearing and disappearing, and it caught
`+onChange +onNativeChange`. What it cannot see is a type change to an existing
name -- changing `Slider.onChange` from `(value: number)` to `(value: string)`
leaves it reporting a match across 186 components. Narrower gap than claimed,
and now demonstrated rather than asserted.
Not in this change, and not pretended otherwise: the recipe migration for the
remaining primitives, and the ps-qa composition fixtures. `warningsAsErrors`
makes the first safe to do incrementally -- no new component can add that debt,
and the baseline only shrinks.
Contracts 93/93, layouts clean, build passes, `check:api` matches at 186, biome
unchanged against the base at 138 errors and 219 warnings.
Both recipes declared empty slots while their layouts kept a `CLASSES` table and built the root's class with `twMerge`. The recipe system was decorative there: the slot names it declared did not even match what the component rendered -- Switch declared `label` and `switch`, and rendered neither. Switch now declares its seven real slots, plus `flavor` and `size` as call-site props with the defaults that used to sit in the component. There is no `disabled` axis, because it is derived from three sources -- `props.disabled`, `state === "disabled"`, the enclosing group -- so no prop the compiler can read by name produces it. `Switch.css` already selected on `.switch[data-disabled="true"]` beside the class the layout was adding, so the attribute path was live the whole time and `.switch--disabled` was a duplicate. The dead selectors go with it. `layouts.library.json` keeps `warningsAsErrors` off, with the reason written down: gating a build on `solid-layouts-lint` costs more than the debt it catches while the linter is not trustworthy, and the first thing a red build from an unreliable linter teaches is to ignore red builds.
The component sweep went from 74 of 74 passing to 0 of 74 the moment `qa-inspect-host` stopped enabling `system-fonts`, and every failure said the same thing: a box with no area. It was never a statement about a component. With no font catalogue every glyph shapes to nothing, so anything sized by its text lays out flat -- the harness heading is 1184x24 with fonts and 1184x0 without, and Dialog's trigger is 78x24 and 0x0. `Paints` asks for area, so it failed everywhere, for a reason that has nothing to do with what was being checked. `generate-checks.ts` now emits two sets from one description. `tests/ps-qa` is the library's real contract and runs where there are fonts -- macOS, and a contributor's machine. `tests/ps-qa-headless` is the same checks with every assertion about *paint* weakened to one about *layout*, which is what a Linux runner can be asked honestly: `Present` still fails when the node the document should have produced is missing, and when it exists but was never laid out. `QA_PROFILE` on `run-all.sh` picks one, and carries the matching target flag with it, because they are two halves of one decision. Three things are deliberately not weakened. `-renders` keeps `Paints`: the fixture's box comes from layout rather than text, it holds on a fontless host for all 74 components, and it is the only check that catches a component which mounted to nothing -- `Present` would pass for one. Geometry, `Measures` and the tree assertions never asked about paint. `Contrast` and `InteriorInk` are absent from the headless profile rather than weakened, because there is no tree-level equivalent of "a person can see this" and without fonts they mislead in opposite directions: `InteriorInk` fails on a trigger whose only ink is its label, and `Contrast` passes on everything, having no painted text to find. Two checks were wrong, and the sweep's own failure was hiding them: - `inline-edit-escape-keeps-value` named a committed value, twice over. First the one `-commits` writes, which is only there when the checks share a host; then the fixture's own, which is only there when they do not. Both are claims about what ran before rather than about Escape. What Escape promises is negative -- the abandoned draft is not what got committed -- and `Absent` says exactly that. - The `settings` and `inline-edit` commit checks retype a field the component pre-fills, which the runtime cannot do without fonts: `SetValue` clears a field by selecting it first, and parley's selection APIs all resolve through the laid-out text. Measured, same build both ways: InlineEdit commits "Renamed title" with fonts and "Original titleRenamed title" without. Those checks run in the full profile only, and the flag naming that limit says what has to change for it to go away. Two new checks, because two existing ones could not fail: - `-reports-input`: `-accepts-input` reads the control's own value, which the renderer updates whether or not the component told anyone. A field whose `onInput` never reaches its caller is exactly as broken as one that refuses keystrokes, and it passed. The fixture names the value it received. - `-reports-saving`: a save refused by validation, or one that throws inside `apply`, leaves the committed value where it was and reports nothing. The fixture names what the panel told its caller, so a silent refusal is a different failure from a save that did not run. It earned its place immediately -- it is what showed the harness was mistyping the address rather than the panel refusing a good one. Measured: 74 of 74 on both profiles.
The runner has no font catalogue and should not have one: a component library that needs a GUI stack installed before it can be tested is a component library nobody can test. `QA_PROFILE: headless` selects the checks written for that host, and `run-all.sh` carries the matching target flag with it. The freshness check covers both generated directories now, so a change to `components.ts` that is not regenerated fails here rather than being noticed a profile later.
They were the two parts Switch could not migrate. A non-root slot publishes
`data-slot="${component}-${slot}"`, and these two are not this component's
parts: eight components in the library render one or both, and five stylesheets
reach them through a descendant selector -- `.date-field
[data-slot="description"]`, `.menu-item [data-slot="label"]` -- which is how a
field styles whichever control is inside it. Qualifying them here would have
made Switch the one control those rules miss.
So the previous commit left `description` spreading the slot and then patching
the attribute back on afterwards, and `label` hand-written entirely, which the
linter reported as a rendered slot the recipe never declared. Both were the
recipe not owning presentation, spelled two different ways.
solid-layouts 0.2.3 lets a slot name what it publishes, so both are declared
now and neither writes a `data-slot` at the JSX site.
Removing those two warnings made the lint baseline stale, which fails the
generator by design -- that is the ratchet working, so a pattern that was
removed cannot come back unnoticed. Regenerated with
`solid-layouts-lint --update-baseline`, per docs/linting-and-porting.md in
solid-layouts. The baseline loses the two Switch warnings and the
`slot-undeclared` error along with them.
84c1a62 to
6481da7
Compare
**`apply()` persisted after it reconnected.** It snapshotted the committed state and called `onApply` straight away; persistence happened in the deferred effect that mirrors `state` into storage, a microtask later. The callback got the right settings and storage still held the old ones, so a callback that reloads or navigates -- which is what a transport change often does -- read back what it was replacing. It writes before the callback now. Deliberately not behind `queue.then(...)`, which would have been simpler and wrong in a way that is hard to see: it puts the body a microtask later, which is also when the effect runs, so the ordering would hold by luck. Measured -- a check written against the queued version passed with the write deleted. **Overlapping applies had no order.** Counting them said when the last one finished, not which one finished last: two saves in quick succession could reconnect in either order, and the losing order leaves storage saying B while the transport is on A, with nothing on screen disagreeing. They are queued now, so the last save is the last thing the transport hears. `onApply` is somebody else's transport, so cancelling a superseded one is not this store's call; not starting it out of turn is. **Store-level validation was bypassable, twice.** `setUrl` accepted anything and persisted it as an active override, and loading accepted any nonempty stored string the same way -- while `docs/ui-usage.md` promises validation before saving. `setUrl` returns the reason it refused and stores nothing; a stored address that does not parse is dropped with its override, so the endpoint falls back to something that works. Invalid values stay drafts in the UI, which is where a value being typed belongs. **The default validator accepted two things that are not addresses.** `wss://host:99999` and `https://[1:2:3]` both matched: a run of digits is not a port and a run of hex and colons is not an IPv6 literal. The host and port are captured and checked as values now -- 16-bit port, and the shape the URL Standard describes for `::`. 16 cases, all correct. **Overlays ranked by mount, not by open.** The shared stack fixed Escape closing every layer, but components registered at setup, so with two popovers mounted, opening the second and then the first sent Escape to the second. An `active()` predicate could skip the closed ones and still ranked the open ones by mount. Registration moves to the effect that runs when an overlay becomes visible, which is the only way the stack can hold open order. Focus trapping joins it. Dialog and Drawer each bound their own document-level Tab listener gated on "am I visible", which is not the same question as "am I the overlay in front": with both open both traps ran and fought over focus. The manager asks the innermost overlay only. `trapFocus` and friends move to `src/lib/focus.ts`, replacing Dialog's and Drawer's identical copies. **Radio still handed `onChange` an `Event`** while Switch and Checkbox reported a value -- the exact inconsistency 3.1 exists to remove. It reports `checked` now, with the native event on `onNativeChange`. **`urls` rebuilt the record on every read.** It is resolved once per state object and frozen: `state` is replaced wholesale on every write, so the object itself is the cache key. Still not a memo, for the reason the old comment gave. **`warningsAsErrors` is on.** It was turned off on the claim that the linter failed spuriously. It does not: what was seen was the baseline ratchet doing its job after debt was removed, which `docs/linting-and-porting.md` describes and `--update-baseline` resolves. Verified to have teeth -- a manual class composition added to a migrated component fails `layouts:generate`. Verification: the QA fixture supplies an `onApply` and names both what it was handed and what storage held when it ran, so `-reconnects-with-what-it-saved` covers the argument and the ordering. It uses an address of its own, because the checks in a group share a host and reusing `commitText` made it pass with the write deleted. The harness also installs an in-memory `localStorage` when the host has none, which `qa-inspect-host` does not: without it every component that persists anything took its storage-unavailable branch and the whole path went untested.
ee1298d to
0ffb810
Compare
The two profiles differed in two ways, and only one of them was a fact about the host. `Contrast` and `InteriorInk` read pixels and stay absent from the headless profile, because there is no tree-level equivalent of "a person can see this". The other was the checks that retype a field the component pre-fills -- a settings panel's "saving commits what was typed", InlineEdit's "Enter commits the edited value". Those were confined to the fontful profile, on the finding that a fontless host appends instead of replacing. That finding was right; the conclusion was not. `qa-inspect-host` cleared the field by selecting all of it first, and every selection API parley exposes resolves through the laid-out text -- so with no glyphs the selection came back collapsed. Clearing by byte count instead has no such dependency. Fixed in qa-inspect-host 0.1.12, and the checks run everywhere now. They are the strongest checks in the suite, and they were the ones Linux could not run. 74 of 74 on both profiles, with the same check set.
A prop's name is not the promise. `onChange` went from handing over an `Event` to handing over a `boolean` on Switch, Checkbox and Radio in this release, and the contract stayed green through all three: the name never moved. A consumer's handler keeps compiling and starts receiving something else, which is the worst shape a breaking change can take. Each entry is `name?: type` now. The type text is scanned rather than matched, because the terminator depends on nesting -- the comma in `Record<string, number>` ends nothing, and neither does the one in `(value: string, index: number) => void` -- and whitespace is collapsed so rewrapping a type across lines does not move the record. `Omit<…>` still subtracts on the name half. The document is a fenced block per component, one prop per line, so the diff of one prop is one line rather than a whole list rewrapping. 168 components gained type text; nothing gained or lost a prop. Verified to catch what it exists for: changing `Slider.onChange` from `(value: number) => void` to `(value: string) => void` now fails in both directions -- undocumented prop, and a broken promise -- where before it was invisible. `tests/api-contract.test.ts` asks whether the intersection walk *found* a prop, which is a different question, so those cases match on the name half. And `PasswordField.test.tsx` asserted `isDisabled` / `isInvalid`, the names on `Input`'s context rather than its props: the contract was corrected when that bug was fixed and the test was not, so the one test that could have caught it was asserting the bug. 180 pass, 0 fail.
`--control-h-*` gave Button and Input the same height at the same *named* size and left them defaulting to different names: an unsized Button is 2.25rem and an unsized Input was 2.5rem. Nothing in this library passes a size to either -- 26 Inputs and 17 Buttons across the composites, none of them sized -- so every row of them was 4px out, including the connection panel and the auth forms. Input defaults to `sm` now. That direction rather than the other because Button's default is measured: the fleet passes `sm` at 349 of 465 sites and `md` at 25, so moving Button would resize 349 call sites to fix a 4px seam. The gaps had the same shape one axis over. `Flex` names `sm`/`md`/`lg` as `gap-2`/`gap-4`/`gap-6`; `AuthFieldGroup` wrote its own rem values, agreeing at `md` and `lg` and disagreeing at `sm` -- 0.75rem against 0.5rem. `--gap-*` names the three, with Tailwind's values, because Flex's utilities are not going to stop being Tailwind's. Both are asserted rather than just fixed. `button-measures` and `input-measures` pin the default height at 36px, so the two cannot drift apart again without a check going red. Height only: the width is the label. Input's subject is `@input-control` rather than the textbox, because the inner element is 17px of text and the box a person sees is the wrapper -- which is worth knowing on its own. 74 of 74 on both profiles; 180 unit tests pass.
`cargo install ps-qa` took whatever was newest, and the sweep needs two things that are not in every version: the flag that lets it target a control on a host with no font catalogue, and a host that can replace a text field on one. Without the pin an older ps-qa fails as `unexpected argument`, which reads as a broken workflow rather than a tool that is too old. The host is worse: an old one silently appends to a pre-filled field instead of replacing it, so nothing fails until a check disagrees about a value and the component gets the blame. A floor rather than an exact version: a newer tool is expected to work, and pinning exactly would mean a commit here for every tool release.
…trailing colon **A synchronous `onApply` that threw left the panel disabled for good.** `Promise.resolve(reconnect())` calls before entering the `try`, so a throw escaped the expression: `inFlight` never decremented, `isApplying` never cleared, and `queue` was never assigned either. `onApply` is documented as allowed to be synchronous, so this is a supported shape, and the failure is permanent rather than for one save. An `async` wrapper with no `await` before the call runs the body synchronously, so the ordering that branch exists for is unchanged -- `write` still happens before the deferred persistence effect, and the check for that still fails if the write is removed -- while a synchronous throw becomes a rejection like any other. Covered: sync throw, async rejection, and a successful retry after both, each leaving `isApplying` false. **A Popover inside a Dialog switched the Dialog's focus containment off.** Tab went to the top overlay only; a popover is not modal and declared no scope, so Tab did nothing at all and focus walked out to the page behind an open dialog. Worse than before the shared manager, where the Dialog's own listener still ran. Trapping in the dialog's element alone is the opposite failure: the popover's content is portalled outside it and Tab could never reach it. The scope is the innermost *modal* plus everything opened above it. A popover names its content without claiming to contain focus, so it joins the scope its opener owns. `trapFocus` takes a list, and pulls focus back when something outside the scope has taken it -- the rules before it only fired at the ends, which assumed focus was already inside. `focusScope` is a pure function and exported, because the bugs here are arrangements rather than components: a sweep that mounts one component per page cannot open two things at once, and this repository has no DOM in its unit tests by design. Six cases, four of which fail against the old behaviour. **`https://[1::2:]` validated.** The trailing colon was stripped unconditionally, so the tail `2:` became `2` and parsed as a group. A separator with nothing after it is a parse failure under the URL Standard. The removal is tied to the thing that justifies it now: the `:` before a dotted-quad suffix. 22 cases, all correct. 74 of 74 on both profiles; 186 unit tests pass.
The switch wrote the override flags the moment it was flipped, and the store persists on every change, so opening the panel and navigating away left the application overridden while the URL fields beside it were still unsaved drafts. The one control that was not a draft was the one deciding whether any of the others counted. It is local state now, and Save commits it with the rest. Save also derives the flags per endpoint rather than calling `setUseCustom`. That helper flips every endpoint the store was configured with, including any the panel does not render, and marks an endpoint overridden even when its value is the fallback -- so "point the API somewhere else, leave auth on production", the case the per-endpoint model exists for, could not be expressed, and an endpoint pinned to a value that merely equalled today's default stayed pinned when that default moved. Addresses are checked before they are saved, by `validate` on the endpoint. Unchecked, a bad address is persisted, fails to connect, and is still there on the next launch with nothing on screen explaining it. The default check is a regex and deliberately not `new URL()`: the native renderer this library is tested against has no `URL` global, so a validator built on it throws, is caught, and refuses every address -- while passing every test written in a browser. It cost a red `-commits` to find that. `onApply` now receives the whole applied state. A site storing an app id here reconfigures its identity from the same call instead of reaching back into the store it had just handed over. `style` and `dataTheme` reach the root, which `UIBaseProps` promises and this component dropped. `urls` is resolved once for the list rather than per row. Reported in review of #286.
Six sites ship a "point this app at a different backend" page and each wrote its own.
Three different file layouts, and 58 of honey.id's 164 store lines are identical to pays.online's.
@pathscale/uiexports nothing for this today, which is why it was written six times.They have drifted in behaviour, not just in shape. pays.online's toggle is broken: clicking it flips the checkbox — the accessibility tree reports
switch "on"— and the URL fields never appear, because the page builds its body with a plainrenderForm()call and gates on<Show when={data().useCustomUrl}>outside any tracked scope. honey.id renders<Fields />as a real component withuseFieldand a local signal, and works. Same flow, one copy correct.What this adds
createConnectionSettingsinsrc/hooks/connection, exported from the root.The storage model is per-endpoint, not global. Four sites had one
useCustomUrlcovering every backend at once. support.cafe and 24x.ai had already grown a flag per backend, because pointing the API at a local instance while leaving auth on production is the ordinary case. Per-endpoint covers both shapes, andsetUseCustomflips them together for a page that only wants one switch.Three behaviours are kept from whichever copy had them, because each was learned the hard way:
isApplyingcovers the awaitedonApply, not just the write, so a page can disable its form for the whole reconnect.Tests
Seven, run under
--conditions=browseras the package script does — the server build of Solid never propagates a signal, so every assertion would otherwise pass while testing nothing.The
isApplyingtest holds the callback open deliberately. A callback that returns inside the same batch flips the flag back before anything can observe it; that is true of the store and not worth asserting either way. Writing it the naive way is how I first "proved" a bug that wasn't there.Not in this PR
The panel component itself. This is the half that is genuinely identical across the six — persistence, resolution, apply and reset. The presentation still varies (labels, i18n, layout, whether it lives behind a route or in a settings tab), and folding it in should follow the
auth-*component family rather than be improvised here.Once the panel lands, the six sites delete their own copies and the pays.online toggle bug goes with them.