-
-
Notifications
You must be signed in to change notification settings - Fork 209
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 Text Inputs and Buttons Selector Shorthand #57
Conversation
Current coverage is 100% (diff: 100%)@@ master #57 diff @@
===================================
Files 15 18 +3
Lines 57 73 +16
Methods 0 0
Messages 0 0
Branches 0 0
===================================
+ Hits 57 73 +16
Misses 0 0
Partials 0 0
|
After working on the Buttons shorthand, I'm going to refactor this a bit to use a shared helper and leverage tagged template literals to produce the selectors. |
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.
Don't forget to add it to .documentation.json
in the ToC!
| null | ||
| string; | ||
|
||
function statefulSelectors(states: Array<State>, template: Function, stateMap: ?Array<State>) { |
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.
Would be nice to have a small high-level comment explaining why we need this!
* const div = styled.div` | ||
* ${buttons('active')} { | ||
* border: none; | ||
* } |
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.
This actually doesn't make sense like this in styled-components
, if anything this should be:
const div = styled.div`
> ${buttons('active')} {
border: none;
}
`
* | ||
* // styled-components usage | ||
* const div = styled.div` | ||
* ${textInputs('active')} { |
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.
Same as above, needs >
@mxstbr should be good now assuming CI passes. |
"hiDPI", | ||
"selection", | ||
"placeholder", | ||
"retinaImage", |
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.
Those were done in #59 🙈 Would you mind merging that first, rebasing on top of master and then adding the shorthands? Thanks and sorry for the troubles!
Adds shorthand helper for textInput selectors, including active, focus, and hover states.
buttons and textInputs shorthand now use a shared internalHelper statefulSelectors. Also removed build export for polished.
Updated styled-components example in buttons and textInputs. Added private doc explaining why we use the statefulSelectors internal helper
Update TOC with missing modules
Rebuild
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.
LGTM, thanks! Sorry for the troubles!
Adds shorthand helper for textInput and buttonsselectors, including active, focus, and hover states.