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

Changed toHaveStyle to use getPropertyValue instead of accessing the property directly #285

Merged
merged 10 commits into from Aug 11, 2020
48 changes: 48 additions & 0 deletions src/__tests__/to-have-style.js
Expand Up @@ -103,6 +103,9 @@ describe('.toHaveStyle', () => {
expect(container.querySelector('.label')).toHaveStyle('color white'),
).toThrowError()

expect(() =>
expect(container.querySelector('.label')).toHaveStyle('--color: black'),
).toThrowError()
document.body.removeChild(style)
document.body.removeChild(container)
})
Expand All @@ -116,6 +119,33 @@ describe('.toHaveStyle', () => {
)
})

test('handles inline custom properties', () => {
const {queryByTestId} = render(`
<span data-testid="color-example" style="--color: blue">Hello World</span>
`)
expect(queryByTestId('color-example')).toHaveStyle('--color: blue')
})

test('handles global custom properties', () => {
const style = document.createElement('style')
style.innerHTML = `
div {
--color: blue;
}
`

const {container} = render(`
<div>
Hello world
</div>
`)

document.body.appendChild(style)
document.body.appendChild(container)

expect(container).toHaveStyle(`--color: blue`)
})

test('properly normalizes colors for border', () => {
const {queryByTestId} = render(`
<span data-testid="color-example" style="border: 1px solid #fff">Hello World</span>
Expand Down Expand Up @@ -170,6 +200,24 @@ describe('.toHaveStyle', () => {
})
})

test('Uses px as the default unit', () => {
const {queryByTestId} = render(`
<span data-testid="color-example" style="font-size: 12px">Hello World</span>
`)
expect(queryByTestId('color-example')).toHaveStyle({
fontSize: 12
Copy link
Member

Choose a reason for hiding this comment

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

Why would this match 12rem? If I set font-size: 12; then this results in 12px. This seems a bit confusing to me.

Also: This test only works in JSDOM. In an actual browser the value would be resolved to (depending on the base font size) to 240px.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right! This test case doesn't make any sense as it is going to work only in JSDOM. Btw, I didn't know that, thanks! I'm going to change that.

It isn't clear to me if you are only against this test case or the idea of the api.

Copy link
Member

Choose a reason for hiding this comment

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

Just want to make sure that we're not documenting a test case that wouldn't work in a correct DOM environment.

Copy link
Member

Choose a reason for hiding this comment

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

Follow up on this. It seems from the discussion above that there's still something to be addressed here before this is ready. Right?

Copy link
Member

Choose a reason for hiding this comment

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

Is JSDOM actually returning 12rem here has the computed style? Then all we have to do is add a comment explaining that this test depends on the user agent (since it will return the computed font-size in pixel which depends on the base font size for rem).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed to px right after your first comment, do you think that is still worth to leave a note about that behavior?

})
})

test('Fails with an invalid unit', () => {
const {queryByTestId} = render(`
<span data-testid="color-example" style="font-size: 12rem">Hello World</span>
`)
expect(() => {
expect(queryByTestId('color-example')).toHaveStyle({ fontSize: '12px' })
}).toThrowError()
})

test('supports dash-cased property names', () => {
const {container} = render(`
<div class="label" style="background-color: blue; height: 100%">
Expand Down
2 changes: 1 addition & 1 deletion src/to-have-style.js
Expand Up @@ -22,7 +22,7 @@ function isSubset(styles, computedStyle) {
Object.entries(styles).every(
([prop, value]) =>
computedStyle[prop] === value ||
computedStyle[prop.toLowerCase()] === value,
computedStyle.getPropertyValue(prop.toLowerCase()) === value,
)
)
}
Expand Down