trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns16g rEventsFlows

Design: top bar diet #2703

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/forge:scout/2699-design-top-bar-diet → main 3 commits fork

  • Merged. scout/2699-design-top-bar-diet was merged into main as 63b8692.

Review

Sign in to review this change.
3 files changed +75 −47
src/web/client.ts +2 −2
@@ -1262,7 +1262,7 @@ export const shellJs = /* js */ `
12621262 if (!m || !m.matches || !m.matches('details.menu[open]')) return;
12631263 closeMenus(m);
12641264 var first = m.querySelector('.menu-panel .menu-item, .menu-panel input');
1265 if (first && !m.querySelector('.theme-option')) first.focus();
1265 if (first && !first.closest('.theme-option')) first.focus();
12661266 }, true);
12671267 doc.addEventListener('keydown', function (e) {
12681268 if (e.key !== 'ArrowDown' && e.key !== 'ArrowUp') return;
@@ -1275,7 +1275,7 @@ export const shellJs = /* js */ `
12751275 });
12761276
12771277 // ---- theme: apply instantly, persist once the choice settles ----
1278 var themeForm = doc.querySelector('.theme-menu form');
1278 var themeForm = doc.querySelector('.theme-form');
12791279 if (themeForm) {
12801280 var apply = themeForm.querySelector('.theme-apply');
12811281 if (apply) apply.hidden = true;
src/web/layout.ts +36 −43
@@ -130,7 +130,8 @@ export interface PageOptions {
130130
131131 // ---- topbar pieces ----
132132
133 function themeMenu(active: ThemeName | undefined): Html {
133 /** The theme choice as a panel section: lives in a menu, never in the bar. */
134 function themeForm(active: ThemeName | undefined): Html {
134135 const option = (value: string, label: string, checked: boolean) =>
135136 html`<label class="theme-option">
136137 <input
@@ -143,11 +144,7 @@ function themeMenu(active: ThemeName | undefined): Html {
143144 ${label}
144145 <span class="tick" aria-hidden="true">${icon("check")}</span>
145146 </label>`;
146 return html`<details class="menu theme-menu">
147 <summary class="btn btn-icon" aria-label="Choose theme" title="Theme">
148 ${themeGlyph}
149 </summary>
150 <form class="menu-panel" method="post" action="/theme">
147 return html`<form class="theme-form" method="post" action="/theme">
151148 <span class="menu-title">Theme</span>
152149 ${option("system", "System", active === undefined)}
153150 ${THEME_NAMES.map((name) =>
@@ -156,8 +153,22 @@ function themeMenu(active: ThemeName | undefined): Html {
156153 <button class="btn theme-apply" data-size="sm" type="submit">
157154 Apply
158155 </button>
159 </form>
160 </details>`;
156 </form>`;
157 }
158
159 /** Keyboard help: js-only, since the shortcuts it lists only exist then. */
160 const helpItem = html`<button class="menu-item js-only hide-sm" type="button" data-help>
161 ${icon("help")}<span class="grow">Keyboard shortcuts</span>
162 </button>`;
163
164 /** Someone not signed in has no user menu; theme and help share this one. */
165 function guestMenu(theme: ThemeName | undefined): Html {
166 return menu({
167 trigger: themeGlyph,
168 items: [helpItem, themeForm(theme)],
169 label: "Appearance and help",
170 triggerOptions: { extra: "btn-icon" },
171 });
161172 }
162173
163174 /** A whole forge for someone (`/forges/new`): any signed-in person's. */
@@ -194,8 +205,13 @@ function newMenu(ctx: Ctx): Html {
194205 });
195206 }
196207
197 function userMenu(ctx: Ctx, user: User, signOut: MenuItem): Html {
198 const items: MenuItem[] = [
208 function userMenu(
209 ctx: Ctx,
210 user: User,
211 theme: ThemeName | undefined,
212 signOut: MenuItem,
213 ): Html {
214 const items: (MenuItem | Html)[] = [
199215 { label: "Your profile", href: `/${user.username}`, icon: "user" },
200216 { label: "Your repositories", href: "/", icon: "repo" },
201217 { label: "Apps", href: "/apps", icon: "rocket" },
@@ -225,6 +241,12 @@ function userMenu(ctx: Ctx, user: User, signOut: MenuItem): Html {
225241 ]
226242 : []),
227243 { separator: true, label: "" },
244 ...(user.isAdmin
245 ? [{ label: "Flow", href: "/flow", icon: "activity" as const }]
246 : []),
247 helpItem,
248 themeForm(theme),
249 { separator: true, label: "" },
228250 signOut,
229251 ];
230252 return menu({
@@ -257,25 +279,6 @@ function searchBar(): Html {
257279 >`;
258280 }
259281
260 /**
261 * Keyboard help. The shortcuts it lists only exist when the script runs, so
262 * the button that opens it only exists then too — `js-only` is revealed by
263 * the shell script rather than shown and then found to be dead.
264 */
265 function helpButton(): Html {
266 // Keyboard shortcuts have no meaning on a phone; the button would only
267 // cost the width that pushes the page sideways.
268 return html`<button
269 class="btn btn-icon js-only hide-sm"
270 type="button"
271 data-help
272 aria-label="Keyboard shortcuts"
273 title="Keyboard shortcuts"
274 >
275 ${icon("help")}
276 </button>`;
277 }
278
279282 // ---- palette index ----
280283
281284 /** The palette's own verbs: where to go, what to make, what to set. */
@@ -459,22 +462,12 @@ function topbar(
459462 )}
460463 <span class="spacer"></span>
461464 <span class="topbar-tools">
462 ${when(
463 user?.isAdmin === true,
464 () =>
465 html`<a
466 id="flow-link"
467 class="btn"
468 data-size="sm"
469 href="/flow"
470 title="The pipeline and its bottleneck"
471 >${icon("activity")}<span class="hide-sm">Flow</span></a
472 >`,
473 )}${searchBar()}<span class="hide-sm">${helpButton()}</span>${themeMenu(theme)}
465 ${searchBar()}
474466 ${user
475467 ? html`<span class="hide-sm">${newMenu(ctx)}</span>${userMenu(
476468 ctx,
477469 user,
470 theme,
478471 ctx.auth.via === "edge"
479472 ? {
480473 label: "Sign out",
@@ -483,12 +476,12 @@ function topbar(
483476 }
484477 : { label: "Sign out", post: "/logout", icon: "signOut" },
485478 )}`
486 : btn("Sign in", {
479 : html`${guestMenu(theme)}${btn("Sign in", {
487480 href:
488481 ctx.app.config.edgeToken === undefined
489482 ? "/signin"
490483 : edgeLoginUrl(ctx.url.pathname + ctx.url.search),
491 })}
484 })}`}
492485 </span>
493486 </header>`;
494487 }
test/flow-pipeline.test.ts +37 −2
@@ -23,6 +23,7 @@ import { createRun } from "../src/core/runs.ts";
2323 import { createUser, type User } from "../src/core/users.ts";
2424 import { createToken } from "../src/core/tokens.ts";
2525 import { bottleneck, readPipeline, span, verdict } from "../src/flow/pipeline.ts";
26 import { shellJs } from "../src/web/client.ts";
2627 import { serve, type Servers } from "../src/http/server.ts";
2728 import type { Ctx, Screen } from "../src/tui/app/app.ts";
2829 import { forgeCommands, Session } from "../src/tui/forge.ts";
@@ -247,16 +248,50 @@ describe("/flow: the forge shows its own bottleneck", () => {
247248 for (const key of ["ready", "building", "checking", "review", "approved", "landed", "live"])
248249 expect(page).toContain(`data-stage="${key}"`);
249250 expect(page).toContain('href="/settings/capacity"');
250 expect(page).toContain('id="flow-link"');
251 // An admin reaches Flow from every page, through the user menu.
252 expect(await (await get("/", cookie)).text()).toContain('href="/flow"');
251253 });
252254
253255 test("nobody but a forge admin reads it", async () => {
254256 const bob = await login("bob");
255257 expect((await get("/flow", bob)).status).toBe(404);
256 expect(await (await get("/", bob)).text()).not.toContain('id="flow-link"');
258 expect(await (await get("/", bob)).text()).not.toContain('href="/flow"');
257259 expect((await get("/flow", "")).status).not.toBe(200);
258260 });
259261
262 test("the top bar holds search, New and the user menu; Flow, help and theme live in menus", async () => {
263 expect(alice.isAdmin).toBe(true);
264 const page = await (await get("/", await login("alice"))).text();
265 const bar = page.slice(page.indexOf('<header class="topbar"'), page.indexOf("</header>"));
266 // Nothing but menus and search sits in the bar: no bare Flow link, no help button.
267 expect(bar).not.toContain("flow-link");
268 expect(bar).not.toContain("theme-menu");
269 expect(bar.match(/<details/g)?.length).toBe(2);
270 // They are still one open away, in the user menu.
271 const panel = bar.slice(bar.lastIndexOf('<details'));
272 expect(panel).toContain('href="/flow"');
273 expect(panel).toContain("data-help");
274 expect(panel).toContain('action="/theme"');
275 });
276
277 test("a guest's bar has search, one appearance menu and Sign in; no bare theme button", async () => {
278 const page = await (await get("/", "")).text();
279 const bar = page.slice(page.indexOf('<header class="topbar"'), page.indexOf("</header>"));
280 expect(bar).toContain("Sign in");
281 expect(bar).not.toContain("theme-menu");
282 expect(bar.replace(/<details[\s\S]*?<\/details>/g, "")).not.toContain("<button");
283 expect(bar.match(/<details/g)?.length).toBe(1);
284 const panel = bar.slice(bar.indexOf("<details"));
285 expect(panel).toContain("data-help");
286 expect(panel).toContain('action="/theme"');
287 });
288
289 test("the shell script wires the theme form by its new class and guards focus on theme options", () => {
290 expect(shellJs).toContain("doc.querySelector('.theme-form')");
291 expect(shellJs).not.toContain(".theme-menu form");
292 expect(shellJs).toContain("first.closest('.theme-option')");
293 });
294
260295 const flowCommand = (user?: User) => {
261296 const pushed: Screen[] = [];
262297 const ctx = { push: (s: Screen) => pushed.push(s) } as unknown as Ctx;

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs