diff --git a/.github/workflows/screenshots-cleanup.yml b/.github/workflows/screenshots-cleanup.yml index 1903573c..1ca501c7 100644 --- a/.github/workflows/screenshots-cleanup.yml +++ b/.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 diff --git a/.github/workflows/screenshots-commit.yml b/.github/workflows/screenshots-commit.yml index 73ece5b2..9fab8d09 100644 --- a/.github/workflows/screenshots-commit.yml +++ b/.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"
\n" + f"{label}\n\n" + f"**Light**\n\n{theme_table(slug, 'light')}\n\n" + f"**Dark**\n\n{theme_table(slug, 'dark')}\n\n" + f"
" ) 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}` " diff --git a/docs/screenshots-workflow.md b/docs/screenshots-workflow.md index 511702e5..ded7d026 100644 --- a/docs/screenshots-workflow.md +++ b/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//-.png +https://raw.githubusercontent.com/open-telemetry/opentelemetry-ecosystem-explorer/otelbot/screenshots//--.png ``` +where `` is `light` or `dark`. + These URLs are embedded directly in the PR comment as inline images. ## Triggering screenshots @@ -69,8 +76,9 @@ 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 @@ -78,8 +86,10 @@ Pages are captured sequentially inside the viewport loop in `take-screenshots.mj 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 `
` 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 diff --git a/ecosystem-explorer/scripts/take-screenshots.mjs b/ecosystem-explorer/scripts/take-screenshots.mjs index b09dbc56..0bc0092f 100644 --- a/ecosystem-explorer/scripts/take-screenshots.mjs +++ b/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!");