Skip to content

use longhand rules for spacing to avoid radium warnings#252

Merged
Brantron merged 2 commits intomasterfrom
brantron/margins-padding
Sep 20, 2018
Merged

use longhand rules for spacing to avoid radium warnings#252
Brantron merged 2 commits intomasterfrom
brantron/margins-padding

Conversation

@Brantron
Copy link
Copy Markdown
Contributor

@Brantron Brantron commented Sep 15, 2018

Fixes #246

@Brantron Brantron force-pushed the brantron/margins-padding branch from 9d3dc59 to da0efc4 Compare September 15, 2018 18:46
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 15, 2018

Codecov Report

Merging #252 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #252      +/-   ##
==========================================
+ Coverage   79.88%   79.92%   +0.03%     
==========================================
  Files          57       57              
  Lines        1064     1066       +2     
  Branches      243      243              
==========================================
+ Hits          850      852       +2     
  Misses        155      155              
  Partials       59       59

Copy link
Copy Markdown
Collaborator

@mmarcuccio mmarcuccio left a comment

Choose a reason for hiding this comment

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

👍

const createYRules = (type, value) => ({
[`${type}Top`]: value,
[`${type}Bottom`]: value
})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we memoize these? It's possible they'll be called very frequently if someone is using them in a render()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think we need to memoize these because it looks like they are used internally only to generate the spacing object. That simple spacing object is then what could be called many times by the user.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ah yeah, that makes sense :)

Copy link
Copy Markdown
Member

@dcocchia dcocchia left a comment

Choose a reason for hiding this comment

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

👍

@Brantron Brantron merged commit 65ae565 into master Sep 20, 2018
@Brantron Brantron mentioned this pull request Sep 20, 2018
Brantron added a commit that referenced this pull request Sep 20, 2018
🏠 Internal
- `spacing`
	- #252 use longhand rules for spacing to avoid radium warnings (#252)

- `isEqual`
	- #249 refactor isEqual for better maintainability
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.

3 participants