Skip to content

Add more selector combinators #100

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

Merged

Conversation

vyorkin
Copy link
Collaborator

@vyorkin vyorkin commented Jul 9, 2018

This PR is a 100% port from Clay, no other additions from my side.

Resolves #98

@vyorkin
Copy link
Collaborator Author

vyorkin commented Jul 9, 2018

Please, don't merge yet, I'm going to add a couple of tests.
I'll add more tests after migrating to purescript-test-unit as we've discussed in #30

Copy link
Member

@garyb garyb left a comment

Choose a reason for hiding this comment

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

A couple of bikesheddy thoughts:

element :: String -> Selector
element e = Selector (Refinement []) (Elem e)

-- | The deep selector composer.
-- | Maps to `sel1 sel2` in CSS.
deep :: Selector -> Selector -> Selector
deep a b = Selector (Refinement []) (Deep a b)
infix 6 deep as **
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could change this to |* for a kind of consistency with >, +?

Copy link
Collaborator Author

@vyorkin vyorkin Jul 9, 2018

Choose a reason for hiding this comment

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

yeah, I also think this is going to look nicer :)


-- | The filter selector composer, adds a filter to a selector.
-- | Maps to something like `sel#filter`, `sel.filter` or `sel:filter` in CSS,
-- | depending on the filter.
with :: Selector -> Refinement -> Selector
with (Selector (Refinement fs) e) (Refinement ps) = Selector (Refinement (fs <> ps)) e
infix 6 with as ##
Copy link
Member

Choose a reason for hiding this comment

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

We could use & here perhaps, since it expresses combination nicely? I guess it's not done in Haskell since & is an operator there (that operator is actually # in PS 😄)

Copy link
Collaborator Author

@vyorkin vyorkin Jul 9, 2018

Choose a reason for hiding this comment

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

yeah, good point, thanks! (btw this might be a breaking change though as this operator was there before)

@vyorkin vyorkin force-pushed the feature/selector-combinators branch from f07c1cb to bd0debd Compare July 9, 2018 12:19
@vyorkin vyorkin force-pushed the feature/selector-combinators branch from bd0debd to bd25238 Compare July 9, 2018 13:05
@vyorkin
Copy link
Collaborator Author

vyorkin commented Jul 9, 2018

Ok, I've added some tests & rebased, should be ready for review.
I think now it supports everything from here https://developer.mozilla.org/en-US/docs/Web/CSS/Attribute_selectors (except [attr operator value i])

Copy link
Member

@garyb garyb left a comment

Choose a reason for hiding this comment

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

Great 👍

Did you still want to hold off merging until things are updated for -test-unit?

@vyorkin
Copy link
Collaborator Author

vyorkin commented Jul 9, 2018

Actually I wanted to migrate tests after merging PR's :) is it ok?

@garyb
Copy link
Member

garyb commented Jul 9, 2018

Sure thing!

@garyb garyb merged commit f86824b into purescript-contrib:master Jul 9, 2018
@vyorkin vyorkin deleted the feature/selector-combinators branch July 9, 2018 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants