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

Create shared helper for components #759

Merged
merged 9 commits into from
Feb 26, 2019

Conversation

andysellick
Copy link
Contributor

@andysellick andysellick commented Feb 21, 2019

A number of the components are now using the same or similar code for setting a margin bottom and creating a heading level. This PR creates a single shared component helper that provides that functionality in one place, and updates those components that need it to use it.

Trello card: https://trello.com/c/DCHfrY2s/82-draft-create-a-shared-set-of-helper-functions-to-be-used-across-components

@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-759 February 21, 2019 10:46 Inactive
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-759 February 21, 2019 11:57 Inactive
@andysellick andysellick changed the title Create shared helper for components [DO NOT MERGE] Create shared helper for components Feb 21, 2019
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-759 February 22, 2019 10:52 Inactive
@andysellick andysellick force-pushed the create-shared-helper-for-components branch from 652e13b to ed518c2 Compare February 22, 2019 10:57
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-759 February 22, 2019 10:58 Inactive
@andysellick andysellick force-pushed the create-shared-helper-for-components branch from ed518c2 to b78e24c Compare February 22, 2019 11:05
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-759 February 22, 2019 11:05 Inactive
@andysellick andysellick force-pushed the create-shared-helper-for-components branch from b78e24c to 445d188 Compare February 22, 2019 11:09
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-759 February 22, 2019 11:09 Inactive
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-759 February 22, 2019 11:34 Inactive
@andysellick andysellick force-pushed the create-shared-helper-for-components branch from 70ec4d0 to 0866905 Compare February 22, 2019 11:35
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-759 February 22, 2019 11:35 Inactive
@andysellick andysellick marked this pull request as ready for review February 22, 2019 11:45
@andysellick andysellick requested a review from alex-ju February 22, 2019 11:45
@andysellick andysellick changed the title [DO NOT MERGE] Create shared helper for components Create shared helper for components Feb 22, 2019
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-759 February 22, 2019 11:49 Inactive
Copy link
Contributor

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @andysellick.

I left a few comments, otherwise looks great. And I'm not sure about the name shared_helper, but I guess makes its purpose clear and it's something we can change if we feel like.

@andysellick andysellick force-pushed the create-shared-helper-for-components branch from 207f026 to de41373 Compare February 26, 2019 08:58
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-759 February 26, 2019 08:58 Inactive
@andysellick
Copy link
Contributor Author

@alex-ju thanks for the comments. Yeah, I struggled with the name a bit. I went with that because it was the clearest and shortest thing I could think of. Happy to change it later if we can think of something better.

- provides margin bottom for components
- restructure textarea slightly to put margin on wrapping element rather than textarea itself, as those two margins were conflicting
- set default margin to be 6, overriding default of 3 in shared helper, means this will be a non-breaking change
- add margin option to documentation
- defaults to no margin as per original behaviour
- update shared helper to return a span if heading level 0 is passed, instead of a heading level
- retain the default of h2 if nothing or an invalid value is passed
@andysellick andysellick force-pushed the create-shared-helper-for-components branch from de41373 to 78bad64 Compare February 26, 2019 12:43
@andysellick andysellick merged commit 7a4b0ba into master Feb 26, 2019
@andysellick andysellick deleted the create-shared-helper-for-components branch February 26, 2019 12:56
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