Correct the shared chrome for its second adopter (Sheets)
Menu rows can POST or open a dialog (menu.Item.Action/.Dialog, via command=show-modal). Bar.Actions is now []chrome.Action, so a bar action can be a POST (Sheets' Create). estilo-shell=fill is a full-height layout for the grid. chrome.js closes every menu in the estilo-menus group on Escape or an outside click, handles Escape in the capture phase, and keeps the menubar's roving tab stop when returning focus. Docs needs a two-line Bar.Actions edit on its next bump. Reviewed by GPT-6-Astra via Codex (#11). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Squashed from feat/r2-sheets-corrections into main.
10 files changed,
+345
−33
README.md+8 −2chrome/chrome.css+19 −0chrome/chrome.go+16 −3chrome/chrome.js+57 −12chrome/chrome.test.mjs+103 −0chrome/chrome_test.go+78 −4chrome/templates/bar.html+9 −6menu/menu.go+18 −1menu/menu_test.go+28 −0menu/templates/menu.html+9 −5
CI failed — run details
CI log
=== vet === CGO_ENABLED=0 go vet ./... ../../cache/go-mod/github.com/carlosframework/rastrillo@v0.23.0/sessions/sessions.go:72:12: pattern migrations/*.sql: no matching files found make: *** [Makefile:22: test-vet] Error 1 amadan: step vet failed: exit status 2 amadan: job failed at step vet — its output is above, not at the tail of this log
diff --git a/README.md b/README.md| index 302947e..a93b8ce 100644 |
| --- a/README.md |
| +++ b/README.md |
| @@ -25,8 +25,14 @@ document menu bar, with `menu.Shortcut` for platform-correct shortcut |
| labels; and the top bar, account menu, app switcher, error page and |
| sign-in pages, with the per-app Appearance preference. Link the three |
| stylesheets in that order, after `tokens.css`, the theme and |
| -`brand.css`. `brand` serves the app icons from Go. Docs adopts first and |
| -Sheets second; until Sheets has, they are a home, not an agreed interface. |
| +`brand.css`. `brand` serves the app icons from Go. Docs adopted first and |
| +Sheets second, and Sheets' corrections are in: a menu row or bar action |
| +can be a link, a POST (`menu.Item.Action`) or a dialog opened by the |
| +platform's invoker command (`menu.Item.Dialog`); `Bar.Actions` is |
| +`[]chrome.Action`; `<div estilo-shell="fill">` is the full-height layout |
| +(the viewport less the 52px bar); and `chrome.js` closes every member of |
| +the one `estilo-menus` group, menubar menus included, on Escape (in the |
| +capture phase) and an outside click. |
| Permission and session logic stay app-owned. There is no |
| shared component gallery yet. |
diff --git a/chrome/chrome.css b/chrome/chrome.css| index d40baff..5670e8f 100644 |
| --- a/chrome/chrome.css |
| +++ b/chrome/chrome.css |
| @@ -58,6 +58,7 @@ |
| [estilo-bar-nav] a[aria-current] { border-block-end-color: var(--rst-accent); color: var(--rst-text); font-weight: 550; } |
| [estilo-bar-end] { display: flex; align-items: center; gap: var(--rst-sp-2); flex: none; margin-inline-start: auto; } |
| [estilo-bar-actions] { display: flex; align-items: center; gap: var(--rst-sp-2); } |
| +[estilo-bar-actions] form { display: contents; } /* a POST action lays out as its button */ |
| [estilo-bar-trigger] { |
| display: inline-flex; |
| align-items: center; |
| @@ -87,6 +88,24 @@ |
| [estilo-bar]:has([estilo-bar-context], [estilo-bar-title]) [estilo-bar-name] { display: none; } /* the icon stays; the title gets the room */ |
| } |
| +/* The shell. <div estilo-shell> wraps the skip link, the bar and <main> |
| + on every page and has no styling of its own. estilo-shell="fill" is the |
| + full-height layout for the one kind of page that must fill the viewport |
| + (Sheets' grid): the shell is exactly the viewport, the bar keeps its |
| + 52px, and <main> takes the rest, so the working area is the viewport |
| + less the bar without any app re-deriving that sum. The document never |
| + scrolls there, so the root drops its reserved scrollbar gutter, which |
| + would otherwise leave a dead strip down the inline end. <main> must be |
| + the shell's direct child. */ |
| +[estilo-shell~="fill"] { |
| + display: flex; |
| + flex-direction: column; |
| + block-size: 100dvh; |
| +} |
| +[estilo-shell~="fill"] > [estilo-bar] { flex: none; } |
| +[estilo-shell~="fill"] > main { flex: 1 1 auto; min-block-size: 0; } |
| +:root:has([estilo-shell~="fill"]) { scrollbar-gutter: auto; } |
| + |
| /* The editable title slot: the open object's title, renamed in place. The |
| input is a bordered control, so it keeps day's radius; its block size is |
| fixed so an input in the bar can never grow the bar. */ |
diff --git a/chrome/chrome.go b/chrome/chrome.go| index c72c159..ef49e6a 100644 |
| --- a/chrome/chrome.go |
| +++ b/chrome/chrome.go |
| @@ -75,6 +75,19 @@ type TitleEdit struct { |
| Label, Submit string |
| } |
| +// Action is one page action. Its Item says what activating it does (a |
| +// link, a POST or a dialog; see menu.Item), so the bar's button and the |
| +// Menu sheet's row are the same element kind and cannot disagree. Primary |
| +// marks the page's main action, drawn as the primary button. |
| +// |
| +// Docs' actions were all links (chrome.Link). Sheets' Create is a POST, |
| +// and a link in the sheet would be a GET to the list that creates |
| +// nothing, so the action carries the item instead. |
| +type Action struct { |
| + menu.Item |
| + Primary bool |
| +} |
| + |
| // BarLabels are the bar's own strings: the skip link (rendered by the |
| // app's layout) and the narrow Menu trigger. |
| type BarLabels struct { |
| @@ -91,9 +104,9 @@ type Bar struct { |
| // Nav is the app's section links; NavLabel their accessible name. |
| Nav []Link |
| NavLabel string |
| - // Actions are page actions ("Share…"): buttons at wide widths, rows |
| - // in the Menu sheet below 800px. |
| - Actions []Link |
| + // Actions are page actions ("Share…", "Create"): buttons at wide |
| + // widths, rows in the Menu sheet below 800px. |
| + Actions []Action |
| // Switcher is optional; nil drops it with nothing else to change. |
| Switcher *Switcher |
| // Account is set when someone is signed in. Otherwise SignIn is the |
diff --git a/chrome/chrome.js b/chrome/chrome.js| index f0de97f..1f583c6 100644 |
| --- a/chrome/chrome.js |
| +++ b/chrome/chrome.js |
| @@ -1,17 +1,46 @@ |
| -// chrome/chrome.js: optional behaviour for estilo-bar. Load it with |
| -// <script defer>. Everything in the bar already works without it; this |
| -// only adds what <details> cannot do for itself: |
| +// chrome/chrome.js: optional behaviour for estilo-bar and the one top-level |
| +// menu group. Load it with <script defer>. Everything already works |
| +// without it; this only adds what <details> and the platform cannot do for |
| +// themselves: |
| // |
| -// - Escape closes the open bar panel (Menu, switcher, account) and |
| -// returns focus to its trigger; |
| -// - a click outside the open bar panel closes it; |
| +// - Escape closes the open member of the group (name="estilo-menus": the |
| +// bar's Menu, switcher and account, and every menubar menu) and returns |
| +// focus to its title; |
| +// - a click outside the open member closes it; |
| // - Escape in the editable title restores the saved value and leaves |
| -// the field. |
| +// the field; |
| +// - a row or button that opens a dialog (command="show-modal") closes |
| +// the menu it sat in, returns focus to that menu's title when the |
| +// dialog closes, and opens the dialog itself on an engine without |
| +// invoker commands. |
| // |
| -// Menubar menus belong to the app that renders them and are not touched. |
| +// The group is one group in every app, so closing it is estilo's job, not |
| +// each app's: Sheets' menubar lost Escape and outside-click entirely when |
| +// it left rastrillo's dropdown. |
| +// |
| +// Both listen in the capture phase. A document app has to stop key |
| +// events from propagating out of its menus — Sheets' grid library acts on |
| +// every key it hears on document, so an arrow in an open menu would walk |
| +// the cell cursor too — and a bubble-phase listener on document never |
| +// hears the Escape such an app stops. Capture hears it first. An app that |
| +// closes its own menu on Escape (Docs' menubar) finds nothing open and |
| +// does nothing, so nothing is handled twice. |
| ;(() => { |
| - const panels = '[estilo-bar] details[estilo-account], [estilo-bar] details[estilo-switcher], [estilo-bar] details[estilo-bar-more]' |
| - const openPanel = () => document.querySelector(panels.split(', ').map((s) => s + '[open]').join(', ')) |
| + const group = 'details[name="estilo-menus"][open]' |
| + const openPanel = () => document.querySelector(group) |
| + |
| + // Focus returns to a menu's title; in a menubar with a roving tab stop |
| + // (one title tabbable, the rest tabindex=-1) the stop moves with it, so |
| + // Tab then Shift+Tab comes back to the menu you were in, not to File |
| + // (Astra, estilo #11). A menubar without one is left without one. |
| + const returnTo = (title) => { |
| + title.focus() |
| + const bar = title.closest('[estilo-menubar]') |
| + if (!bar) return |
| + const titles = bar.querySelectorAll('[estilo-menubar-title]') |
| + if (!Array.prototype.some.call(titles, (t) => t.hasAttribute('tabindex'))) return |
| + Array.prototype.forEach.call(titles, (t) => { t.tabIndex = t === title ? 0 : -1 }) |
| + } |
| document.addEventListener('keydown', (e) => { |
| if (e.key !== 'Escape' || e.defaultPrevented) return |
| @@ -26,12 +55,28 @@ |
| if (!open) return |
| open.open = false |
| const summary = open.querySelector('summary') |
| - if (summary) summary.focus() |
| + if (summary) returnTo(summary) |
| e.preventDefault() |
| - }) |
| + }, true) |
| document.addEventListener('click', (e) => { |
| const open = openPanel() |
| if (open && !open.contains(e.target)) open.open = false |
| }, true) |
| + |
| + const invokers = typeof HTMLButtonElement !== 'undefined' && 'command' in HTMLButtonElement.prototype |
| + document.addEventListener('click', (e) => { |
| + const button = e.target.closest && e.target.closest('button[commandfor][command="show-modal"]') |
| + if (!button || button.disabled) return |
| + const dialog = document.getElementById(button.getAttribute('commandfor')) |
| + const holder = button.closest('details[name="estilo-menus"]') |
| + if (holder) { |
| + holder.open = false |
| + const summary = holder.querySelector('summary') |
| + if (dialog && summary) dialog.addEventListener('close', () => returnTo(summary), { once: true }) |
| + } |
| + if (invokers || !dialog || typeof dialog.showModal !== 'function' || dialog.open) return |
| + dialog.showModal() |
| + e.preventDefault() |
| + }) |
| })() |
diff --git a/chrome/chrome.test.mjs b/chrome/chrome.test.mjs| new file mode 100644 |
| index 0000000..bf8f924 |
| --- /dev/null |
| +++ b/chrome/chrome.test.mjs |
| @@ -0,0 +1,103 @@ |
| +// chrome.js behaviour, run under Node against a minimal DOM: just enough |
| +// elements, attribute selectors and event dispatch for the script's own |
| +// selectors. estilo has no browser harness; the browser proof is in the |
| +// adopting apps (Sheets' tools/drive/menu-actions.mjs, Docs' menu tests). |
| +import assert from "node:assert/strict"; |
| +import fs from "node:fs"; |
| +import test from "node:test"; |
| +import vm from "node:vm"; |
| + |
| +const src = fs.readFileSync(new URL("./chrome.js", import.meta.url), "utf8"); |
| + |
| +// A compound selector: tag, [attr] and [attr="value"], comma-separated. |
| +function matches(el, selector) { |
| + return selector.split(",").some((part) => { |
| + const m = part.trim().match(/^([a-z]*)((?:\[[^\]]+\])*)$/); |
| + if (!m) throw new Error("unsupported selector " + part); |
| + if (m[1] && el.tag !== m[1]) return false; |
| + for (const [, name, value] of m[2].matchAll(/\[([^=\]]+)(?:="([^"]*)")?\]/g)) { |
| + if (!el.attrs.has(name)) return false; |
| + if (value !== undefined && el.attrs.get(name) !== value) return false; |
| + } |
| + return true; |
| + }); |
| +} |
| + |
| +class El { |
| + constructor(tag, attrs = {}, children = []) { |
| + this.tag = tag; |
| + this.attrs = new Map(Object.entries(attrs)); |
| + this.children = children; |
| + this.parent = null; |
| + for (const c of children) c.parent = this; |
| + } |
| + hasAttribute(n) { return this.attrs.has(n); } |
| + getAttribute(n) { return this.attrs.has(n) ? this.attrs.get(n) : null; } |
| + get open() { return this.attrs.has("open"); } |
| + set open(v) { if (v) this.attrs.set("open", ""); else this.attrs.delete("open"); } |
| + get tabIndex() { return Number(this.attrs.get("tabindex") ?? 0); } |
| + set tabIndex(v) { this.attrs.set("tabindex", String(v)); } |
| + *walk() { for (const c of this.children) { yield c; yield* c.walk(); } } |
| + querySelectorAll(s) { return [...this.walk()].filter((e) => matches(e, s)); } |
| + querySelector(s) { return this.querySelectorAll(s)[0] ?? null; } |
| + closest(s) { for (let e = this; e; e = e.parent) if (e.tag && matches(e, s)) return e; return null; } |
| + contains(o) { for (let e = o; e; e = e.parent) if (e === this) return true; return false; } |
| + focus() { doc.activeElement = this; } |
| +} |
| + |
| +let doc; |
| +function load(root) { |
| + const listeners = []; |
| + doc = { |
| + activeElement: null, |
| + querySelector: (s) => root.querySelector(s), |
| + getElementById: () => null, |
| + addEventListener: (type, fn, capture) => listeners.push({ type, fn, capture: !!capture }), |
| + }; |
| + vm.runInNewContext(src, { document: doc, HTMLButtonElement: function () {} }); |
| + return (type, target, init = {}) => { |
| + const e = { type, target, defaultPrevented: false, preventDefault() { this.defaultPrevented = true; }, ...init }; |
| + for (const l of listeners.filter((l) => l.type === type && l.capture)) l.fn(e); |
| + for (const l of listeners.filter((l) => l.type === type && !l.capture)) l.fn(e); |
| + return e; |
| + }; |
| +} |
| + |
| +function menubar(tabindex) { |
| + const titles = ["File", "Edit", "Tools"].map((name, i) => new El("summary", { "estilo-menubar-title": "", ...(tabindex ? { tabindex: i === 0 ? "0" : "-1" } : {}) })); |
| + const menus = titles.map((t) => new El("details", { "estilo-menubar-menu": "", name: "estilo-menus" }, [t, new El("div", { "estilo-menu": "" }, [new El("button", { "estilo-menu-item": "" })])])); |
| + return { root: new El("", {}, [new El("nav", { "estilo-menubar": "" }, menus)]), titles, menus }; |
| +} |
| + |
| +// Astra, estilo #11: Escape closed the menu and focused its title, but |
| +// the menubar's roving tab stop stayed on File, so Tab then Shift+Tab |
| +// landed on File instead of the menu you had been in. |
| +test("Escape moves a menubar's roving tab stop to the title it focuses", () => { |
| + const { root, titles, menus } = menubar(true); |
| + const dispatch = load(root); |
| + menus[2].open = true; |
| + const item = menus[2].querySelector("button"); |
| + const e = dispatch("keydown", item, { key: "Escape" }); |
| + assert.equal(menus[2].open, false); |
| + assert.equal(doc.activeElement, titles[2]); |
| + assert.deepEqual(titles.map((t) => t.getAttribute("tabindex")), ["-1", "-1", "0"]); |
| + assert.equal(e.defaultPrevented, true); |
| +}); |
| + |
| +test("a menubar without a roving tab stop is left without one", () => { |
| + const { root, titles, menus } = menubar(false); |
| + const dispatch = load(root); |
| + menus[1].open = true; |
| + dispatch("keydown", menus[1].querySelector("button"), { key: "Escape" }); |
| + assert.equal(doc.activeElement, titles[1]); |
| + assert.deepEqual(titles.map((t) => t.getAttribute("tabindex")), [null, null, null]); |
| +}); |
| + |
| +test("Escape is heard before a menu that stops propagation", () => { |
| + const { root, menus } = menubar(true); |
| + const dispatch = load(root); |
| + menus[0].open = true; |
| + // The capture listener runs first, whatever the target's own handlers do. |
| + dispatch("keydown", menus[0].querySelector("button"), { key: "Escape" }); |
| + assert.equal(menus[0].open, false); |
| +}); |
diff --git a/chrome/chrome_test.go b/chrome/chrome_test.go| index c78bdd1..97a312a 100644 |
| --- a/chrome/chrome_test.go |
| +++ b/chrome/chrome_test.go |
| @@ -6,6 +6,7 @@ import ( |
| "net/http" |
| "net/http/httptest" |
| "net/url" |
| + "os/exec" |
| "regexp" |
| "strings" |
| "testing" |
| @@ -88,8 +89,11 @@ func signedIn(label string) Bar { |
| ContextEdit: &TitleEdit{Action: "/d/abc/title", Name: "title", Value: label + "-title", Label: label + "-title-label", Submit: label + "-rename"}, |
| Nav: []Link{{Href: "/", Label: label + "-nav", Current: true}}, |
| NavLabel: label + "-navlabel", |
| - Actions: []Link{{Href: "/d/abc/share", Label: label + "-share"}}, |
| - Switcher: &Switcher{Label: label + "-apps", AllApps: menu.Item{Href: "https://oficina.example/", Label: label + "-allapps"}}, |
| + Actions: []Action{ |
| + {Item: menu.Item{Href: "/d/abc/share", Label: label + "-share"}}, |
| + {Item: menu.Item{Action: "/d/new", Label: label + "-create"}, Primary: true}, |
| + }, |
| + Switcher: &Switcher{Label: label + "-apps", AllApps: menu.Item{Href: "https://oficina.example/", Label: label + "-allapps"}}, |
| Account: &Account{ |
| TriggerLabel: label + "-trigger", Initials: "P", Address: "paul@example.test", |
| Labels: AccountLabels{SignedInAs: label + "-signedinas", Appearance: label + "-appearance", Help: label + "-help"}, |
| @@ -111,7 +115,7 @@ func TestBarRendersCallerLabelsInEveryLocale(t *testing.T) { |
| `<a estilo-bar-brand href="/">`, esc(label) + "-app", |
| `<form estilo-bar-title method="post" action="/d/abc/title">`, `value="` + esc(label) + `-title"`, `aria-label="` + esc(label) + `-title-label"`, esc(label) + "-rename", |
| `aria-label="` + esc(label) + `-navlabel"`, `aria-current="page"`, |
| - esc(label) + "-share", esc(label) + "-menu", |
| + esc(label) + "-share", esc(label) + "-create", esc(label) + "-menu", |
| `aria-label="` + esc(label) + `-apps"`, esc(label) + "-allapps", |
| `aria-label="` + esc(label) + `-trigger"`, esc(label) + "-signedinas", "paul@example.test", |
| esc(label) + "-connect", esc(label) + "-appearance", esc(label) + "-light", esc(label) + "-dark", esc(label) + "-system", |
| @@ -235,11 +239,60 @@ func TestNoMenuTriggerWithoutNavOrActions(t *testing.T) { |
| } |
| } |
| +// An action is not always a link. Sheets' Create is a POST, and it must |
| +// be one in both places the bar draws it: a primary button at wide widths |
| +// and a row in the narrow Menu sheet. Drawn as a link in the sheet, it |
| +// would be a GET to the list and create nothing. |
| +func TestActionsCanPostInTheBarAndTheSheet(t *testing.T) { |
| + html := render(t, "estilo-bar", signedIn("x")) |
| + actions := html[strings.Index(html, "<div estilo-bar-actions>"):strings.Index(html, "<details estilo-bar-more")] |
| + for _, want := range []string{ |
| + `<a rst-btn href="/d/abc/share">x-share</a>`, |
| + `<form method="post" action="/d/new"><button rst-btn="primary" type="submit">x-create</button></form>`, |
| + } { |
| + if !strings.Contains(actions, want) { |
| + t.Errorf("bar actions lack %q in %s", want, actions) |
| + } |
| + } |
| + sheet := html[strings.Index(html, `<div estilo-menu="sheet">`):strings.Index(html, "<details estilo-switcher")] |
| + for _, want := range []string{ |
| + `<a estilo-menu-item href="/d/abc/share"><span estilo-menu-label>x-share</span></a>`, |
| + `<form method="post" action="/d/new"><button estilo-menu-item type="submit"><span estilo-menu-label>x-create</span></button></form>`, |
| + } { |
| + if !strings.Contains(sheet, want) { |
| + t.Errorf("the Menu sheet lacks %q in %s", want, sheet) |
| + } |
| + } |
| + b := signedIn("x") |
| + b.Actions = []Action{{Item: menu.Item{Dialog: "help", Label: "x-help"}}} |
| + html = render(t, "estilo-bar", b) |
| + if !strings.Contains(html, `<button rst-btn type="button" commandfor="help" command="show-modal">x-help</button>`) { |
| + t.Error("a dialog action is not an invoker button", html) |
| + } |
| +} |
| + |
| +// A full-height app (Sheets' grid) keys its layout on the shell. The rule |
| +// is estilo's, not each app's: the working area is the viewport less the |
| +// bar, and the bar must neither grow nor shrink inside it. |
| +func TestTheFillShellGivesTheWorkingAreaTheRestOfTheViewport(t *testing.T) { |
| + src := string(CSS()) |
| + for _, want := range []string{ |
| + "[estilo-shell~=\"fill\"] {\n display: flex;\n flex-direction: column;\n block-size: 100dvh;", |
| + "[estilo-shell~=\"fill\"] > [estilo-bar] { flex: none; }", |
| + "[estilo-shell~=\"fill\"] > main { flex: 1 1 auto; min-block-size: 0; }", |
| + } { |
| + if !strings.Contains(src, want) { |
| + t.Errorf("chrome.css lacks the fill shell rule %q", want) |
| + } |
| + } |
| +} |
| + |
| func TestUntrustedValuesAreEscaped(t *testing.T) { |
| b := signedIn("x") |
| b.ContextEdit.Value = `"><script>bad()</script>` |
| b.Account.Address = `<img src=x onerror=bad>` |
| b.Actions[0].Href = "javascript:bad()" |
| + b.Actions[1].Action = "javascript:bad()" |
| html := render(t, "estilo-bar", b) |
| if strings.Contains(html, "<script>") || strings.Contains(html, "<img src=x") || !strings.Contains(html, "#ZgotmplZ") { |
| t.Fatal("unsafe bar markup", html) |
| @@ -368,9 +421,30 @@ func TestScriptIsClassicAndSelfContained(t *testing.T) { |
| if strings.Contains(src, "import ") || strings.Contains(src, "export ") { |
| t.Fatal("chrome.js must be a classic script") |
| } |
| - for _, want := range []string{"Escape", "estilo-bar-title-input", "defaultValue"} { |
| + // Escape and outside-click close every member of the one top-level |
| + // group, the menubar's menus included: Sheets' menubar lost both when |
| + // it left rastrillo's dropdown. And an invoker row closes the menu it |
| + // was in, with a fallback where the platform lacks invoker commands. |
| + for _, want := range []string{"Escape", "estilo-bar-title-input", "defaultValue", `details[name="estilo-menus"][open]`, "commandfor", "showModal"} { |
| if !strings.Contains(src, want) { |
| t.Errorf("chrome.js lost %q", want) |
| } |
| } |
| + // Escape is heard in the capture phase: an app that stops a key from |
| + // propagating out of its menus (Sheets, whose grid library listens on |
| + // document) would otherwise swallow it before a bubble listener ran. |
| + keydown := src[strings.Index(src, "addEventListener('keydown'"):] |
| + keydown = keydown[:strings.Index(keydown[1:], "document.addEventListener(")+1] |
| + if !strings.HasSuffix(strings.TrimSpace(keydown), "}, true)") { |
| + t.Error("chrome.js's Escape handler is not in the capture phase") |
| + } |
| +} |
| + |
| +// chrome.js's behaviour runs under Node against a minimal DOM |
| +// (chrome.test.mjs), the way savestatus's does. |
| +func TestScriptBehaviour(t *testing.T) { |
| + out, err := exec.Command("node", "--test", "chrome.test.mjs").CombinedOutput() |
| + if err != nil { |
| + t.Fatalf("node --test chrome.test.mjs: %v\n%s", err, out) |
| + } |
| } |
diff --git a/chrome/templates/bar.html b/chrome/templates/bar.html| index 4ac79e8..84bd9e8 100644 |
| --- a/chrome/templates/bar.html |
| +++ b/chrome/templates/bar.html |
| @@ -36,7 +36,7 @@ |
| <div estilo-menu="sheet"> |
| {{- range .Nav}}<a estilo-menu-item href="{{.Href}}"{{if .Current}} aria-current="page"{{end}}><span estilo-menu-label>{{.Label}}</span></a>{{end}} |
| {{- if and .Nav .Actions}}<hr>{{end}} |
| - {{- range .Actions}}<a estilo-menu-item href="{{.Href}}"><span estilo-menu-label>{{.Label}}</span></a>{{end}} |
| + {{- range .Actions}}{{template "estilo-menu-item" .Item}}{{end}} |
| </div> |
| </details> |
| {{- end}} |
| @@ -51,11 +51,14 @@ |
| </div> |
| </header>{{end}} |
| -{{/* estilo-bar-actions: the wide-screen actions, from Bar.Actions. The |
| - same links appear as rows in the narrow Menu sheet. An app may |
| - redefine this after parsing to change how they are drawn, but the |
| - sheet always renders Bar.Actions, so both stay one list. */}} |
| -{{define "estilo-bar-actions"}}{{range .Actions}}<a rst-btn href="{{.Href}}">{{.Label}}</a>{{end}}{{end}} |
| +{{/* estilo-bar-actions: the wide-screen actions, from Bar.Actions, each |
| + the element its Item names: a link, a one-button POST form, or a |
| + button opening a dialog. The same items are rows in the narrow Menu |
| + sheet, so both stay one list. An app may redefine this after parsing |
| + to change how they are drawn, but the sheet always renders |
| + Bar.Actions. */}} |
| +{{define "estilo-bar-actions"}}{{range .Actions}}{{template "estilo-bar-action" .}}{{end}}{{end}} |
| +{{define "estilo-bar-action"}}{{if .Action}}<form method="post" action="{{.Action}}"><button rst-btn{{if .Primary}}="primary"{{end}} type="submit">{{.Label}}</button></form>{{else if .Dialog}}<button rst-btn{{if .Primary}}="primary"{{end}} type="button" commandfor="{{.Dialog}}" command="show-modal">{{.Label}}</button>{{else}}<a rst-btn{{if .Primary}}="primary"{{end}} href="{{.Href}}">{{.Label}}</a>{{end}}{{end}} |
| {{/* estilo-bar-title: the open object's title, editable in place. A real |
| form, so Enter renames with scripts off. The submit button is visually |
diff --git a/menu/menu.go b/menu/menu.go| index 59a78c6..f143c47 100644 |
| --- a/menu/menu.go |
| +++ b/menu/menu.go |
| @@ -29,11 +29,28 @@ func CSS() []byte { return append([]byte(nil), css...) } |
| // the app's page templates with ParseFS(Templates(), "templates/*.html"). |
| func Templates() embed.FS { return templateFS } |
| -// Item is one link row rendered by estilo-menu-item. |
| +// Item is one row rendered by estilo-menu-item. Set exactly one of Href, |
| +// Action and Dialog; each is a different element, and each works with |
| +// scripts off: |
| +// |
| +// - Href: a link, <a href>, for a row that navigates. |
| +// - Action: a one-button <form method="post"> to that path, for a row |
| +// that changes something ("New sheet"). |
| +// - Dialog: a <button> that opens the <dialog> with that id through the |
| +// platform's invoker command (command="show-modal"), for a row that |
| +// asks for more before anything happens ("Keyboard shortcuts…"). |
| +// chrome.js opens it where the engine has no invoker commands. |
| +// |
| +// Action and Dialog came with Sheets, the second adopter: Docs' rows were |
| +// all links, and Sheets' Help moved into the account menu as a dialog. |
| type Item struct { |
| Href, Label string |
| // Key is an already-formatted shortcut, from Shortcut. Optional. |
| Key string |
| + // Action is the POST target of a form row. |
| + Action string |
| + // Dialog is the id of the <dialog> a button row opens. |
| + Dialog string |
| } |
| // Apple reports whether the request comes from an Apple platform, the |
diff --git a/menu/menu_test.go b/menu/menu_test.go| index 4af67e2..60c5158 100644 |
| --- a/menu/menu_test.go |
| +++ b/menu/menu_test.go |
| @@ -71,6 +71,34 @@ func TestItemRendersCallerLabelsInEveryLocale(t *testing.T) { |
| } |
| } |
| +// Sheets, the second adopter, needs rows that are not links: New sheet |
| +// is a POST, and Keyboard shortcuts… opens a dialog. Both must work with |
| +// scripts off, so a POST is a one-button form and a dialog opens with the |
| +// platform's invoker command (command="show-modal"), not a click handler. |
| +func TestItemCanPostOrOpenADialog(t *testing.T) { |
| + for locale, label := range locales { |
| + t.Run(locale, func(t *testing.T) { |
| + esc := template.HTMLEscapeString(label) |
| + html := render(t, "estilo-menu-item", Item{Action: "/sheets", Label: label, Key: "⌥N"}) |
| + if !strings.Contains(html, `<form method="post" action="/sheets"><button estilo-menu-item type="submit"><span estilo-menu-label>`+esc+`</span><kbd estilo-menu-key>⌥N</kbd></button></form>`) || strings.Contains(html, "<a ") { |
| + t.Fatalf("a POST row is not a one-button form: %s", html) |
| + } |
| + html = render(t, "estilo-menu-item", Item{Dialog: "sheets-shortcuts", Label: label}) |
| + if !strings.Contains(html, `<button estilo-menu-item type="button" commandfor="sheets-shortcuts" command="show-modal"><span estilo-menu-label>`+esc+`</span></button>`) || strings.Contains(html, "<a ") || strings.Contains(html, "<form") { |
| + t.Fatalf("a dialog row is not an invoker button: %s", html) |
| + } |
| + }) |
| + } |
| + html := render(t, "estilo-menu-item", Item{Action: "javascript:bad()", Label: "x"}) |
| + if !strings.Contains(html, "#ZgotmplZ") { |
| + t.Fatal("an unsafe POST target is not neutralised", html) |
| + } |
| + html = render(t, "estilo-menu-item", Item{Dialog: `x" onclick="bad()`, Label: "x"}) |
| + if strings.Contains(html, `onclick="bad()"`) { |
| + t.Fatal("a dialog id escapes its attribute", html) |
| + } |
| +} |
| + |
| func TestItemEscapesUntrustedHrefAndLabel(t *testing.T) { |
| html := render(t, "estilo-menu-item", Item{Href: "javascript:alert(1)", Label: "<img src=x onerror=bad>"}) |
| if !strings.Contains(html, "#ZgotmplZ") || strings.Contains(html, "<img") { |
diff --git a/menu/templates/menu.html b/menu/templates/menu.html| index 8eb07c0..f0982b8 100644 |
| --- a/menu/templates/menu.html |
| +++ b/menu/templates/menu.html |
| @@ -1,8 +1,12 @@ |
| -{{/* estilo-menu-item: one row in any estilo-menu. A link: every action that |
| - can be a link or a form works with scripts off. Callers supply the |
| - label in their own locale; Key is an already-formatted shortcut |
| - (menu.Shortcut). */}} |
| -{{define "estilo-menu-item"}}<a estilo-menu-item href="{{.Href}}"><span estilo-menu-label>{{.Label}}</span>{{with .Key}}<kbd estilo-menu-key>{{.}}</kbd>{{end}}</a>{{end}} |
| +{{/* estilo-menu-item: one row in any estilo-menu. Data: menu.Item, with |
| + exactly one of Href (a link), Action (a one-button POST form) or |
| + Dialog (a button opening the <dialog> with that id through the |
| + platform's invoker command). All three work with scripts off; chrome.js |
| + only adds the dialog for engines without invoker commands. Callers |
| + supply the label in their own locale; Key is an already-formatted |
| + shortcut (menu.Shortcut). */}} |
| +{{define "estilo-menu-item"}}{{if .Action}}<form method="post" action="{{.Action}}"><button estilo-menu-item type="submit">{{template "estilo-menu-item-body" .}}</button></form>{{else if .Dialog}}<button estilo-menu-item type="button" commandfor="{{.Dialog}}" command="show-modal">{{template "estilo-menu-item-body" .}}</button>{{else}}<a estilo-menu-item href="{{.Href}}">{{template "estilo-menu-item-body" .}}</a>{{end}}{{end}} |
| +{{define "estilo-menu-item-body"}}<span estilo-menu-label>{{.Label}}</span>{{with .Key}}<kbd estilo-menu-key>{{.}}</kbd>{{end}}{{end}} |
| {{/* estilo-icon-check: the glyph inside an estilo-menu-check column. |
| Lucide "check" (ISC, brand/LICENSE.lucide). */}} |
| {{define "estilo-icon-check"}}<svg viewBox="0 0 24 24" width="16" height="16" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true" focusable="false"><path d="M20 6 9 17l-5-5"/></svg>{{end}} |