fix(screenshots): render light and dark in collapsible per-page sections

.github/workflows/screenshots-cleanup.yml

@@ -19,6 +19,8 @@ concurrency:
 
 jobs:
   cleanup:
+    # Disabled to avoid breaking images on older PRs. Flip to `true` to re-enable deletion.
+    if: false
     runs-on: ubuntu-latest
     steps:
       - name: Get otelbot token

.github/workflows/screenshots-commit.yml

@@ -121,13 +121,26 @@ jobs:
           base_url   = f"https://raw.githubusercontent.com/{repo}/otelbot/screenshots/{pr_number}"
           timestamp  = datetime.now(timezone.utc).strftime("%Y-%m-%d %H:%M UTC")
 
-          def table(slug):
+          VIEWPORTS = ("desktop", "tablet", "mobile")
+
+          def theme_table(slug, theme):
+              cells = " | ".join(
+                  f"![{slug} {vp} {theme}]({base_url}/{vp}-{theme}-{slug}.png)"
+                  for vp in VIEWPORTS
+              )
               return (
                   "| Desktop | Tablet | Mobile |\n"
                   "|---|---|---|\n"
-                  f"| ![{slug} desktop]({base_url}/desktop-{slug}.png)"
-                  f" | ![{slug} tablet]({base_url}/tablet-{slug}.png)"
-                  f" | ![{slug} mobile]({base_url}/mobile-{slug}.png) |"
+                  f"| {cells} |"
+              )
+
+          def page_section(label, slug):
+              return (
+                  f"<details>\n"
+                  f"<summary><strong>{label}</strong></summary>\n\n"
+                  f"**Light**\n\n{theme_table(slug, 'light')}\n\n"
+                  f"**Dark**\n\n{theme_table(slug, 'dark')}\n\n"
+                  f"</details>"
               )
 
           pages = [
@@ -140,9 +153,7 @@ jobs:
               ("Collector Detail",            "collector-detail"),
           ]
 
