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

Section break styles fix when used with Elements #682

Merged
merged 2 commits into from
May 8, 2018
Merged

Conversation

kr8n3r
Copy link

@kr8n3r kr8n3r commented May 4, 2018

<hr> styling in Elements has background and height
set, whereas in Frontend we're using border bottom.

To fix, height:0 was added in a compatibility conditional include.

This fixes both the thickness and the fact that styles from Elements were making invisible section breaks visible
Before:
screen shot 2018-05-04 at 14 14 37

After:
screen shot 2018-05-04 at 14 14 55

Elements styles causing non-visible section break to be visible:

screen shot 2018-05-08 at 09 06 28

Ticket: https://trello.com/c/Sc7RtFZK/815-remove-section-break-border-coming-from-elements

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-682 May 4, 2018 12:59 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-682 May 4, 2018 13:00 Inactive
@kr8n3r kr8n3r force-pushed the section-break-fix branch from b49362a to 3e2ffef Compare May 4, 2018 13:02
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-682 May 4, 2018 13:02 Inactive
@kr8n3r kr8n3r changed the title Section border styles fix when used with Elements Section break styles fix when used with Elements May 4, 2018
@kr8n3r kr8n3r force-pushed the section-break-fix branch from 3e2ffef to 03dc6a2 Compare May 4, 2018 13:27
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-682 May 4, 2018 13:27 Inactive
@kr8n3r kr8n3r force-pushed the section-break-fix branch from 03dc6a2 to d486f71 Compare May 4, 2018 13:37
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-682 May 4, 2018 13:38 Inactive
@@ -44,6 +44,9 @@

%govuk-section-break--visible {
border-bottom: 1px solid $govuk-border-colour;
@include govuk-compatibility(govuk_elements) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when elements overrides the invisible break?

Copy link
Author

Choose a reason for hiding this comment

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

me being stupid and not moving it into where i said i was. fixed now.

@kr8n3r kr8n3r force-pushed the section-break-fix branch from d486f71 to 9855dff Compare May 8, 2018 12:09
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-682 May 8, 2018 12:09 Inactive
@kr8n3r kr8n3r force-pushed the section-break-fix branch from 9855dff to 1dc1861 Compare May 8, 2018 12:25
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-682 May 8, 2018 12:25 Inactive
@@ -5,6 +5,12 @@
%govuk-section-break {
margin: 0;
border: 0;

// fix doouble-width section break and forced visible section break
Copy link
Contributor

Choose a reason for hiding this comment

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

Typoe 'doouble'

Jani Kraner added 2 commits May 8, 2018 14:09
`<hr>` styling in Elements has background and height
set, whereas in Frontend we're using border bottom.

To fix, `height:0` was added in a compatibility conditional include.
Copy link
Contributor

@NickColley NickColley left a comment

Choose a reason for hiding this comment

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

Nice fix 👍

@kr8n3r kr8n3r merged commit c01b383 into master May 8, 2018
@kr8n3r kr8n3r deleted the section-break-fix branch May 8, 2018 15:27
@kr8n3r kr8n3r mentioned this pull request May 18, 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.

3 participants