Public: anyone can read this; the crew can change it.
Every link in the forge pill is a 44 px tap target, and none is drawn without somewhere to go #8467
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/8462-every-link-in-the-forge-pill-is-a-44-px → main 1 commit fork 2 threads · 2 unresolved
2 files
changed +19
−3
src/apps/toolbar.ts +4 −3
| @@ -92,9 +92,10 @@ export const toolbarJs = `(function () { | ||
| 92 | 92 | link.textContent = "Home"; |
| 93 | 93 | link.rel = "noopener"; |
| 94 | 94 | link.style.cssText = LS; |
| 95 | var hasHome = false; | |
| 95 | 96 | try { |
| 96 | 97 | var target = new URL(home); |
| 97 | if (target.origin === fo) link.href = target.href; | |
| 98 | if (target.origin === fo) { link.href = target.href; hasHome = true; } | |
| 98 | 99 | } catch (e) {} |
| 99 | 100 | // Tell the builders: one box, one button, no account. The note goes to |
| 100 | 101 | // this app's own host (same-origin, so the sender's cookie goes along) and |
| @@ -295,7 +296,7 @@ export const toolbarJs = `(function () { | ||
| 295 | 296 | var crew = document.createElement("div"); |
| 296 | 297 | crew.setAttribute("aria-live", "polite"); |
| 297 | 298 | panel.appendChild(crew); |
| 298 | panel.appendChild(link); | |
| 299 | if (hasHome) panel.appendChild(link); | |
| 299 | 300 | // The actions live on a forge page, opened in a tab of its own: the bar |
| 300 | 301 | // posts nothing and reads nothing of this page beyond its address. |
| 301 | 302 | if (d.repo && /^[\\w.-]+\\/[\\w.-]+$/.test(d.repo)) { |
| @@ -303,7 +304,7 @@ export const toolbarJs = `(function () { | ||
| 303 | 304 | act.textContent = "Comment, tell, make it yours"; |
| 304 | 305 | act.target = "_blank"; |
| 305 | 306 | act.rel = "noopener noreferrer"; |
| 306 | act.style.cssText = "color:#8ab4ff;text-decoration:none;margin-top:4px"; | |
| 307 | act.style.cssText = LS + ";margin-top:4px"; | |
| 307 | 308 | try { |
| 308 | 309 | var dest = new URL(home); |
| 309 | 310 | dest.pathname = "/" + d.repo + "/toolbar"; |
test/apps-toolbar.test.ts +15
| @@ -684,3 +684,18 @@ describe("readPage", () => { | ||
| 684 | 684 | expect((await readPage(stream([" \nplain"]), 100, 1000)).whole).toBe(false); |
| 685 | 685 | }); |
| 686 | 686 | }); |
| 687 | ||
| 688 | describe("pill link tap targets (#8462)", () => { | |
| 689 | const js = toolbarJs; | |
| 690 | test("the Comment link carries the 44 px floor of LS", () => { | |
| 691 | expect(js).toContain('act.style.cssText = LS + ";margin-top:4px"'); | |
| 692 | expect(js).toContain("min-height:44px"); | |
| 693 | }); | |
Source-text match is brittle: it pins the exact spelling of the statement. Prefer rendering through the fake DOM and checking the Comment link style and the panel children. |
||
| 694 | test("Home is appended only after its href is set", () => { | |
| 695 | const set = js.indexOf("link.href = target.href"); | |
| 696 | const add = js.indexOf("panel.appendChild(link)"); | |
| 697 | expect(set).toBeGreaterThan(0); | |
| 698 | expect(add).toBeGreaterThan(set); | |
| 699 | expect(js).toContain("if (hasHome) panel.appendChild(link)"); | |
| 700 | }); | |
| 701 | }); |
new above
offline
Good: hasHome gates the append. Consider also making the no-home case explicit in a test that runs the script with a mismatched data-forge.