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

Border, Padding, and Margin shorthands #56

Merged
merged 3 commits into from
Dec 18, 2016
Merged

Border, Padding, and Margin shorthands #56

merged 3 commits into from
Dec 18, 2016

Conversation

bhough
Copy link
Contributor

@bhough bhough commented Dec 15, 2016

Shorthand helpers for border-width, border-color, border-style, margin, padding, and the general directionalProperty helper.

@bhough
Copy link
Contributor Author

bhough commented Dec 15, 2016

@mxstbr Wanted to get your eyes on this implementation before I wrote up the docs for it.

@mxstbr mxstbr mentioned this pull request Dec 15, 2016
50 tasks
@mxstbr
Copy link
Member

mxstbr commented Dec 16, 2016

Looks really good from a first glance! 😱

@bhough bhough mentioned this pull request Dec 16, 2016
@bhough bhough force-pushed the directional-helper branch 2 times, most recently from 7043d14 to 165cc6a Compare December 17, 2016 01:52
@codecov-io
Copy link

codecov-io commented Dec 17, 2016

Current coverage is 100% (diff: 100%)

Merging #56 into master will not change coverage

@@           master   #56   diff @@
===================================
  Files          21    28     +7   
  Lines          84   107    +23   
  Methods         0     0          
  Messages        0     0          
  Branches        0     0          
===================================
+ Hits           84   107    +23   
  Misses          0     0          
  Partials        0     0          

Powered by Codecov. Last update f8ee951...05215d3

@bhough bhough requested a review from mxstbr December 17, 2016 02:06
Copy link
Member

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

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

Need to update types, otherwise this looks great!

* }
*/

function position(positionKeyword: string|null, ...values: Array<string|null>) {
Copy link
Member

Choose a reason for hiding this comment

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

If you set null as a type flow will think that's an any, which we need to avoid. Instead of saying that, do param: ?string. That'll tell flow that the param is null-able!

* }
*/

function padding(...values: Array<string|null>) {
Copy link
Member

Choose a reason for hiding this comment

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

If you set null as a type flow will think that's an any, which we need to avoid. Instead of saying that, do param: ?string. That'll tell flow that the param is null-able!

* }
*/

function margin(...values: Array<string|null>) {
Copy link
Member

Choose a reason for hiding this comment

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

If you set null as a type flow will think that's an any, which we need to avoid. Instead of saying that, do param: ?string. That'll tell flow that the param is null-able!

return splitPropertyName.join('-')
}

function generateStyles(property: string, valuesWithDefaults: Array<string|null>) {
Copy link
Member

Choose a reason for hiding this comment

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

If you set null as a type flow will think that's an any, which we need to avoid. Instead of saying that, do param: ?string. That'll tell flow that the param is null-able!

* }
*/

function borderColor(...values: Array<string|null>) {
Copy link
Member

Choose a reason for hiding this comment

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

If you set null as a type flow will think that's an any, which we need to avoid. Instead of saying that, do param: ?string. That'll tell flow that the param is null-able!

* }
*/

function borderStyle(...values: Array<string|null>) {
Copy link
Member

Choose a reason for hiding this comment

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

If you set null as a type flow will think that's an any, which we need to avoid. Instead of saying that, do param: ?string. That'll tell flow that the param is null-able!

* }
*/

function borderWidth(...values: Array<string|null>) {
Copy link
Member

Choose a reason for hiding this comment

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

If you set null as a type flow will think that's an any, which we need to avoid. Instead of saying that, do param: ?string. That'll tell flow that the param is null-able!

* }
*/

function directionalProperty(property: string, ...values: Array<string|null>) {
Copy link
Member

Choose a reason for hiding this comment

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

If you set null as a type flow will think that's an any, which we need to avoid. Instead of saying that, do param: ?string. That'll tell flow that the param is null-able!

Shorthand helpers for border-width, border-color, border-style, margin, padding, and the general

directionalProperty helper
Added docs for new shorthands, added flow to test files misssing it, added npm run docs command to

serve docs locally, fixed doc typos
@bhough
Copy link
Contributor Author

bhough commented Dec 17, 2016

@mxstbr should be good now.

@mxstbr
Copy link
Member

mxstbr commented Dec 17, 2016

I'm seeing some totally unrelated commits we already have in master here?

screen shot 2016-12-17 at 21 31 41

@bhough
Copy link
Contributor Author

bhough commented Dec 17, 2016

Odd I'm not seeing those, let me re-push the rebase onto master and see if that fixes it for ya.

Update types on directional shorthands to properly show values array elements are nullable.
@bhough
Copy link
Contributor Author

bhough commented Dec 17, 2016

k @mxstbr did another force push of my branch. I've had some wonkiness going on with my pushes today, as I'm working from Taiwan and the connection isn't always stable. Hopefully that fixed it.

@mxstbr mxstbr merged commit 78cd60f into master Dec 18, 2016
@mxstbr
Copy link
Member

mxstbr commented Dec 18, 2016

Awesomeeee!!

@mxstbr mxstbr deleted the directional-helper branch December 18, 2016 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants