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.

Sign up for GitHub

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

Jump to bottom

fix: support uppercase custom props in toHaveStyle #552

Merged
merged 4 commits into from
Nov 30, 2023
Merged

fix: support uppercase custom props in toHaveStyle #552

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b12b4f4
fix: support uppercase custom props in toHaveStyle
depoulo Nov 28, 2023
e7aee18
fix: make the test pass
depoulo Nov 28, 2023
118f295
fix: handle different use cases
depoulo Nov 29, 2023
90296fc
fix: turned out to be a little more complex
depoulo Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/__tests__/to-have-style.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ describe('.toHaveStyle', () => {

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

test('handles global custom properties', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/to-have-style.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function isSubset(styles, computedStyle) {
Object.entries(styles).every(
([prop, value]) =>
computedStyle[prop] === value ||
computedStyle.getPropertyValue(prop.toLowerCase()) === value,
computedStyle.getPropertyValue(prop) === value,
Copy link
Member

Choose a reason for hiding this comment

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

Wait, I think we may have a problem. I suspect that it was done this way because non-custom properties are case-insensitive. At least a quick check in the browser shows that setting Display: none hides the element.

Can you make it so that we keep the lower case transformation when the prop does not look like a custom css prop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, I'll look into it tomorrow!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good that there was a test case added with #148. Could have found out via git blame.

)
)
}
Expand Down