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

Add withWhiteSpace to spacing lib #523

Merged
merged 2 commits into from
Feb 1, 2019
Merged

Conversation

stevesims
Copy link
Member

  • adds new withWhiteSpace function to spacing lib
    • generates spacing styles, accepting margin, padding and mb props
    • intended for use within styled component definitions

withWhiteSpace HOC refactored to use function, but is now deprecated

Aims to provide a solution to #522

Checklist:

  • Documentation
  • Tests
  • Ready to be merged

* adds new `withWhiteSpace` function to spacing lib
  - generates spacing styles, accepting `margin`, `padding` and `mb` props
  - intended for use within styled component definitions

`withWhiteSpace` HOC refactored to use function, but is now deprecated
@penx
Copy link
Member

penx commented Jan 31, 2019

Deploy preview for govuk-react processing.

Building with commit c006739

https://app.netlify.com/sites/govuk-react/deploys/5c54211e568bc200086b81ff

@codecov-io
Copy link

codecov-io commented Jan 31, 2019

Codecov Report

Merging #523 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #523      +/-   ##
==========================================
+ Coverage   99.77%   99.78%   +0.01%     
==========================================
  Files          83       84       +1     
  Lines         440      467      +27     
  Branches       29       30       +1     
==========================================
+ Hits          439      466      +27     
  Misses          1        1
Impacted Files Coverage Δ
packages/lib/src/spacing/index.js 100% <100%> (ø) ⬆️
packages/hoc/src/withWhiteSpace/index.js 100% <100%> (ø) ⬆️
components/error-text/src/index.js 100% <0%> (ø) ⬆️
components/back-link/src/index.js 100% <0%> (ø) ⬆️
packages/constants/src/focus.js 100% <0%> (ø) ⬆️
components/breadcrumb/src/index.js 100% <0%> (ø) ⬆️
components/hint-text/src/index.js 100% <0%> (ø) ⬆️
packages/lib/src/link/index.js 100% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f01366c...c006739. Read the comment docs.

@penx
Copy link
Member

penx commented Feb 1, 2019

I am guessing this is a breaking change so needs a point release?

@penx
Copy link
Member

penx commented Feb 1, 2019

The CONTRIBUTING.md doc will need updating
https://github.com/govuk-react/govuk-react/blob/master/CONTRIBUTING.md#white-space

@penx
Copy link
Member

penx commented Feb 1, 2019

ah ok maybe not breaking... but breaking when we move to styled-components?

@stevesims
Copy link
Member Author

Yeah - this won't be breaking by itself. When we actually remove the withWhiteSpace HOC that'd be breaking. And yes - moving to styled-components changes the peer dependency, so that'd breaking too.

@stevesims stevesims merged commit 9ae1379 into master Feb 1, 2019
@stevesims stevesims deleted the feature/whitespace-lib branch February 1, 2019 11:04
@penx penx mentioned this pull request Feb 4, 2019
penx added a commit that referenced this pull request Feb 4, 2019
We are moving to styled-components (see #520). v0.5.x will remain as emotion releases as long as there are upstream users that don't want to switch to styled-components, but the point release will no longer indicate breaking vs. non breaking.

Changes in this release:

## Util
Add new spacing lib (#514)
Enhanced withWhiteSpace HOC (#515)
Add `withWhiteSpace` to spacing lib (#523)

## Breaking style changes
Fix/Improve component typography (#519)
Button shadow colour tweak (#513)
Update Button styling (#512)
Typography updates (#508)

## Documentation/dev tooling
Minor readme updates (#510)
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