Repository navigation
feat(frontend): add visual diff tooling for UI PR reviews #2576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,145 @@ | ||||||
| name: Visual diff | ||||||
|
|
||||||
| concurrency: | ||||||
| group: visual-diff-${{ github.event.pull_request.number || github.sha }} | ||||||
| cancel-in-progress: true | ||||||
|
|
||||||
| on: | ||||||
| pull_request: | ||||||
| types: | ||||||
| - opened | ||||||
| synchronize | ||||||
| reopened | ||||||
| labeled | ||||||
| unlabeled | ||||||
| edited | ||||||
|
|
||||||
| permissions: | ||||||
| contents: read | ||||||
| pull-requests: write | ||||||
|
|
||||||
| jobs: | ||||||
| check: | ||||||
| runs-on: ubuntu-latest | ||||||
| outputs: | ||||||
| requested: ${{ steps.check.outputs.requested }} | ||||||
| steps: | ||||||
| - name: Detect visual diff request | ||||||
| id: check | ||||||
| env: | ||||||
| PR_LABELS: ${{ toJson(github.event.pull_request.labels) }} | ||||||
| PR_BODY: ${{ github.event.pull_request.body }} | ||||||
| run: | | ||||||
| requested=false | ||||||
| if printf '%s' "$PR_LABELS" | grep -q '"name": "visual-change"'; then | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verifies current vs hardened regex against compact and spaced JSON forms.
compact='[{"name":"visual-change"}]'
spaced='[{ "name": "visual-change" }]'
echo "$compact" | grep -q '"name": "visual-change"' && echo "current matched compact" || echo "current FAILED compact"
echo "$spaced" | grep -q '"name": "visual-change"' && echo "current matched spaced" || echo "current FAILED spaced"
echo "$compact" | grep -Eq '"name"[[:space:]]*:[[:space:]]*"visual-change"' && echo "hardened matched compact" || echo "hardened FAILED compact"
echo "$spaced" | grep -Eq '"name"[[:space:]]*:[[:space:]]*"visual-change"' && echo "hardened matched spaced" || echo "hardened FAILED spaced"Repository: Cap-go/capgo Length of output: 245 🏁 Script executed: #!/bin/bash
# Check if the file exists and inspect the relevant section
if [ -f .github/workflows/visual-diff.yml ]; then
echo "File found. Showing lines around line 34:"
cat -n .github/workflows/visual-diff.yml | sed -n '25,45p'
else
echo "File not found at expected location"
find . -name "visual-diff.yml" 2>/dev/null
fiRepository: Cap-go/capgo Length of output: 1068 Fix label detection to handle both compact and spaced JSON formats from The current pattern on line 34 only matches spaced JSON ( Suggested fix- if printf '%s' "$PR_LABELS" | grep -q '"name": "visual-change"'; then
+ if printf '%s' "$PR_LABELS" | grep -Eq '"name"[[:space:]]*:[[:space:]]*"visual-change"'; then
requested=true📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| requested=true | ||||||
| elif printf '%s' "$PR_BODY" | grep -q '<!-- visual-diff:required -->'; then | ||||||
| requested=true | ||||||
| fi | ||||||
| echo "requested=${requested}" >> "$GITHUB_OUTPUT" | ||||||
|
|
||||||
| visual-diff: | ||||||
| needs: check | ||||||
| if: needs.check.outputs.requested == 'true' | ||||||
| runs-on: ubuntu-latest | ||||||
| timeout-minutes: 45 | ||||||
| name: Generate visual diff report | ||||||
| steps: | ||||||
| - name: Setup Node.js | ||||||
| uses: actions/setup-node@v6 | ||||||
| with: | ||||||
| node-version: 24.x | ||||||
|
|
||||||
| - name: Checkout repository | ||||||
| uses: actions/checkout@v6 | ||||||
| with: | ||||||
| fetch-depth: 0 | ||||||
|
|
||||||
| - name: Setup bun | ||||||
| run: bash scripts/setup-bun.sh | ||||||
|
|
||||||
| - name: Install dependencies | ||||||
| run: bun install | ||||||
|
|
||||||
| - name: Install Playwright browser | ||||||
| run: bunx playwright install --with-deps chromium | ||||||
|
|
||||||
| - name: Install Supabase CLI | ||||||
| uses: supabase/setup-cli@v2 | ||||||
| with: | ||||||
| version: 2.84.2 | ||||||
|
|
||||||
| - name: Link Supabase templates | ||||||
| run: ln -sfn supabase/templates templates | ||||||
|
|
||||||
| - name: Run visual diff pipeline | ||||||
| run: | | ||||||
| bun scripts/visual-diff.ts run \ | ||||||
| --base "${{ github.event.pull_request.base.sha }}" \ | ||||||
| --head "${{ github.event.pull_request.head.sha }}" | ||||||
|
|
||||||
| - name: Upload visual diff report | ||||||
| if: always() | ||||||
| uses: actions/upload-artifact@v6 | ||||||
| with: | ||||||
| name: visual-diff-report-${{ github.event.pull_request.head.sha }} | ||||||
| path: .context/visual-diff/report/ | ||||||
| if-no-files-found: ignore | ||||||
| retention-days: 14 | ||||||
|
|
||||||
| - name: Comment visual diff summary on PR | ||||||
| if: always() | ||||||
| uses: actions/github-script@v8 | ||||||
| env: | ||||||
| HEAD_SHA: ${{ github.event.pull_request.head.sha }} | ||||||
| JOB_STATUS: ${{ job.status }} | ||||||
| RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} | ||||||
| ARTIFACT_NAME: visual-diff-report-${{ github.event.pull_request.head.sha }} | ||||||
| with: | ||||||
| script: | | ||||||
| const fs = require('node:fs') | ||||||
| const path = require('node:path') | ||||||
| const marker = '<!-- capgo-visual-diff -->' | ||||||
| const issue_number = context.payload.pull_request.number | ||||||
| const summaryPath = path.join(process.env.GITHUB_WORKSPACE, '.context/visual-diff/report/summary.md') | ||||||
| const ok = process.env.JOB_STATUS === 'success' | ||||||
| const summary = fs.existsSync(summaryPath) | ||||||
| ? fs.readFileSync(summaryPath, 'utf8').trim() | ||||||
| : 'Visual diff did not produce a report. Check the workflow logs and artifacts.' | ||||||
|
|
||||||
| const body = [ | ||||||
| marker, | ||||||
| `## Visual diff ${ok ? 'passed' : 'failed'}`, | ||||||
| '', | ||||||
| summary, | ||||||
| '', | ||||||
| `Commit: \`${process.env.HEAD_SHA}\``, | ||||||
| `[Download the HTML report from workflow artifacts](${process.env.RUN_URL}) (artifact: \`${process.env.ARTIFACT_NAME}\`).`, | ||||||
| '', | ||||||
| 'Open `index.html` from the artifact for side-by-side before/after/diff screenshots.', | ||||||
| ].join('\n') | ||||||
|
|
||||||
| const { owner, repo } = context.repo | ||||||
| const comments = await github.paginate(github.rest.issues.listComments, { | ||||||
| owner, | ||||||
| repo, | ||||||
| issue_number, | ||||||
| per_page: 100, | ||||||
| }) | ||||||
| const existing = comments.find(comment => comment.body?.includes(marker)) | ||||||
| if (existing) { | ||||||
| await github.rest.issues.updateComment({ | ||||||
| owner, | ||||||
| repo, | ||||||
| comment_id: existing.id, | ||||||
| body, | ||||||
| }) | ||||||
| } | ||||||
| else { | ||||||
| await github.rest.issues.createComment({ | ||||||
| owner, | ||||||
| repo, | ||||||
| issue_number, | ||||||
| body, | ||||||
| }) | ||||||
| } | ||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| export interface VisualDiffRoute { | ||
| slug: string | ||
| path: string | ||
| /** When true, logs in as test@capgo.app before visiting the route. */ | ||
| auth?: boolean | ||
| } | ||
|
|
||
| /** | ||
| * Console pages captured for before/after visual diffs. | ||
| * Add routes here when a PR touches a new screen reviewers should compare. | ||
| */ | ||
| export const visualDiffRoutes: VisualDiffRoute[] = [ | ||
| { slug: 'login', path: '/login/', auth: false }, | ||
| { slug: 'dashboard', path: '/dashboard', auth: true }, | ||
| { slug: 'apps', path: '/apps', auth: true }, | ||
| { slug: 'app-overview', path: '/app/com.demo.app', auth: true }, | ||
| { slug: 'channels', path: '/app/com.demo.app/channels', auth: true }, | ||
| { slug: 'devices', path: '/app/com.demo.app/devices', auth: true }, | ||
| ] | ||
|
|
||
| export const visualDiffViewport = { | ||
| width: 1280, | ||
| height: 720, | ||
| } as const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix invalid YAML list syntax in
pull_request.types(workflow currently won’t parse).Line 11 onward uses bare scalars instead of list items, which breaks YAML parsing and prevents the workflow from loading.
Suggested fix
on: pull_request: types: - opened - synchronize - reopened - labeled - unlabeled - edited + - synchronize + - reopened + - labeled + - unlabeled + - edited📝 Committable suggestion
🧰 Tools
🪛 actionlint (1.7.12)
[error] 11-11: could not parse as YAML: could not find expected ':'
(syntax-check)
🪛 YAMLlint (1.37.1)
[error] 12-12: syntax error: could not find expected ':'
(syntax)
🤖 Prompt for AI Agents
Source: Linters/SAST tools