-          sections = "\n\n".join(
-              f"**{label}**\n\n{table(slug)}" for label, slug in pages
-          )
+          sections = "\n\n".join(page_section(label, slug) for label, slug in pages)
 
           footer = (
               f"_Captured from [`{short_sha}`]({commit_url}) on `{head_ref}` "

docs/screenshots-workflow.md

@@ -7,11 +7,11 @@ as an inline PR comment.
 
 The workflow is split across three files to support fork PRs safely:
 
-| File                      | Trigger                          | Token                                     | Responsibility                                                                 |
-| ------------------------- | -------------------------------- | ----------------------------------------- | ------------------------------------------------------------------------------ |
-| `screenshots-capture.yml` | `pull_request`                   | `contents: read`                          | Builds the app, captures screenshots, uploads artifacts                        |
-| `screenshots-commit.yml`  | `workflow_run` (after capture)   | `contents: write`, `pull-requests: write` | Downloads artifacts, commits to `otelbot/screenshots` branch, posts PR comment |
-| `screenshots-cleanup.yml` | `pull_request_target` (on close) | `contents: write`                         | Deletes the PR's subfolder from the `otelbot/screenshots` branch               |
+| File                      | Trigger                          | Token                                     | Responsibility                                                                                                                        |
+| ------------------------- | -------------------------------- | ----------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- |
+| `screenshots-capture.yml` | `pull_request`                   | `contents: read`                          | Builds the app, captures screenshots, uploads artifacts                                                                               |
+| `screenshots-commit.yml`  | `workflow_run` (after capture)   | `contents: write`, `pull-requests: write` | Downloads artifacts, commits to `otelbot/screenshots` branch, posts PR comment                                                        |
+| `screenshots-cleanup.yml` | `pull_request_target` (on close) | `contents: write`                         | Currently disabled (`if: false`). Originally deleted the PR's subfolder on close; retained as a no-op so old PR comments keep working |
 
 **Why two workflows for capture + commit?** GitHub restricts fork PRs from running workflows with
 write access. Splitting into a read-only capture phase and a write-capable commit phase (triggered
@@ -31,25 +31,32 @@ The layout is flat:
 ```text
 screenshots (branch)
 ├── 377/
-│   ├── desktop-home.png
-│   ├── tablet-home.png
-│   ├── mobile-home.png
-│   ├── desktop-instrumentation-list.png
+│   ├── desktop-light-home.png
+│   ├── desktop-dark-home.png
+│   ├── tablet-light-home.png
+│   ├── tablet-dark-home.png
+│   ├── mobile-light-home.png
+│   ├── mobile-dark-home.png
+│   ├── desktop-light-instrumentation-list.png
 │   └── ...
 ├── 381/
 │   └── ...
 └── ...
 ```
 
-The commit workflow creates the `otelbot/screenshots` branch automatically on first run. When a PR
-closes, the cleanup workflow deletes its subfolder and commits the removal.
+The commit workflow creates the `otelbot/screenshots` branch automatically on first run. The cleanup
+workflow is currently disabled (gated by `if: false`) so PR subfolders are retained indefinitely,
+which keeps image links in older PR comments alive. Flip the gate to `true` if deletion-on-close
+needs to come back.
 
 Raw content URLs follow the pattern:
 
 ```text
-https://raw.githubusercontent.com/open-telemetry/opentelemetry-ecosystem-explorer/otelbot/screenshots/<pr>/<viewport>-<page>.png
+https://raw.githubusercontent.com/open-telemetry/opentelemetry-ecosystem-explorer/otelbot/screenshots/<pr>/<viewport>-<theme>-<page>.png
 ```
 
+where `<theme>` is `light` or `dark`.
+
 These URLs are embedded directly in the PR comment as inline images.
 
 ## Triggering screenshots
@@ -69,17 +76,20 @@ const VIEWPORTS = [
 ];
 ```
 
-Add, remove, or resize entries there. The `name` field becomes the filename prefix
-(`desktop-home.png`) and the column header in the PR comment table.
+Add, remove, or resize entries there. The `name` field becomes the filename prefix and the column
+header in the PR comment table. Each viewport is captured twice (light + dark), yielding filenames
+like `desktop-light-home.png` and `desktop-dark-home.png`.
 
 ## Adding or removing pages
 
 Pages are captured sequentially inside the viewport loop in `take-screenshots.mjs`. To add a page,
 navigate to the URL and call `page.screenshot()` with an appropriate name. To remove a page, delete
 its navigation block.
 
-The PR comment table is built from a hardcoded `pages` list in the `Build comment body` step of
-`screenshots-commit.yml`. Update it to match any changes to the page set in the script.
+The PR comment renders one collapsible `<details>` block per page (closed by default), each
+containing a Light and a Dark viewport table. Both the page list and the per-page section are built
+from a hardcoded `pages` list in the `Build comment body` step of `screenshots-commit.yml`. Update
+it to match any changes to the page set in the script.
 
 ## Fork PR limitations
 

ecosystem-explorer/scripts/take-screenshots.mjs

@@ -49,6 +49,9 @@ const VIEWPORTS = [
   { name: "mobile", width: 390, height: 844 },
 ];
 
+// Themes captured for each page/viewport. Dark first because it's the default.
+const THEMES = ["dark", "light"];
+
 async function startServer() {
   return new Promise((resolve) => {
     const server = http.createServer((req, res) => {
@@ -139,7 +142,6 @@ async function takeScreenshots() {
 
     logTime("Launching browser...");
     browser = await chromium.launch({ headless: true });
-    const page = await browser.newPage();
 
     // Block external requests that can cause timeouts
     const BLOCKED_HOSTS = new Set([
@@ -148,7 +150,7 @@ async function takeScreenshots() {
       "fonts.googleapis.com",
       "fonts.gstatic.com",
     ]);
-    await page.route("**/*", (route) => {
+    const blockExternal = (route) => {
       try {
         const hostname = new URL(route.request().url()).hostname;
         if (
@@ -162,67 +164,78 @@ async function takeScreenshots() {
         // If URL parsing fails, allow the request
       }
       route.continue();
-    });
+    };
 
     logTime("Browser ready");
 
-    for (const viewport of VIEWPORTS) {
-      logTime(`Starting ${viewport.name} (${viewport.width}×${viewport.height})...`);
-      await page.setViewportSize({ width: viewport.width, height: viewport.height });
-      const p = (name) => path.join(SCREENSHOTS_DIR, `${viewport.name}-${name}.png`);
-
-      // 1. Home page
-      await page.goto(BASE_URL, { waitUntil: "domcontentloaded", timeout: 10000 });
-      await page.waitForSelector("h1", { state: "visible", timeout: 5000 });
-      await assertNoError(page, BASE_URL);
-      await page.screenshot({ path: p("home") });
-
-      // 2. Java agent instrumentation list
-      await page.goto(`${BASE_URL}/java-agent/instrumentation`, {
-        waitUntil: "domcontentloaded",
-        timeout: 10000,
-      });
-      await settle(page);
-      await assertNoError(page, `${BASE_URL}/java-agent/instrumentation`);
-      await page.screenshot({ path: p("instrumentation-list") });
-
-      // 3. Java agent instrumentation detail - Details tab
-      const detailUrl = `${BASE_URL}/java-agent/instrumentation/${DETAIL_VERSION}/${DETAIL_NAME}`;
-      await page.goto(detailUrl, { waitUntil: "domcontentloaded", timeout: 10000 });
-      await settle(page);
-      await assertNoError(page, detailUrl);
-      await page.screenshot({ path: p("detail-details"), fullPage: true });
-
-      // 4. Telemetry tab (skipped gracefully if tabs aren't present in this branch)
-      await clickTab(page, "Telemetry");
-      await assertNoError(page, detailUrl);
-      await page.screenshot({ path: p("detail-telemetry"), fullPage: true });
-
-      // 5. Configuration tab (skipped gracefully if tabs aren't present in this branch)
-      await clickTab(page, "Configuration");
-      await assertNoError(page, detailUrl);
-      await page.screenshot({ path: p("detail-configuration"), fullPage: true });
-
-      // 6. Collector list
-      await page.goto(`${BASE_URL}/collector/components`, {
-        waitUntil: "domcontentloaded",
-        timeout: 10000,
-      });
-      await settle(page);
-      await assertNoError(page, `${BASE_URL}/collector/components`);
-      await page.screenshot({ path: p("collector-list") });
-
-      // 7. Collector detail
-      const collectorDetailUrl = `${BASE_URL}/collector/components/${COLLECTOR_VERSION}/${COLLECTOR_DETAIL_ID}`;
-      await page.goto(collectorDetailUrl, {
-        waitUntil: "domcontentloaded",
-        timeout: 10000,
-      });
-      await settle(page);
-      await assertNoError(page, collectorDetailUrl);
-      await page.screenshot({ path: p("collector-detail"), fullPage: true });
+    for (const theme of THEMES) {
+      logTime(`Starting theme: ${theme}`);
+      const context = await browser.newContext({ colorScheme: theme });
+      const page = await context.newPage();
+      await page.route("**/*", blockExternal);
 
-      logTime(`${viewport.name} done`);
+      try {
+        for (const viewport of VIEWPORTS) {
+          logTime(`  ${theme} / ${viewport.name} (${viewport.width}×${viewport.height})...`);
+          await page.setViewportSize({ width: viewport.width, height: viewport.height });
+          const p = (name) => path.join(SCREENSHOTS_DIR, `${viewport.name}-${theme}-${name}.png`);
+
+          // 1. Home page
+          await page.goto(BASE_URL, { waitUntil: "domcontentloaded", timeout: 10000 });
+          await page.waitForSelector("h1", { state: "visible", timeout: 5000 });
+          await assertNoError(page, BASE_URL);
+          await page.screenshot({ path: p("home") });
+
+          // 2. Java agent instrumentation list
+          await page.goto(`${BASE_URL}/java-agent/instrumentation`, {
+            waitUntil: "domcontentloaded",
+            timeout: 10000,
+          });
+          await settle(page);
+          await assertNoError(page, `${BASE_URL}/java-agent/instrumentation`);
+          await page.screenshot({ path: p("instrumentation-list") });
+
+          // 3. Java agent instrumentation detail - Details tab
+          const detailUrl = `${BASE_URL}/java-agent/instrumentation/${DETAIL_VERSION}/${DETAIL_NAME}`;
+          await page.goto(detailUrl, { waitUntil: "domcontentloaded", timeout: 10000 });
+          await settle(page);
+          await assertNoError(page, detailUrl);
+          await page.screenshot({ path: p("detail-details"), fullPage: true });
+
+          // 4. Telemetry tab (skipped gracefully if tabs aren't present in this branch)
+          await clickTab(page, "Telemetry");
+          await assertNoError(page, detailUrl);
+          await page.screenshot({ path: p("detail-telemetry"), fullPage: true });
+
+          // 5. Configuration tab (skipped gracefully if tabs aren't present in this branch)
+          await clickTab(page, "Configuration");
+          await assertNoError(page, detailUrl);
+          await page.screenshot({ path: p("detail-configuration"), fullPage: true });
+
+          // 6. Collector list
+          await page.goto(`${BASE_URL}/collector/components`, {
+            waitUntil: "domcontentloaded",
+            timeout: 10000,
+          });
+          await settle(page);
+          await assertNoError(page, `${BASE_URL}/collector/components`);
+          await page.screenshot({ path: p("collector-list") });
+
+          // 7. Collector detail
+          const collectorDetailUrl = `${BASE_URL}/collector/components/${COLLECTOR_VERSION}/${COLLECTOR_DETAIL_ID}`;
+          await page.goto(collectorDetailUrl, {
+            waitUntil: "domcontentloaded",
+            timeout: 10000,
+          });
+          await settle(page);
+          await assertNoError(page, collectorDetailUrl);
+          await page.screenshot({ path: p("collector-detail"), fullPage: true });
+
+          logTime(`  ${theme} / ${viewport.name} done`);
+        }
+      } finally {
+        await context.close();
+      }
     }
 
     logTime("All screenshots completed successfully!");