Skip to content
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 - Bump testing library, causing slight changes in rect size #3967

Merged
merged 3 commits into from
Sep 5, 2023

Conversation

svanherk
Copy link
Contributor

@svanherk svanherk commented Sep 5, 2023

The changes in BrightspaceUI/testing#159 mean that the rect size is calculated slightly differently. Specifically, the rounding here is different than what page.screenshot was doing when we gave it fractions of pixels. This will result in some diffs changing size by a couple pixels.

Final report: https://vdiff.d2l.dev/BrightspaceUI/core/8253f016c5b2d2b6fdae6baf9cdf430666732bd4/2023-09-05_20-17-12/report/

@svanherk svanherk requested a review from a team as a code owner September 5, 2023 20:14
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

Thanks for the PR! 🎉

We've deployed an automatic preview for this PR - you can see your changes here:

URL https://live.d2l.dev/prs/BrightspaceUI/core/pr-3967/

Note
The build needs to finish before your changes are deployed.
Changes to the PR will automatically update the instance.

dlockhart
dlockhart previously approved these changes Sep 5, 2023
@svanherk svanherk dismissed dlockhart’s stale review September 5, 2023 21:39

Pushed new changes that should be reviewed

@@ -739,8 +718,7 @@ describe('d2l-input-number', () => {
});

it('should handle different locales', async() => {
documentLocaleSettings.language = 'fr';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bearfriend @dlockhart Changes to the testing library made these tests start failing. I've updated them to set the language through fixture instead (and removed the extra waiting that shouldn't be needed anymore). But let me know if this was testing this method of setting the language specifically, and there's actually an issue in testing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope this is the perfect change!

return true;
}, { interval: 10 });
return elem;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet! 💥

@@ -739,8 +718,7 @@ describe('d2l-input-number', () => {
});

it('should handle different locales', async() => {
documentLocaleSettings.language = 'fr';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope this is the perfect change!

@svanherk svanherk merged commit a0f6b0e into main Sep 5, 2023
6 checks passed
@svanherk svanherk deleted the US152268_Adjust_rect_calculation branch September 5, 2023 21:53
@ghost
Copy link

ghost commented Sep 6, 2023

🎉 This PR is included in version 2.143.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ghost ghost added the released label Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants