chore: upgrade capybara-screenshot-diff to v1.12.0#279
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSwaps the snap-diff gem source and rewrites snapshot test setup (animation handling, thresholds, HTML reporter), tightens Capybara waits, adds a Hugo composite action with caching, consolidates CI test workflows (removes old unit workflow, adds consolidated test workflow), and bumps CI Ruby to 4.0. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Gemfile`:
- Line 17: The GitHub-sourced gem declaration for capybara-screenshot-diff is
not pinned to a release, which breaks reproducible installs; update the Gemfile
entry for the gem (the line starting with gem "capybara-screenshot-diff",
github: "snap-diff/snap_diff-capybara") to include the release identifier by
adding tag: "v1.12.0" (or ref: with the corresponding commit SHA) so Bundler
resolves the specific v1.12.0 release instead of the default branch.
In `@test/support/setup_snap_diff.rb`:
- Around line 3-5: The comment block documents a stale env var
SCREENSHOT_TOLERANCE that the code no longer reads; remove the
SCREENSHOT_TOLERANCE line from the header comment so only active settings (e.g.,
SCREENSHOT_STABILITY_TIME) are listed, ensuring the documentation matches the
behavior in setup_snap_diff.rb.
- Around line 19-20: Update the global visual-diff threshold by changing the
Capybara::Screenshot::Diff.perceptual_threshold assignment from 2.0 to the
documented baseline (use 0.005) so the global setting aligns with
SCREENSHOT_TOLERANCE; edit the Capybara::Screenshot::Diff.perceptual_threshold
line to set 0.005 instead of 2.0.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 72796274-2f99-4ea2-b191-771320b39e8c
⛔ Files ignored due to path filters (6)
Gemfile.lockis excluded by!**/*.locktest/fixtures/screenshots/macos/pages/careers/benefits-section.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/pages/careers/benefits-spacing.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/pages/careers/careers-page-full.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/pages/careers/feature-cards-section.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/pages/careers/newsletter-section.pngis excluded by!**/*.png
📒 Files selected for processing (2)
Gemfiletest/support/setup_snap_diff.rb
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/fixtures/screenshots/snap_diff_report.html (1)
186-203: Add ARIA pressed-state semantics to toggleable controls.View buttons and the annotation toggle expose only visual active state. Add
aria-pressedso screen readers get the same state.♿ Minimal accessibility improvement
-<button class="tbtn active" data-view="both" title="Both (1)"> +<button class="tbtn active" data-view="both" title="Both (1)" aria-label="Both view" aria-pressed="true"> -<button class="tbtn" data-view="base" title="Base only (2)"> +<button class="tbtn" data-view="base" title="Base only (2)" aria-label="Base view" aria-pressed="false"> -<button class="tbtn" data-view="new" title="New only (3)"> +<button class="tbtn" data-view="new" title="New only (3)" aria-label="New view" aria-pressed="false"> -<button class="tbtn" data-view="heatmap" title="Heatmap (4)"> +<button class="tbtn" data-view="heatmap" title="Heatmap (4)" aria-label="Heatmap view" aria-pressed="false"> -<button class="toggle-btn" id="annotate-toggle" title="Show annotated versions (A)"> +<button class="toggle-btn" id="annotate-toggle" title="Show annotated versions (A)" aria-pressed="false"> - viewBtns.forEach(function(b) { b.classList.toggle('active', b.dataset.view === state.view); }); + viewBtns.forEach(function(b) { + var isActive = b.dataset.view === state.view; + b.classList.toggle('active', isActive); + b.setAttribute('aria-pressed', isActive ? 'true' : 'false'); + }); annotateBtn.classList.toggle('active', state.annotated); + annotateBtn.setAttribute('aria-pressed', state.annotated ? 'true' : 'false');Also applies to: 329-331
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/fixtures/screenshots/snap_diff_report.html` around lines 186 - 203, The view buttons (elements with class "tbtn" and data-view attributes) and the annotation toggle (id "annotate-toggle") only have visual active state; add ARIA pressed-state semantics by setting aria-pressed="true" on the initially active control and aria-pressed="false" on the others, and update the JS that toggles the 'active' class to also set aria-pressed appropriately (when toggling a tbtn or the annotate-toggle, flip the aria-pressed boolean to match the active state). Ensure initial markup reflects the current active state (e.g., the button with class "tbtn active" gets aria-pressed="true") and any event handlers that change class names also update aria-pressed to keep screen readers in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/fixtures/screenshots/snap_diff_report.html`:
- Around line 9-25: Update the theme tokens to match JetVelocity colors by
replacing --accent with `#a855f7` and --red with `#cc342d`, and adjust the derived
tokens --accent-dim, --accent-bg, and --red-dim to use semi-transparent variants
of those new hex values (e.g., appropriate rgba(...) with similar alpha used
previously) so the CSS variables --accent, --accent-dim, --accent-bg, --red, and
--red-dim reflect the required neon purple and Ruby red palette.
- Line 430: The input handler attached in searchEl.addEventListener updates
state.search and rebuilds filtered results but skips render() when
filtered.length is 0, leaving the main viewer/top counter stale; modify the
handler (the closure calling rebuildFiltered(), buildSidebar(), render()) to
explicitly handle the zero-match case by still invoking the UI update path
(e.g., call a dedicated update function or ensure render() runs with an empty
dataset) and reset/clear the main viewer and top counter when filtered.length
=== 0 so the sidebar and main view stay consistent (refer to
searchEl.addEventListener, state.search, rebuildFiltered, buildSidebar, render,
and the filtered variable).
---
Nitpick comments:
In `@test/fixtures/screenshots/snap_diff_report.html`:
- Around line 186-203: The view buttons (elements with class "tbtn" and
data-view attributes) and the annotation toggle (id "annotate-toggle") only have
visual active state; add ARIA pressed-state semantics by setting
aria-pressed="true" on the initially active control and aria-pressed="false" on
the others, and update the JS that toggles the 'active' class to also set
aria-pressed appropriately (when toggling a tbtn or the annotate-toggle, flip
the aria-pressed boolean to match the active state). Ensure initial markup
reflects the current active state (e.g., the button with class "tbtn active"
gets aria-pressed="true") and any event handlers that change class names also
update aria-pressed to keep screen readers in sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 976a7e42-dc8b-4d90-a2f4-0a64c725183d
⛔ Files ignored due to path filters (3)
Gemfile.lockis excluded by!**/*.locktest/fixtures/screenshots/macos/debug_test.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/homepage.pngis excluded by!**/*.png
📒 Files selected for processing (2)
.gitignoretest/fixtures/screenshots/snap_diff_report.html
✅ Files skipped from review due to trivial changes (1)
- .gitignore
pftg
force-pushed
the
chore/update-capybara-screenshot-diff
branch
from
2eaaa0b to
38bfe82
Compare
Member
Author
|
CodeRabbit findings triaged:
|
Contributor
Screenshot diffs detected
|
pftg
force-pushed
the
chore/update-capybara-screenshot-diff
branch
13 times, most recently
from
e972a27 to
9698b01
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
39-44: Make PR commenting conditional on PR-triggered runs.Line 44 hard-codes
pr-comment: 'true'; onworkflow_dispatch, there may be no PR context for commenting.Suggested patch
- name: Upload screenshot report if: failure() uses: snap-diff/snap_diff-capybara/.github/actions/upload-screenshots@master with: name: screenshots report-path: test/fixtures/screenshots - pr-comment: 'true' + pr-comment: ${{ github.event_name == 'pull_request' && 'true' || 'false' }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 39 - 44, The workflow currently always sets pr-comment: 'true' for the snap-diff action (uses: snap-diff/snap_diff-capybara/.github/actions/upload-screenshots@master), which will attempt to comment even on workflow_dispatch runs without a PR; change the pr-comment input to be conditional on the event by using a GitHub Actions expression that evaluates to true only for pull_request events (e.g., set pr-comment to an expression referencing github.event_name == 'pull_request'), so the action only posts PR comments when a PR context exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/setup-hugo/action.yml:
- Line 40: The cache key for Hugo (the key: line using hashFiles(...)) omits
dependency lock/manifest files; update the hashFiles call used in the key
expression to include package.json and bun.lockb so dependency changes affect
the cache key, e.g. add 'package.json' and 'bun.lockb' to the list inside the
hashFiles(...) expression referenced by the existing key: entry.
In @.github/workflows/test.yml:
- Line 26: The workflow uses mutable refs for external actions; update both
occurrences of
snap-diff/snap_diff-capybara/.github/actions/setup-ruby-and-dependencies@master
and snap-diff/snap_diff-capybara/.github/actions/upload-screenshots@master to
use immutable commit SHAs: locate the two action usages in the workflow and
replace the `@master` tag with the exact commit SHA for each action (obtain the
SHA from the corresponding repository commit you trust), commit the updated
workflow, and verify the workflow runs successfully.
In `@test/system/desktop_site_test.rb`:
- Around line 22-23: Replace the non-asserting probe call has_css?(".hero,
[data-section=hero], main", wait: 2) with an assertion so the test fails if the
hero never appears; use an asserting helper such as assert_selector or assert
has_css? with the same selector and wait option (referencing the selector
".hero, [data-section=hero], main") to enforce the wait in the test.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 39-44: The workflow currently always sets pr-comment: 'true' for
the snap-diff action (uses:
snap-diff/snap_diff-capybara/.github/actions/upload-screenshots@master), which
will attempt to comment even on workflow_dispatch runs without a PR; change the
pr-comment input to be conditional on the event by using a GitHub Actions
expression that evaluates to true only for pull_request events (e.g., set
pr-comment to an expression referencing github.event_name == 'pull_request'), so
the action only posts PR comments when a PR context exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f2c67b59-eeca-4ec0-8a57-2ee778b600d9
⛔ Files ignored due to path filters (6)
Gemfile.lockis excluded by!**/*.locktest/fixtures/screenshots/macos/pages/careers/benefits-section.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/pages/careers/benefits-spacing.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/pages/careers/careers-page-full.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/pages/careers/feature-cards-section.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/pages/careers/newsletter-section.pngis excluded by!**/*.png
📒 Files selected for processing (9)
.github/actions/setup-hugo/action.yml.github/workflows/sync-and-publish.yml.github/workflows/test-unit.yml.github/workflows/test.yml.gitignoreGemfiletest/support/setup_capybara.rbtest/support/setup_snap_diff.rbtest/system/desktop_site_test.rb
💤 Files with no reviewable changes (1)
- .github/workflows/test-unit.yml
✅ Files skipped from review due to trivial changes (3)
- .gitignore
- test/support/setup_capybara.rb
- Gemfile
🚧 Files skipped from review as they are similar to previous changes (1)
- test/support/setup_snap_diff.rb
| steps: | ||
| - uses: actions/checkout@v6 | ||
|
|
||
| - uses: snap-diff/snap_diff-capybara/.github/actions/setup-ruby-and-dependencies@master |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/test.yml | head -50Repository: jetthoughts/jetthoughts.github.io
Length of output: 1369
🏁 Script executed:
grep -n "@" .github/workflows/test.yml | grep "uses:"Repository: jetthoughts/jetthoughts.github.io
Length of output: 298
Pin external action refs to immutable commit SHAs instead of @master.
Lines 26 and 40 reference moving branches which can break reproducibility and increase supply-chain risk. Replace @master with specific commit SHAs for both setup-ruby-and-dependencies and upload-screenshots actions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/test.yml at line 26, The workflow uses mutable refs for
external actions; update both occurrences of
snap-diff/snap_diff-capybara/.github/actions/setup-ruby-and-dependencies@master
and snap-diff/snap_diff-capybara/.github/actions/upload-screenshots@master to
use immutable commit SHAs: locate the two action usages in the workflow and
replace the `@master` tag with the exact commit SHA for each action (obtain the
SHA from the corresponding repository commit you trust), commit the updated
workflow, and verify the workflow runs successfully.
1a7fdb5 to
065f977
Compare
- Switch gem source to snap-diff/snap_diff-capybara - Enable built-in disable_animations (removes 45-line custom module) - Switch to perceptual_threshold: 2.0 (dE00) for anti-aliasing - Enable Web UI reporter for interactive diff review - Add snap_diff_report.html to .gitignore - Re-record careers page baselines Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pftg
force-pushed
the
chore/update-capybara-screenshot-diff
branch
from
065f977 to
fa79ef4
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/support/setup_snap_diff.rb (1)
18-19: Consider adding a clarifying comment for the dE00 unit.The value 2.0 is correct for delta E 2000 color distance (not percentage). Adding a brief inline comment could prevent future confusion since the unit isn't obvious from the code alone.
📝 Optional: Add unit clarification
Capybara::Screenshot::Diff.driver = :vips -Capybara::Screenshot::Diff.perceptual_threshold = 2.0 +Capybara::Screenshot::Diff.perceptual_threshold = 2.0 # dE00 color distance; handles anti-aliasing variance Capybara::Screenshot::Diff.delayed = true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/support/setup_snap_diff.rb` around lines 18 - 19, Add a brief inline comment explaining that Capybara::Screenshot::Diff.perceptual_threshold = 2.0 uses delta E 2000 (dE00) units rather than a percentage; update the line where perceptual_threshold is set (Capybara::Screenshot::Diff.perceptual_threshold) to include a short clarifying comment like “# dE00 (delta E 2000) color distance” so future readers understand the unit expected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/support/setup_snap_diff.rb`:
- Around line 18-19: Add a brief inline comment explaining that
Capybara::Screenshot::Diff.perceptual_threshold = 2.0 uses delta E 2000 (dE00)
units rather than a percentage; update the line where perceptual_threshold is
set (Capybara::Screenshot::Diff.perceptual_threshold) to include a short
clarifying comment like “# dE00 (delta E 2000) color distance” so future readers
understand the unit expected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de5d67d3-bacf-4758-9372-0beeff541f0d
⛔ Files ignored due to path filters (6)
Gemfile.lockis excluded by!**/*.locktest/fixtures/screenshots/macos/pages/careers/benefits-section.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/pages/careers/benefits-spacing.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/pages/careers/careers-page-full.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/pages/careers/feature-cards-section.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/pages/careers/newsletter-section.pngis excluded by!**/*.png
📒 Files selected for processing (9)
.github/actions/setup-hugo/action.yml.github/workflows/sync-and-publish.yml.github/workflows/test-unit.yml.github/workflows/test.yml.gitignoreGemfiletest/support/setup_capybara.rbtest/support/setup_snap_diff.rbtest/system/desktop_site_test.rb
💤 Files with no reviewable changes (1)
- .github/workflows/test-unit.yml
✅ Files skipped from review due to trivial changes (4)
- .gitignore
- .github/workflows/sync-and-publish.yml
- test/support/setup_capybara.rb
- .github/actions/setup-hugo/action.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- Gemfile
- .github/workflows/test.yml
Member
Author
CodeRabbit findings triaged
|
pftg
force-pushed
the
chore/update-capybara-screenshot-diff
branch
from
3e92f16 to
befe154
Compare
pftg
force-pushed
the
chore/update-capybara-screenshot-diff
branch
from
befe154 to
f37ef3c
Compare
pftg
force-pushed
the
chore/update-capybara-screenshot-diff
branch
from
f37ef3c to
cf96730
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
disable_animations(removes 25-line custom ScreenshotPreparation module)perceptual_threshold: 2.0(dE00 color distance) for anti-aliasing tolerancerequire 'capybara_screenshot_diff/reporters/html')What's new in v1.12.0
disable_animationshelper (replaces manual CSS injection)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Tests