ReviewOS
Reload

also looking at this

loghqorg/loghq

Promote main to production: signed-out chrome and the subscriber tables

#17
Merged glennmichael123 wants to merge main into production
54 files +4252 -1353

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

resources/views/index.stxmodified+1-1
@@ -203,7 +203,7 @@ async function subscribe() {
203203 required
204204 x-model="email"
205205 >
206 <button class="btn primary" type="submit" :disabled="subscribeMutation.loading()" :text="subscribeMutation.loading() ? 'Sending' : 'Keep me posted'">Keep me posted</button>
206 <button class="btn primary" type="submit" :disabled="subscribeMutation.loading()" :aria-busy="subscribeMutation.loading()" :text="subscribeMutation.loading() ? 'Sending' : 'Keep me posted'">Keep me posted</button>
207207 </form>
208208 <p class="sub-status" role="status" aria-live="polite" :data-state="failed() ? 'error' : ''" :text="status()"></p>
209209 </div>
resources/views/login.stxmodified+2-2
@@ -163,7 +163,7 @@ async function onVerify(e: Event): Promise<void> {
163163 <span>Keep me signed in for 30 days</span>
164164 </label>
165165
166 <Button type="submit" size="sm" fullWidth :disabled="loading()" :label="loading() ? 'Signing in…' : 'Sign in'" className="auth-submit">Sign in</Button>
166 <Button type="submit" size="sm" fullWidth :loading="loading()" :label="loading() ? 'Signing in…' : 'Sign in'" className="auth-submit">Sign in</Button>
167167 </form>
168168
169169 {{-- Step 2: TOTP, shown only when the account has 2FA enabled. --}}
@@ -173,7 +173,7 @@ async function onVerify(e: Event): Promise<void> {
173173 <label for="login-code" class="block mb-1.5 font-medium text-muted text-xs">Authentication code</label>
174174 <input id="login-code" name="code" type="text" inputmode="numeric" autocomplete="one-time-code" maxlength="6" placeholder="123456" :model="code" class="px-3.5 py-2.5 w-full text-sm field" :aria-invalid="error() ? 'true' : 'false'" :aria-describedby="error() ? 'login-error' : ''">
175175 </div>
176 <Button type="submit" size="sm" fullWidth :disabled="loading()" :label="loading() ? 'Verifying…' : 'Verify and sign in'" className="auth-submit">Verify and sign in</Button>
176 <Button type="submit" size="sm" fullWidth :loading="loading()" :label="loading() ? 'Verifying…' : 'Verify and sign in'" className="auth-submit">Verify and sign in</Button>
177177 </form>
178178
179179 @if (oauthError)
resources/views/pricing.stxmodified+1-1
@@ -125,7 +125,7 @@ async function upgrade(): Promise<void> {
125125 <li class="flex gap-2 items-center"><span class="text-accent">✓</span> Full retention</li>
126126 <li class="flex gap-2 items-center"><span class="text-accent">✓</span> Priority support</li>
127127 </ul>
128 <button @click="upgrade" :disabled="checkout.loading()" class="mt-7 py-2.5 w-full font-semibold text-accent-ink text-sm bg-accent rounded-[10px] disabled:opacity-60 active:translate-y-px disabled:cursor-default [transition:transform_0.12s_ease,opacity_0.15s_ease]" :text="checkout.loading() ? 'Redirecting…' : 'Upgrade to Pro'">Upgrade to Pro</button>
128 <button @click="upgrade" :disabled="checkout.loading()" :aria-busy="checkout.loading()" class="mt-7 py-2.5 w-full font-semibold text-accent-ink text-sm bg-accent rounded-[10px] disabled:opacity-60 active:translate-y-px disabled:cursor-default [transition:transform_0.12s_ease,opacity_0.15s_ease]" :text="checkout.loading() ? 'Redirecting…' : 'Upgrade to Pro'">Upgrade to Pro</button>
129129 @if (err())
130130 <p class="mt-3 text-[#ef4444] text-center text-xs">{{ err() }}</p>
131131 @endif
resources/views/register.stxmodified+1-1
@@ -112,7 +112,7 @@ async function onRegister(e: Event): Promise<void> {
112112 <input id="register-password" name="password" type="password" required autocomplete="new-password" placeholder="At least 8 characters" :model="password" class="px-3.5 py-2.5 w-full text-sm field" :aria-invalid="error() ? 'true' : 'false'" :aria-describedby="error() ? 'form-error' : ''">
113113 </div>
114114
115 <Button type="submit" size="sm" fullWidth :disabled="loading()" :label="loading() ? 'Creating account…' : 'Create account'" className="auth-submit">Create account</Button>
115 <Button type="submit" size="sm" fullWidth :loading="loading()" :label="loading() ? 'Creating account…' : 'Create account'" className="auth-submit">Create account</Button>
116116 </form>
117117
118118 @if (anyOauth)
resources/views/settings.stxmodified+22-5
@@ -722,6 +722,14 @@ const repo = state<any>(repository || null)
722722const repoRef = state('')
723723const repoToken = state('')
724724const repoBusy = state(false)
725/*
726 * Which repository button is working. `repoBusy` gates all three, which is right
727 * for :disabled -- none of them should be clickable while any runs -- and wrong
728 * for aria-busy, which names ONE element as updating. Verify and Disconnect sit
729 * in the same `:show="!!repo()"` block, so with the shared signal on both,
730 * verifying would announce Disconnect as busy too.
731 */
732const repoAction = state('')
725733const repoNote = state('')
726734const repoNoteOk = state(true)
727735
@@ -733,10 +741,12 @@ async function connectRepo(): Promise<void> {
733741 return
734742 }
735743 repoBusy.set(true)
744 repoAction.set('connect')
736745 try {
737746 const res = await apiFetch(`${projUrl}/repository`, { method: 'POST', body: JSON.stringify({ repository: ref, token }) })
738747 const body = await readJson(res)
739748 repoBusy.set(false)
749 repoAction.set('')
740750 if (!res.ok) {
741751 note(repoNote, repoNoteOk, body.error || 'Could not connect.', false)
742752 return
@@ -749,16 +759,19 @@ async function connectRepo(): Promise<void> {
749759 }
750760 catch {
751761 repoBusy.set(false)
762 repoAction.set('')
752763 note(repoNote, repoNoteOk, 'Network error. Try again.', false)
753764 }
754765}
755766
756767async function verifyRepo(): Promise<void> {
757768 repoBusy.set(true)
769 repoAction.set('verify')
758770 try {
759771 const res = await apiFetch(`${projUrl}/repository/verify`, { method: 'POST' })
760772 const body = await readJson(res)
761773 repoBusy.set(false)
774 repoAction.set('')
762775 if (!res.ok) {
763776 note(repoNote, repoNoteOk, body.error || 'Verification failed.', false)
764777 return
@@ -768,6 +781,7 @@ async function verifyRepo(): Promise<void> {
768781 }
769782 catch {
770783 repoBusy.set(false)
784 repoAction.set('')
771785 note(repoNote, repoNoteOk, 'Network error. Try again.', false)
772786 }
773787}
@@ -779,9 +793,11 @@ async function disconnectRepo(): Promise<void> {
779793 if (!(await Promise.resolve(ask)))
780794 return
781795 repoBusy.set(true)
796 repoAction.set('disconnect')
782797 try {
783798 const res = await apiFetch(`${projUrl}/repository`, { method: 'DELETE' })
784799 repoBusy.set(false)
800 repoAction.set('')
785801 if (!res.ok) {
786802 const body = await readJson(res)
787803 note(repoNote, repoNoteOk, body.error || 'Could not disconnect.', false)
@@ -792,6 +808,7 @@ async function disconnectRepo(): Promise<void> {
792808 }
793809 catch {
794810 repoBusy.set(false)
811 repoAction.set('')
795812 note(repoNote, repoNoteOk, 'Network error. Try again.', false)
796813 }
797814}
@@ -1460,8 +1477,8 @@ onMount(() => {
14601477 <div :show="!!repo()">
14611478 <div class="flex flex-wrap gap-2 items-center mb-3.5">
14621479 <code class="flex-1 px-3.5 py-2.5 min-w-[220px] font-mono text-ink text-sm truncate bg-canvas border border-line rounded-[10px]" :text="repo()?.full_name">.</code>
1463 <button @click="verifyRepo" :disabled="repoBusy()" class="btn-quiet btn-sm">Verify</button>
1464 <button @click="disconnectRepo" :disabled="repoBusy()" class="btn-danger btn-sm">Disconnect</button>
1480 <button @click="verifyRepo" :disabled="repoBusy()" :aria-busy="repoAction() === 'verify'" class="btn-quiet btn-sm">Verify</button>
1481 <button @click="disconnectRepo" :disabled="repoBusy()" :aria-busy="repoAction() === 'disconnect'" class="btn-danger btn-sm">Disconnect</button>
14651482 </div>
14661483 <dl class="grid gap-y-1.5 grid-cols-2 sm:grid-cols-3 text-xs">
14671484 <div><dt class="text-subtle">Default branch</dt><dd class="font-mono text-muted" :text="repo()?.default_branch || 'unknown'">.</dd></div>
@@ -1478,7 +1495,7 @@ onMount(() => {
14781495 <label class="block mb-1.5 font-medium text-muted text-xs" for="repo-token">GitHub token</label>
14791496 <input id="repo-token" :model="repoToken" type="password" autocomplete="off" placeholder="github_pat_..." class="mb-3 px-3.5 py-2.5 w-full font-mono text-ink text-sm bg-canvas border border-line rounded-[10px] focus:border-[color-mix(in_srgb,var(--accent)_55%,var(--border))] focus:outline-none">
14801497
1481 <button @click="connectRepo" :disabled="repoBusy()" class="btn-cta btn-solid">Connect repository</button>
1498 <button @click="connectRepo" :disabled="repoBusy()" :aria-busy="repoAction() === 'connect'" class="btn-cta btn-solid">Connect repository</button>
14821499
14831500 <p class="mt-3.5 leading-relaxed text-subtle text-xs">
14841501 Use a <span class="text-muted">fine-grained</span> personal access token scoped to this one
@@ -1538,7 +1555,7 @@ onMount(() => {
15381555 when it lands.
15391556 tests/unit/button-vocabulary.test.ts fails if a <Button> here
15401557 ever gets an x-tooltip. --}}
1541 <button @click="removeMember('')" :disabled="removingMember() === ''" :text="memberBtnLabel('')" class="btn-danger btn-xs" x-tooltip="Remove">Remove</button>
1558 <button @click="removeMember('')" :disabled="removingMember() === ''" :text="memberBtnLabel('')" :aria-busy="removingMember() === ''" class="btn-danger btn-xs" x-tooltip="Remove">Remove</button>
15421559 </div>
15431560 @endforeach
15441561 @foreach (inviteView as i)
@@ -1550,7 +1567,7 @@ onMount(() => {
15501567 </div>
15511568 <span class="px-[9px] py-0.5 font-semibold text-[11px] text-muted tracking-[0.02em] bg-[color-mix(in_srgb,var(--text-3)_18%,var(--panel))] rounded-[999px]" x-tooltip="Pending: becomes a member once they accept">Invited</span>
15521569 <button @click="copyJoin('', '')" :text="joinBtnLabel('')" class="btn-quiet btn-xs" x-tooltip="Copy join link">Copy link</button>
1553 <button @click="removeMember('')" :disabled="removingMember() === ''" :text="memberBtnLabel('')" class="btn-danger btn-xs" x-tooltip="Revoke invite">Remove</button>
1570 <button @click="removeMember('')" :disabled="removingMember() === ''" :text="memberBtnLabel('')" :aria-busy="removingMember() === ''" class="btn-danger btn-xs" x-tooltip="Revoke invite">Remove</button>
15541571 </div>
15551572 @endforeach
15561573 </div>
tests/unit/busy-state.test.tsadded+129-0
@@ -0,0 +1,129 @@
1/**
2 * A control that is working has to SAY it is working.
3 *
4 * Every async button here gates itself on a busy signal through `:disabled`,
5 * which stops a second click and greys the control. For a screen reader that is
6 * not the same thing: `disabled` removes the button from the focus order, so
7 * someone who just activated it loses their place and hears nothing about why.
8 *
9 * Two vehicles, and which one is right depends on the element:
10 *
11 * - A plain `<button>` takes `:aria-busy`. An ATTRIBUTE binding, deliberately,
12 * because a reactive mustache in a button's own text makes stx cloak the
13 * element until hydration. stx special-cases `aria-*` in dist/signals.js
14 * (`attrName.startsWith('aria-') && typeof v === 'boolean'` writes `String(v)`)
15 * so a boolean emits `aria-busy="true"` rather than the bare `aria-busy=""`
16 * the ordinary boolean path writes, and an empty value reads as FALSE.
17 * - A `<Button>` takes `:loading`. The component writes `aria-busy` and shows a
18 * spinner from that prop and computes `isDisabled = disabled || loading`, so
19 * passing `:aria-busy` from here would fight its own binding.
20 *
21 * The pairing with the signal matters as much as the attribute. `aria-busy`
22 * means "this element is being updated", so one signal cannot drive two buttons
23 * that are on screen together: verifying a repository would otherwise announce
24 * Disconnect as busy too. That is what `repoAction` is for.
25 */
26import { describe, expect, test } from 'bun:test'
27import { readdirSync, readFileSync, statSync } from 'node:fs'
28import { join } from 'node:path'
29
30const ROOT = join(import.meta.dir, '../..')
31const BUSY = /busy|loading|saving|sending|submitting|removing|deleting/i
32
33function views(dir: string): string[] {
34 const out: string[] = []
35 for (const entry of readdirSync(dir, { withFileTypes: true })) {
36 const path = join(dir, entry.name)
37 let directory = entry.isDirectory()
38 if (entry.isSymbolicLink()) {
39 try { directory = statSync(path).isDirectory() }
40 catch { continue }
41 }
42 if (directory) out.push(...views(path))
43 else if (entry.name.endsWith('.stx')) out.push(path)
44 }
45 return out
46}
47
48/**
49 * Comments, all three kinds, because prose about a shape matches a grep for it.
50 * The block above `repoAction` says "repoBusy gates all three", and that
51 * sentence is exactly what the mutation-count assertion would miscount.
52 */
53function code(source: string): string {
54 return source
55 .replace(/\{\{--[\s\S]*?--\}\}/g, '')
56 .replace(/\/\*[\s\S]*?\*\//g, '')
57 .replace(/(^|[^:])\/\/[^\n]*/g, '$1')
58}
59
60/** Open tags only, with quoted values kept whole so a `>` inside one cannot end the tag. */
61const OPEN_TAG = /<(button|Button)\b((?:[^>"]|"[^"]*")*)>/g
62
63function asyncButtons(source: string): { tag: string, attrs: string }[] {
64 const found: { tag: string, attrs: string }[] = []
65 for (const match of code(source).matchAll(OPEN_TAG)) {
66 const attrs = match[2]
67 const disabled = /:disabled="([^"]*)"/.exec(attrs)
68 const gated = (disabled && BUSY.test(disabled[1])) || /:loading="/.test(attrs)
69 if (gated)
70 found.push({ tag: match[1], attrs })
71 }
72 return found
73}
74
75const files = views(join(ROOT, 'resources'))
76const settings = () => readFileSync(join(ROOT, 'resources/views/settings.stx'), 'utf8')
77
78describe('async buttons announce that they are working', () => {
79 test('every button gated on a busy signal announces it', () => {
80 const missing: string[] = []
81 for (const file of files) {
82 for (const button of asyncButtons(readFileSync(file, 'utf8'))) {
83 const announced = button.tag === 'Button'
84 ? /:loading="/.test(button.attrs)
85 : /:aria-busy="/.test(button.attrs)
86 if (!announced)
87 missing.push(`${file.slice(ROOT.length + 1)}: <${button.tag}${button.attrs.trimEnd().slice(0, 80)}>`)
88 }
89 }
90 expect(missing).toEqual([])
91 })
92
93 test('the count does not quietly fall to zero', () => {
94 const total = files.reduce((n, f) => n + asyncButtons(readFileSync(f, 'utf8')).length, 0)
95 // 16 at the time of writing. A regex that stops matching would pass the test
96 // above by finding nothing at all, which is the failure mode of every gate in
97 // this repo that greps for a shape.
98 expect(total).toBeGreaterThanOrEqual(16)
99 })
100
101 test('the three repository buttons do not share one aria-busy source', () => {
102 const found = asyncButtons(settings()).filter(b => /Repo"/.test(b.attrs) || /Repo'/.test(b.attrs) || /"(verify|disconnect|connect)Repo"/.test(b.attrs))
103 expect(found).toHaveLength(3)
104 const sources = found.map(b => /:aria-busy="([^"]*)"/.exec(b.attrs)?.[1])
105 expect(new Set(sources).size).toBe(3)
106 for (const source of sources)
107 expect(source).toContain('repoAction()')
108 })
109
110 test('repoAction is cleared wherever repoBusy is', () => {
111 // A discriminator left set after the work finishes announces a button as busy
112 // forever, which is worse than not announcing it at all.
113 const body = code(settings())
114 expect((body.match(/repoBusy\.set\(/g) ?? []).length)
115 .toBe((body.match(/repoAction\.set\(/g) ?? []).length)
116 })
117
118 test('the per-member buttons scope their busy state to one row', () => {
119 // removingMember() holds an id, so each row already answers for itself and
120 // needs no discriminator; the binding just has to use the same expression.
121 const found = asyncButtons(settings()).filter(b => /removingMember\(\)/.test(b.attrs))
122 expect(found).toHaveLength(2)
123 for (const button of found) {
124 const disabled = /:disabled="([^"]*)"/.exec(button.attrs)?.[1]
125 const busy = /:aria-busy="([^"]*)"/.exec(button.attrs)?.[1]
126 expect(busy).toBe(disabled)
127 }
128 })
129})
tests/unit/shared-controls.test.tsmodified+7-1
@@ -41,7 +41,13 @@ describe('shared auth submit controls', () => {
4141 // A label binding belongs to Button, not :text on its root, which
4242 // would destroy its accessible slot and internal state elements.
4343 expect(control).toContain(':label=')
44 expect(control).toContain(':disabled="loading()"')
44 // `:loading`, not `:disabled`. Both gate the control -- Button computes
45 // `isDisabled = disabled || loading` -- but only `loading` also writes
46 // `aria-busy` and shows the spinner, and a submit that is merely disabled
47 // is dropped from the focus order while saying nothing about why. Passing
48 // `:aria-busy` here instead would fight the binding Button writes itself.
49 expect(control).toContain(':loading="loading()"')
50 expect(control).not.toContain(':disabled=')
4551 expect(control).not.toContain(':text=')
4652 }
4753 })