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 #8487

Merged opened this

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

Merged

scout/forge:scout/8462-every-link-in-the-forge-pill-is-a-44-px → main 1 commit fork

  • Merged. scout/8462-every-link-in-the-forge-pill-is-a-44-px was merged into main as 85f900d.

Review

Sign in to review this change.
2 files changed +19 −3
src/apps/toolbar.ts +4 −3
@@ -92,9 +92,10 @@ export const toolbarJs = `(function () {
9292 link.textContent = "Home";
9393 link.rel = "noopener";
9494 link.style.cssText = LS;
95 var hasHome = false;
9596 try {
9697 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; }
9899 } catch (e) {}
99100 // Tell the builders: one box, one button, no account. The note goes to
100101 // this app's own host (same-origin, so the sender's cookie goes along) and
@@ -295,7 +296,7 @@ export const toolbarJs = `(function () {
295296 var crew = document.createElement("div");
296297 crew.setAttribute("aria-live", "polite");
297298 panel.appendChild(crew);
298 panel.appendChild(link);
299 if (hasHome) panel.appendChild(link);
299300 // The actions live on a forge page, opened in a tab of its own: the bar
300301 // posts nothing and reads nothing of this page beyond its address.
301302 if (d.repo && /^[\\w.-]+\\/[\\w.-]+$/.test(d.repo)) {
@@ -303,7 +304,7 @@ export const toolbarJs = `(function () {
303304 act.textContent = "Comment, tell, make it yours";
304305 act.target = "_blank";
305306 act.rel = "noopener noreferrer";
306 act.style.cssText = "color:#8ab4ff;text-decoration:none;margin-top:4px";
307 act.style.cssText = LS + ";margin-top:4px";
307308 try {
308309 var dest = new URL(home);
309310 dest.pathname = "/" + d.repo + "/toolbar";
test/apps-toolbar.test.ts +15
@@ -684,3 +684,18 @@ describe("readPage", () => {
684684 expect((await readPage(stream([" \nplain"]), 100, 1000)).whole).toBe(false);
685685 });
686686 });
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 });
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 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs