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

Implement a hasPseudoElementStyle assertion #232

Merged
merged 1 commit into from
Jan 4, 2019
Merged

Implement a hasPseudoElementStyle assertion #232

merged 1 commit into from
Jan 4, 2019

Conversation

joankaradimov
Copy link
Contributor

@joankaradimov joankaradimov commented Jan 3, 2019

Resolves #229

Copy link
Collaborator

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

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

nice work @joankaradimov! this looks awesome already, just one minor thing left to do :)

lib/assertions.ts Outdated Show resolved Hide resolved
@joankaradimov
Copy link
Contributor Author

nice work @joankaradimov!

Thanks!

just one minor thing left to do :)

I've committed the suggested change.

Just let me know if everything else looks good so that I squash the commits.

@joankaradimov
Copy link
Contributor Author

It turns out the initial overload signature for hasPseudoElementStyle was the correct one. The one in a858a73 does not generate any documentation. I've reverted it.

Also there seemed to be some code which did not have a documentation generated for it. I've committed that separately.

Finally - there are some DOS new lines mixed with the Unix new lines in the examples in API.md. This appears to be the case for the existing examples too. There are no DOS new lines in the docstrings. Should I log a bug for that?

@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 4, 2019

there are some DOS new lines mixed with the Unix new lines in the examples in API.md. This appears to be the case for the existing examples too. There are no DOS new lines in the docstrings. Should I log a bug for that?

Interesting. That sounds like a bug (or maybe even feature 🤔) in documentation to me

@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 4, 2019

@joankaradimov the problem with documentation is that is does not pick up the docs from these method specializations. I've moved the docstring on the method implementation, while keeping the selector: string specialization and now it seems to work as intended :)

@joankaradimov
Copy link
Contributor Author

I've moved the docstring on the method implementation

Ah, ok. I was thinking of something different.

there seemed to be some code which did not have a documentation generated for it.

I see you made this private and removed the commit.

I'm going to squash the rest of it and force push.

@Turbo87 Turbo87 merged commit 45e1e0f into mainmatter:master Jan 4, 2019
@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 4, 2019

thanks again 🙏

@joankaradimov
Copy link
Contributor Author

Thank you for the help and the quick responses. And also for quickly releasing this!

@joankaradimov joankaradimov deleted the get-pseudo-element-style branch January 4, 2019 15:59
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.

2 participants