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

Add assert.dom(...).hasStyle() assertion #168

Merged
merged 5 commits into from
Nov 23, 2018

Conversation

selvagsz
Copy link
Contributor

@selvagsz selvagsz commented Oct 17, 2018

Add assertions for styles like

assert.dom('.some-element').hasStyle({
  opacity: 1,
  width: '340px',
});

Fixes #90

supersedes #106

@selvagsz selvagsz mentioned this pull request Oct 17, 2018
@selvagsz
Copy link
Contributor Author

@Turbo87 this is how it renders on failure

image

@selvagsz
Copy link
Contributor Author

Just saw there is yarn docs to generate the documentation. Will use it & update the pr

@selvagsz
Copy link
Contributor Author

@Turbo87 fixed the docs & updated the tests. lmk if I'd missed anything else :)

for (let property in expected) {
actual[property] = cssStyleDeclaration[property];
if (!result) {
result = '' + expected[property] === actual[property];
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is this doing?

maybe something like this might be easier to read/understand:

let computedStyle = window.getComputedStyle(element);

let expectedProperties = Object.keys(expected);
let result = expectedProperties.every(property => computedStyle[property] === expected[property]);

let actual = {};
expectedProperties.forEach(property => actual[property] = computedStyle[property]);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

refactored as suggested 👍

lib/__tests__/has-style.ts Show resolved Hide resolved
@jayjayjpg
Copy link
Contributor

@Turbo87 @selvagsz Is it alright if I pick this one up? It would be sweet to see the work that has already been done land ✨

@gabrielcsapo
Copy link

@jessica-jordan please 🙏 this would be awesome to have!

@selvagsz
Copy link
Contributor Author

Been working on product releases for a while. Sorry for the delay

@selvagsz selvagsz force-pushed the feature/has_style_assertion branch 2 times, most recently from 209e35a to b25a111 Compare November 22, 2018 10:42
Copy link
Contributor

@jayjayjpg jayjayjpg left a comment

Choose a reason for hiding this comment

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

@Turbo87
Copy link
Collaborator

Turbo87 commented Nov 23, 2018

nice work, thanks @selvagsz!

@Turbo87 Turbo87 merged commit e384990 into mainmatter:master Nov 23, 2018
@selvagsz selvagsz deleted the feature/has_style_assertion branch December 28, 2018 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants