-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(ui): add tabbed failure view for toMatchScreenshot with comparison slider
#8813
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
Conversation
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.
I like the direction of this, but I wonder if instead of adding a separate artifact entity, we can just extend attachments with additional fields, but have them as @private? The two share a lot of similarities and it's hard to justify the separation, to be honest.
I feel like in the future we could also make the API public, the metadata seems useful
This comment was marked as duplicate.
This comment was marked as duplicate.
|
Can't all of the new required fields just go into an |
b5ca8a2 to
0ec8536
Compare
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
9287be4 to
c239bf0
Compare
b9660bf to
ddaa9a8
Compare
| optimizeDeps: { | ||
| include: ['vue-router', 'splitpanes', 'd3-graph-controller', 'vue-virtual-scroller'], | ||
| }, |
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.
Not entirely sure about this, but without it Vite would trigger a reload and hang the test until the pipeline reaches the 30m timeout: https://github.com/vitest-dev/vitest/actions/runs/19538883275/job/55939684333?pr=8813
sheremet-va
left a comment
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.
Found a bug. I was using expect-dom/toMatchScreenshot to test this, not sure if it was a good idea, but if you click between different tests, the screenshots are lost:
Screen.Recording.2025-11-21.at.17.37.21.mov
672795d to
6c31ddd
Compare
|
Should be fixed now @sheremet-va. Noticed Vue reused the node when switching test results so "Reference" moved to the left, that should be fixed now too. |
6c31ddd to
83f75cc
Compare
|
Thank you! |
Description
This PR adds a new failure view for
toMatchScreenshotin the report panel. The view displays diff, reference, and actual screenshots in separate tabs, with a slider tab for side-by-side comparison. Under the hood, this feature is implemented using the new artifacts API.Preview:
slider-demo-h264.mp4
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yamlunless you introduce a new test example.Tests
pnpm test:ci.Documentation
pnpm run docscommand.Changesets
feat:,fix:,perf:,docs:, orchore:.