-
Notifications
You must be signed in to change notification settings - Fork 25
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
US152268 - Migrate async-container-mixin and typography tests #3937
US152268 - Migrate async-container-mixin and typography tests #3937
Conversation
mixins/*/test/screenshots/current/ | ||
mixins/*/test/screenshots/golden/ |
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.
async-container-mixin
is the only vdiff
test in mixins
so we can clean this up now. You'll need to clean up any lingering local copies next time you pull after this merges.
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
|
@@ -0,0 +1,54 @@ | |||
import '../typography.js'; |
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.
So including this is a little bit conflict-y with what vdiff is already including. It's probably fine, but we'll need to be careful to keep them in sync if we ever publish new versions of those assets to the CDN.
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.
Yeah I can't really think of a way around it to still test the typography
code, unless we isolated the typography tests into their own job and created a custom testRunnerHtml
for them?
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.
Yeah doesn't seem worth the trouble since this is working.
Co-authored-by: github-actions <[email protected]>
🎉 This PR is included in version 2.141.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Nothing super interesting about the typography migration.
For the
async-container-mixin
migration, I modified the existingdemo/async-item.js
component so thattest/async-item.js
could be deleted. The result is that two of the diffs are a slightly bigger version of themselves now, because the demo size was100px
and the test size was50px
.Final report: https://vdiff.d2l.dev/BrightspaceUI/core/dfedfcf7f988a2343d7d800b25a04108255c65d0/2023-08-28_19-26-07/report/