-
Notifications
You must be signed in to change notification settings - Fork 73
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
Adds Not type class, Fixes #239 #240
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code is looking great :) can you add two simple unit tests using each instance?
I've added four simple tests in PseudoSpec.hs There is one disadvantage to the current type class approach: if you write |
Yes, that's a disadvantage. Recently, a similar issue was raised: #241 |
This issue has not seen any activity in a long time. If no further activity occurs, it will be closed after ten weeks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this PR is still a useful addition!
FYI, this can break existing code when the type for the selector is not explicit. For example, img # (not ".keep-colors" <> "src" $= ".svg") will now result in an ambiguous type. This is not a big deal for me (I plan to simply add an explicit type annotation), but it should at least be mentioned in any release notes. |
The
not
pseudo selector can only be applied to one argument at a time in most browsers, Safari seems to be the exception. The instance forNot Refinement
allows you to applyNot
to multiple arguments, however, there is no contaminator to combine twoRefinement
directly.