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

Update tabs spacing #886

Merged
merged 3 commits into from
Jul 16, 2018
Merged

Update tabs spacing #886

merged 3 commits into from
Jul 16, 2018

Conversation

dashouse
Copy link

@dashouse dashouse commented Jul 12, 2018

Updates internal padding of tab content in the Tabs component (backlog alphagov/govuk-design-system-backlog#100)

Before
screen shot 2018-07-12 at 09 57 58

After
screen shot 2018-07-12 at 14 24 56

This required margin to be defined on the tab container at mobile breakpoint to separate content.
I also increased the space below the contents menu.

screen shot 2018-07-12 at 09 54 06

Have also used the :last-child solution from conditional radios and checkboxes to remove the margin-bottom of the last item that is inside the container to avoid double margin.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-886 July 12, 2018 09:16 Inactive
@@ -8,7 +8,7 @@ examples:
panel:
html: |
<h2 class="govuk-heading-l">Past day</h2>
<table class="govuk-table">
<table class="govuk-table govuk-!-margin-bottom-0">
Copy link
Contributor

Choose a reason for hiding this comment

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

This works, just a question: In the conditional reveal I think we do this to any :last-child, should we consider doing that here too?

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-886 July 12, 2018 09:54 Inactive
@dashouse dashouse force-pushed the update-tabs-spacing branch from ef50f88 to 1d321d9 Compare July 12, 2018 10:02
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-886 July 12, 2018 10:02 Inactive
@dashouse dashouse force-pushed the update-tabs-spacing branch from 1d321d9 to f213136 Compare July 12, 2018 10:05
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-886 July 12, 2018 10:05 Inactive
@adamsilver
Copy link
Contributor

The padding left and right was as it was so that the text in the first tab vertically aligned with the text (or whatever) inside the panel. This was something @joelanman originally asked me to do. I don't mind the change, but mentioning just in case.

@dashouse
Copy link
Author

@adamsilver @joelanman Ok great, sorry for the slight alignment issue (no pun intended). I'll keep the top/bottom margin and check in with Joe on the left and right...makes sense

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-886 July 12, 2018 13:22 Inactive
@dashouse dashouse force-pushed the update-tabs-spacing branch from 8ba13b5 to 2ce39d8 Compare July 12, 2018 13:24
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-886 July 12, 2018 13:24 Inactive
@dashouse
Copy link
Author

Actually this is the right thing to do, have updated this to only affect the top/bottom relationship :)

@36degrees 36degrees added this to the [NEXT] milestone Jul 13, 2018
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

LGTM, but needs a changelog entry.

@36degrees 36degrees removed this from the [NEXT] milestone Jul 13, 2018
@NickColley NickColley force-pushed the update-tabs-spacing branch from 2ce39d8 to d159925 Compare July 16, 2018 09:45
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-886 July 16, 2018 09:45 Inactive
@NickColley NickColley added this to the [NEXT] milestone Jul 16, 2018
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

👍

@NickColley NickColley merged commit 8f2fe56 into master Jul 16, 2018
@NickColley
Copy link
Contributor

Thanks @dashouse 👍

@NickColley NickColley deleted the update-tabs-spacing branch July 16, 2018 09:52
@NickColley NickColley mentioned this pull request Jul 17, 2018
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.

5 participants