You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@magpie.apache.org by po...@apache.org on 2026/09/17 13:37:12 UTC

(magpie-site) 05/16: feat(preview): region clamping, caption text and diff-line URL resolution

This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/magpie-site.git

commit 08d163ca2498e070c4d1246caa1bbf512a8db12b
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Thu Sep 17 13:34:38 2026 +0200

    feat(preview): region clamping, caption text and diff-line URL resolution
    
    Generated-by: Claude Opus 5
---
 scripts/preview/overlay/logic.mjs      | 43 +++++++++++++++++++++
 scripts/preview/overlay/logic.test.mjs | 69 ++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+)

diff --git a/scripts/preview/overlay/logic.mjs b/scripts/preview/overlay/logic.mjs
new file mode 100644
index 0000000..809c72f
--- /dev/null
+++ b/scripts/preview/overlay/logic.mjs
@@ -0,0 +1,43 @@
+const MIN_SIDE = 12;
+
+/** Normalise a drag into a viewport-clipped rectangle, or null if it is a stray click. */
+export function clampRegion({ x1, y1, x2, y2 }, viewport) {
+  const x = Math.max(0, Math.min(x1, x2));
+  const y = Math.max(0, Math.min(y1, y2));
+  const w = Math.min(viewport.w, Math.max(x1, x2)) - x;
+  const h = Math.min(viewport.h, Math.max(y1, y2)) - y;
+
+  if (w < MIN_SIDE || h < MIN_SIDE) return null;
+  return { x, y, w, h };
+}
+
+/**
+ * Burned into the image rather than written beside it: a caption survives being
+ * dragged into a comment, quoted or downloaded, where a separate line would not.
+ */
+export function captionFor({ url, source, region, sha }) {
+  const where = source ?? "source not resolved";
+  return `${url} — ${where} — ${region.w}×${region.h} at (${region.x},${region.y}) — built from ${sha}`;
+}
+
+/**
+ * The Files tab anchored at the marked line when that line is part of the diff,
+ * and the Conversation tab otherwise. It never guesses a line.
+ */
+export function targetUrl({ repo, pr, source, anchors }) {
+  const conversation = `https://github.com/${repo}/pull/${pr}`;
+  if (!source || !anchors) return conversation;
+
+  const match = /^(.*):(\d+)$/.exec(source);
+  if (!match) return conversation;
+
+  const [, file, lineText] = match;
+  const line = Number(lineText);
+  const entry = anchors[file];
+  if (!entry?.anchor) return conversation;
+
+  const inDiff = (entry.ranges ?? []).some(([from, to]) => line >= from && line <= to);
+  if (!inDiff) return conversation;
+
+  return `https://github.com/${repo}/pull/${pr}/files#${entry.anchor}R${line}`;
+}
diff --git a/scripts/preview/overlay/logic.test.mjs b/scripts/preview/overlay/logic.test.mjs
new file mode 100644
index 0000000..5201d59
--- /dev/null
+++ b/scripts/preview/overlay/logic.test.mjs
@@ -0,0 +1,69 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { clampRegion, captionFor, targetUrl } from "./logic.mjs";
+
+const viewport = { w: 1000, h: 800 };
+
+test("clampRegion normalises a drag in any direction", () => {
+  assert.deepEqual(clampRegion({ x1: 300, y1: 200, x2: 100, y2: 50 }, viewport), {
+    x: 100, y: 50, w: 200, h: 150,
+  });
+});
+
+test("clampRegion clips to the viewport", () => {
+  assert.deepEqual(clampRegion({ x1: -50, y1: -20, x2: 1200, y2: 900 }, viewport), {
+    x: 0, y: 0, w: 1000, h: 800,
+  });
+});
+
+test("clampRegion rejects a region too small to be deliberate", () => {
+  assert.equal(clampRegion({ x1: 10, y1: 10, x2: 14, y2: 14 }, viewport), null);
+});
+
+test("captionFor names the page, the source and the build", () => {
+  const caption = captionFor({
+    url: "https://magpie-pr180.staged.apache.org/tools",
+    source: "src/components/landing/SiteFooter.tsx:72",
+    region: { x: 240, y: 1150, w: 420, h: 180 },
+    sha: "14fdc13",
+  });
+
+  assert.match(caption, /magpie-pr180\.staged\.apache\.org\/tools/);
+  assert.match(caption, /SiteFooter\.tsx:72/);
+  assert.match(caption, /420×180/);
+  assert.match(caption, /14fdc13/);
+});
+
+test("captionFor says so when no source was resolved", () => {
+  const caption = captionFor({
+    url: "https://x/",
+    source: null,
+    region: { x: 0, y: 0, w: 10, h: 10 },
+    sha: "abc1234",
+  });
+  assert.match(caption, /source not resolved/i);
+});
+
+test("targetUrl anchors on the diff line when the line is in the diff", () => {
+  const anchors = { "a.tsx": { anchor: "diff-deadbeef", ranges: [[10, 20]] } };
+  assert.equal(
+    targetUrl({ repo: "apache/magpie-site", pr: 180, source: "a.tsx:12", anchors }),
+    "https://github.com/apache/magpie-site/pull/180/files#diff-deadbeefR12",
+  );
+});
+
+test("targetUrl falls back to the conversation for a line outside the diff", () => {
+  const anchors = { "a.tsx": { anchor: "diff-deadbeef", ranges: [[10, 20]] } };
+  assert.equal(
+    targetUrl({ repo: "apache/magpie-site", pr: 180, source: "a.tsx:99", anchors }),
+    "https://github.com/apache/magpie-site/pull/180",
+  );
+});
+
+test("targetUrl falls back when the file, the manifest or the source is missing", () => {
+  const base = { repo: "apache/magpie-site", pr: 180 };
+  const conversation = "https://github.com/apache/magpie-site/pull/180";
+  assert.equal(targetUrl({ ...base, source: "other.tsx:1", anchors: {} }), conversation);
+  assert.equal(targetUrl({ ...base, source: "a.tsx:1", anchors: null }), conversation);
+  assert.equal(targetUrl({ ...base, source: null, anchors: {} }), conversation);
+});