From 31c7b6cf35b44dac7e62db7f85ed59c172f207a2 Mon Sep 17 00:00:00 2001
From: Takashi Matsuyama <hello@takashimatsuyama.com>
Date: Mon, 10 Aug 2026 19:37:28 +0900
Subject: [PATCH] Make the row-access grant look like something you can press

Two people have now failed to find it, and both fixes before this one changed
the wording. The settled scope for this control's discoverability is wording and
presentation, and presentation had never been touched: measured on the built
app, `enable browsing` was a 92x14px run of blue text with `cursor: pointer` and
nothing else - no border, no background, no padding - sitting beside a status
badge that did have a border. The row therefore showed one bordered thing, which
is the state, and some coloured words, which are the only way to grant anything.

It now draws a box: 112x21px with a 1px blue border, a filled background and room
around the label, against the 92x14 it replaced. The label itself is unchanged at
12px - an earlier draft of this set `font-size: 0.72rem` "for the button", which
the button was already inheriting, so the only thing it did was shrink the words
two people could not find by 4%.

It is also a real <button> where one is legal, which is the surface that matters.
The overview card cannot hold one: the card is itself a <button> that selects the
profile, and a <button> inside a <button> is invalid HTML - the accessibility tree
of a nested interactive element is undefined and readers disagree about what to
announce. It is NOT a rendering failure, which an earlier version of the comment
claimed: Svelte builds the card and this block as separate templates and joins
them by DOM insertion, so the parser repair that would close the card's <button>
early never runs. The profile bar has no interactive host, and it already proved
the point, since `delete` up there has been a real button since the component
existed.

The flag is `asButton`, off by default, so the answer a host gets by forgetting
is the one that renders correctly anywhere. It was first written as
`insideButton = false`, which reads as a safe default and was the opposite of one:
that spelling handed a real <button> to any host that said nothing, which is the
single case the flag exists to prevent.

The three controls are one derived list now rather than three written-out
branches, so the two element kinds cannot drift into offering different grants.

The e2e asserts the box and the colours, not "has some border and some
background": a bare <button> with this app's rule deleted still carries
Chromium's own 2px border, grey fill and padding, so three inequalities on those
pass while the control is back to a run of text. Broken four ways while writing
it - the bar falling back to a span, the fill and padding deleted, the border
made transparent, and the card forgetting its host - each failed a different
assertion. Computed style is read through `getPropertyValue`, because a
hand-written record of camelCase fields makes the type authoritative over the
real API and a renamed field then reads `undefined`, which passes an inequality
forever.

Recorded and not fixed: on the card, Space reaches the card's own <button> and
selects the profile instead of granting, and Enter fires both. Pre-existing - the
bar's spans did the same before this control had a box - and left as it is, but
it is the sharper reason the bar takes a real button.

Not touched: the MCP controls and the card's own refresh/delete are still linkish
text, so the bar now shows a bordered button on one row and coloured words on the
next. Only the row-access control has a report behind it, and widening this to
every clickable word in the app is a separate decision.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 e2e/app.spec.ts                        |  65 +++++++++++
 src/renderer/src/OverviewCards.svelte  |   3 +
 src/renderer/src/ProfileBar.svelte     |   4 +
 src/renderer/src/RowAccessBlock.svelte | 150 ++++++++++++++++++-------
 4 files changed, 183 insertions(+), 39 deletions(-)

diff --git a/e2e/app.spec.ts b/e2e/app.spec.ts
index 6dcf589..8900083 100644
--- a/e2e/app.spec.ts
+++ b/e2e/app.spec.ts
@@ -26,6 +26,15 @@ function toolText(payload: unknown): string {
 
 test.skip(!url, 'KOZOU_TEST_DATABASE_URL not set');
 
+/** Read through `getPropertyValue` rather than a camelCase field, and typed as
+ *  the one method that is called. Both because this suite typechecks under the
+ *  node tsconfig, which has no DOM globals - and because a hand-written record of
+ *  camelCase fields makes the type authoritative over the real API: a misspelled
+ *  or renamed field then reads `undefined`, and `undefined !== '0px'` is true, so
+ *  the assertion passes forever. `getPropertyValue` answers '' for a property it
+ *  does not know, which fails these comparisons instead of passing them. */
+type StyleReader = { getPropertyValue: (property: string) => string };
+
 test('two profiles: map, detail pane, and AI view end-to-end', async () => {
   const userData = mkdtempSync(join(tmpdir(), 'kozou-desktop-e2e-'));
   const app = await electron.launch({
@@ -182,12 +191,68 @@ test('two profiles: map, detail pane, and AI view end-to-end', async () => {
   await expect(page.getByTestId('overview-cards')).toHaveCount(0);
   await expect(page.getByTestId('rowaccess-badge-beta')).toHaveCount(1);
 
+  // The way to grant row access is a control, and on this surface it is a real
+  // one. Two attempts at the discoverability of this thing changed only the
+  // wording; it was a bare run of blue words beside a bordered status badge, with
+  // no border, background or padding of its own. Asserted as the box it now draws
+  // rather than as a class name, because a class says nothing about what is on
+  // screen.
+  const grant = page.getByTestId('rowaccess-enable-beta');
+  const box = async (loc: typeof grant): Promise<{ tag: string; h: number; w: number; border: string; bg: string }> =>
+    loc.evaluate((el) => {
+      const s = (globalThis as unknown as { getComputedStyle: (e: unknown) => StyleReader })
+        .getComputedStyle(el);
+      const r = el.getBoundingClientRect();
+      return {
+        tag: el.tagName,
+        h: Math.round(r.height),
+        w: Math.round(r.width),
+        border: s.getPropertyValue('border-top-color'),
+        bg: s.getPropertyValue('background-color'),
+      };
+    });
+  const barGrant = await box(grant);
+  // A real <button> here, so Enter and Space work without this app reimplementing
+  // them. The card cannot have one (it is itself a <button>) and keeps the span.
+  expect(barGrant.tag).toBe('BUTTON');
+  // The box, not "has some border and some background": a bare <button> with this
+  // app's rule deleted still carries Chromium's own 2px border, #efefef fill and
+  // 6px padding, so three inequalities on those pass while the control is back to
+  // a run of text. Measured here: 109x22 against the 92x14 it replaced, and the
+  // app's own blue rather than the platform's grey.
+  expect(barGrant.h).toBeGreaterThanOrEqual(20);
+  expect(barGrant.w).toBeGreaterThanOrEqual(100);
+  expect(barGrant.border).toBe('rgb(47, 111, 237)');
+  expect(barGrant.bg).toBe('rgb(238, 243, 255)');
+
   // F1: the all-databases view puts every profile side by side, which is what
   // makes coverage comparable; the cards carry counts and annotation coverage.
   await page.getByTestId('rail-all').click();
   await expect(page.getByTestId('overview-cards')).toBeVisible();
   await expect(page.getByTestId('profile-bar')).toHaveCount(0);
   await expect(page.getByTestId('rowaccess-badge-beta')).toHaveCount(1);
+
+  // On the card it is a span carrying role=button, and it has to stay one: nesting
+  // a <button> in the card's own <button> is invalid HTML, and the browser's repair
+  // for it is to close the outer element early - which would take the rest of the
+  // card out of the button that selects the profile. Same presentation either way.
+  const cardSpan = page.getByTestId('rowaccess-enable-beta');
+  const cardGrant = await box(cardSpan);
+  const cardRole = await cardSpan.evaluate((el) => ({
+    role: el.getAttribute('role'),
+    focusable: el.getAttribute('tabindex'),
+  }));
+  expect(cardGrant.tag).toBe('SPAN');
+  expect(cardRole).toEqual({ role: 'button', focusable: '0' });
+  // Same presentation on both surfaces - measured against the bar's own numbers
+  // rather than restated, since "they match" is the property and two separate
+  // property sets would not notice them drifting apart.
+  expect({ h: cardGrant.h, w: cardGrant.w, border: cardGrant.border, bg: cardGrant.bg }).toEqual({
+    h: barGrant.h,
+    w: barGrant.w,
+    border: barGrant.border,
+    bg: barGrant.bg,
+  });
   await expect(page.getByTestId('card-alpha')).toContainText('tables');
   await expect(page.getByTestId('card-alpha')).toContainText('annotated');
 
diff --git a/src/renderer/src/OverviewCards.svelte b/src/renderer/src/OverviewCards.svelte
index 3401630..ac81ec7 100644
--- a/src/renderer/src/OverviewCards.svelte
+++ b/src/renderer/src/OverviewCards.svelte
@@ -120,6 +120,9 @@
           onkeydown={(e) => e.key === 'Enter' && (e.stopPropagation(), ondelete(p.name))}>delete</span
         >
       </div>
+      <!-- No `asButton`: the card is itself a <button> that selects the profile, so
+           the controls in here stay spans. That is also the default, so a host that
+           says nothing cannot get this wrong. -->
       <RowAccessBlock
         profile={p.name}
         level={p.rowAccess}
diff --git a/src/renderer/src/ProfileBar.svelte b/src/renderer/src/ProfileBar.svelte
index 5c86b24..09367b0 100644
--- a/src/renderer/src/ProfileBar.svelte
+++ b/src/renderer/src/ProfileBar.svelte
@@ -87,10 +87,14 @@
       >
     </span>
   </div>
+  <!-- `asButton`: nothing here is an interactive host (the bar's own `delete` has
+       been a real button since this component existed), so the grant can be one
+       too and take Enter and Space from the browser. -->
   <RowAccessBlock
     profile={profile.name}
     level={profile.rowAccess}
     pending={rowAccessPending === profile.name}
+    asButton
     {onrowaccess}
   />
   <McpBlock
diff --git a/src/renderer/src/RowAccessBlock.svelte b/src/renderer/src/RowAccessBlock.svelte
index fee7bb9..250c8de 100644
--- a/src/renderer/src/RowAccessBlock.svelte
+++ b/src/renderer/src/RowAccessBlock.svelte
@@ -15,6 +15,7 @@
     profile,
     level,
     pending,
+    asButton = false,
     onrowaccess,
   }: {
     profile: string;
@@ -23,6 +24,23 @@
      *  The dialog is modal to the window, so at most one request is ever in
      *  flight app-wide. */
     pending: boolean;
+    /** Whether this host can hold a real <button>. Only the profile bar can: the
+     *  overview card is itself a <button> that selects the profile, and a <button>
+     *  inside a <button> is invalid HTML — the accessibility tree of a nested
+     *  interactive element is not defined, and screen readers disagree about what
+     *  to announce.
+     *
+     *  (What it is NOT: a rendering failure. Svelte builds the card and this block
+     *  as separate templates and joins them by DOM insertion, so the parser's
+     *  repair — which would close the card's <button> early — never runs, and the
+     *  nesting simply survives. An earlier version of this comment claimed
+     *  otherwise. Measured, the card keeps all its children either way.)
+     *
+     *  Declared by the host and OFF by default, so that the answer a host gets by
+     *  forgetting is the one that renders correctly anywhere. The default used to
+     *  be spelled the other way round, `insideButton = false`, which read as the
+     *  safe default and was the opposite of one. */
+    asButton?: boolean;
     onrowaccess: (name: string, level: RowAccess) => void;
   } = $props();
 
@@ -52,56 +70,81 @@
 
   const ra = $derived(rowAccessBadge(level));
 
-  /** Keyboard/click helper for the linkish span-buttons. `stopPropagation` is
-   *  unconditional: on the overview card these sit inside the card's own
-   *  <button> (which selects the profile), and a click that enables browsing
-   *  must not also select. In the profile bar there is nothing above them to
-   *  stop, so it costs nothing there. Spans rather than buttons for the same
-   *  reason — a <button> inside a <button> is not valid HTML. */
+  /** Keyboard/click helper for the controls. `stopPropagation` is unconditional:
+   *  on the overview card these sit inside the card's own <button> (which selects
+   *  the profile), and a click that enables browsing must not also select. In the
+   *  profile bar there is nothing above them to stop, so it costs nothing there.
+   *
+   *  The span branch adds a keydown handler for Enter only. A real button answers
+   *  Enter AND Space; this does not, and on the card Space is worse than inert —
+   *  the key reaches the card's own <button> and selects the profile instead, and
+   *  Enter fires both, because Chromium's default handlers walk the event path and
+   *  are gated on preventDefault rather than on propagation. That is pre-existing
+   *  (the bar's own spans behaved the same before this control had a box) and it is
+   *  recorded rather than fixed here. It is also the sharper reason the bar takes a
+   *  real button: on that surface the keyboard simply works. */
   const act = (e: Event, fn: () => void): void => {
     e.stopPropagation();
     fn();
   };
+
+  /** The grants this level offers, in the order they are offered. One list so the
+   *  two element kinds above cannot drift into offering different things. */
+  const controls = $derived.by((): { testid: string; cls: string; label: string; run: () => void }[] =>
+    level === 'off'
+      ? [
+          {
+            testid: `rowaccess-enable-${profile}`,
+            cls: 'grant',
+            label: 'enable browsing',
+            run: () => onrowaccess(profile, 'read'),
+          },
+        ]
+      : [
+          ...(level === 'read'
+            ? [
+                {
+                  testid: `rowaccess-edit-${profile}`,
+                  cls: 'grant danger',
+                  label: 'enable editing',
+                  run: () => onrowaccess(profile, 'readwrite'),
+                },
+              ]
+            : []),
+          {
+            testid: `rowaccess-off-${profile}`,
+            cls: 'grant danger',
+            label: 'turn off',
+            run: () => onrowaccess(profile, 'off'),
+          },
+        ],
+  );
 </script>
 
 <div class="row rowaccess" data-testid={`rowaccess-${profile}`}>
   <span class={`ra-badge ${ra.cls}`} data-testid={`rowaccess-badge-${profile}`}>{ra.text}</span>
   {#if pending}
     <span class="ra-wait">waiting for approval...</span>
-  {:else if level === 'off'}
+  {:else}
     <!-- Browsing first, editing as its own step. Each level is a separate
          native approval, so nobody arrives at write access by clicking
          once — and the dialog for editing says what editing costs. -->
-    <span
-      class="linkish"
-      role="button"
-      tabindex="0"
-      data-testid={`rowaccess-enable-${profile}`}
-      onclick={(e) => act(e, () => onrowaccess(profile, 'read'))}
-      onkeydown={(e) => e.key === 'Enter' && act(e, () => onrowaccess(profile, 'read'))}
-      >enable browsing</span
-    >
-  {:else}
-    {#if level === 'read'}
-      <span
-        class="linkish danger"
-        role="button"
-        tabindex="0"
-        data-testid={`rowaccess-edit-${profile}`}
-        onclick={(e) => act(e, () => onrowaccess(profile, 'readwrite'))}
-        onkeydown={(e) => e.key === 'Enter' && act(e, () => onrowaccess(profile, 'readwrite'))}
-        >enable editing</span
-      >
-    {/if}
-    <span
-      class="linkish danger"
-      role="button"
-      tabindex="0"
-      data-testid={`rowaccess-off-${profile}`}
-      onclick={(e) => act(e, () => onrowaccess(profile, 'off'))}
-      onkeydown={(e) => e.key === 'Enter' && act(e, () => onrowaccess(profile, 'off'))}
-      >turn off</span
-    >
+    {#each controls as c (c.testid)}
+      {#if !asButton}
+        <span
+          class={c.cls}
+          role="button"
+          tabindex="0"
+          data-testid={c.testid}
+          onclick={(e) => act(e, c.run)}
+          onkeydown={(e) => e.key === 'Enter' && act(e, c.run)}>{c.label}</span
+        >
+      {:else}
+        <button type="button" class={c.cls} data-testid={c.testid} onclick={(e) => act(e, c.run)}
+          >{c.label}</button
+        >
+      {/if}
+    {/each}
   {/if}
 </div>
 <!-- Stated at every level, 'off' included: the second approval and the place
@@ -130,13 +173,42 @@
     border-top: 1px dashed #eee;
     padding-top: 0.3rem;
   }
-  .linkish {
+  /* These look like controls, which they had not until now. What they were: blue
+     0.75rem text with `cursor: pointer` and nothing else — no border, no
+     background, no padding — sitting next to a status badge that DID have a
+     border. An operator reading that row sees one bordered thing (the state) and
+     some coloured words, and the words are the only way to grant anything.
+     Two attempts at this failed, and both changed the wording; the settled scope
+     for the discoverability of this control is "wording and presentation", and
+     presentation had never been touched. So: a box, a background, and room to be
+     pressed, the same shape as every other control in the app.
+     Written to cover both element kinds, since the card cannot use a <button>. */
+  .grant {
+    border: 1px solid #2f6fed;
+    border-radius: 6px;
+    background: #eef3ff;
+    padding: 0.15rem 0.55rem;
     color: #2f6fed;
+    /* `font: inherit` is for the button, which would otherwise take the platform's
+       own family and size. Nothing else about the type is set here: an earlier
+       version added `font-size: 0.72rem` "for the button", which the button was
+       already inheriting from `.rowaccess` — so all it did was shrink the label
+       from 12px to 11.52px on both kinds. Making the words 4% smaller is the
+       wrong direction for a control two people could not find. */
+    font: inherit;
     cursor: pointer;
   }
-  .linkish.danger {
+  .grant:hover {
+    background: #dfe9ff;
+  }
+  .grant.danger {
+    border-color: #a00;
+    background: #fff2f2;
     color: #a00;
   }
+  .grant.danger:hover {
+    background: #ffe6e6;
+  }
   .ra-badge {
     border: 1px solid #ccc;
     border-radius: 999px;