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.

Sign up for GitHub

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

Jump to bottom

Add default container to header #807

Merged
merged 5 commits into from
Jun 20, 2018
Merged

Add default container to header #807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2a8d44a
Add govuk-width-container as default
alex-ju Jun 20, 2018
6ffa4e5
Remove default values
alex-ju Jun 20, 2018
93de9a1
Move full-width modifier from the component to the container
alex-ju Jun 20, 2018
5775f15
Update readme
alex-ju Jun 20, 2018
3ae63b7
Update changelog
alex-ju Jun 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Note: We're not following semantic versioning yet, we are going to talk about th

- Add default text for back-link component
([PR #793](https://github.com/alphagov/govuk-frontend/pull/793))
- Add default container class to the header component
([PR #807](https://github.com/alphagov/govuk-frontend/pull/807))

🏠 Internal:

Expand All @@ -16,7 +18,7 @@ Note: We're not following semantic versioning yet, we are going to talk about th

- Update docs with the assistive technology we support ([PR #800](https://github.com/alphagov/govuk-frontend/pull/800))

- Update docs about installing fonts ([PR #802] (https://github.com/alphagov/govuk-frontend/pull/802))
- Update docs about installing fonts ([PR #802](https://github.com/alphagov/govuk-frontend/pull/802))

- Update browser support matrix
Remove Windows Phone
Expand Down
2 changes: 0 additions & 2 deletions app/views/examples/template-custom/index.njk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@
{% block header %}
<!-- block:header -->
{{ govukHeader({
homepageUrl: "/",
containerClasses: "govuk-width-container",
serviceName: "Nom du service",
navigation: [
{
Expand Down
13 changes: 2 additions & 11 deletions src/components/header/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,7 @@ Find out when to use the Header component in your service in the [GOV.UK Design

{% from 'header/macro.njk' import govukHeader %}

{{ govukHeader({
"homepageUrl": "/",
"containerClasses": "govuk-width-container"
}) }}
{{ govukHeader({}) }}

### Header--with-service-name

Expand Down Expand Up @@ -110,10 +107,8 @@ Find out when to use the Header component in your service in the [GOV.UK Design
{% from 'header/macro.njk' import govukHeader %}

{{ govukHeader({
"homepageUrl": "/",
"serviceName": "Service Name",
"serviceUrl": "/components/header",
"containerClasses": "govuk-width-container"
"serviceUrl": "/components/header"
}) }}

### Header--with-navigation
Expand Down Expand Up @@ -193,8 +188,6 @@ Find out when to use the Header component in your service in the [GOV.UK Design
{% from 'header/macro.njk' import govukHeader %}

{{ govukHeader({
"homepageUrl": "/",
"containerClasses": "govuk-width-container",
"navigation": [
{
"href": "#1",
Expand Down Expand Up @@ -297,10 +290,8 @@ Find out when to use the Header component in your service in the [GOV.UK Design
{% from 'header/macro.njk' import govukHeader %}

{{ govukHeader({
"homepageUrl": "/",
"serviceName": "Service Name",
"serviceUrl": "/components/header",
"containerClasses": "govuk-width-container",
"navigation": [
{
"href": "#1",
Expand Down
29 changes: 18 additions & 11 deletions src/components/header/_header.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,13 @@

}

.govuk-header--full-width {
.govuk-header__container--full-width {
Copy link
Contributor

Choose a reason for hiding this comment

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

I find it a bit weird that the container class adds a border and has a side effect on a contained element – I'd expect the container just to set margins and width… but might just be me. Anyone else got any thoughts on the matter?

Copy link
Contributor Author

@alex-ju alex-ju Jun 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yep, it bugs me too, but otherwise I'll have to check the classes applied to the header and the ones applied to the container to make sure we don't have a 'strange' mix (i.e. classes: govuk-header--full-width and containerClasses: govuk-width-container)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd go with what we have now, and reconsider modifiers again in the future as it allows you to do what you need to inside the component.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

padding: 0 govuk-spacing(3);
border-color: $govuk-header-border-color;

.govuk-header__menu-button {
right: govuk-spacing(3);
}
}

.govuk-header__container {
Expand Down Expand Up @@ -175,23 +179,26 @@
list-style: none;
}

.js-enabled .govuk-header__menu-button {
display: block;
@include mq ($from: desktop) {
.js-enabled {
.govuk-header__menu-button {
display: block;
@include mq ($from: desktop) {
display: none;
}
}

.govuk-header__navigation {
display: none;
@include mq ($from: desktop) {
display: block;
}
}
}

.js-enabled .govuk-header__navigation {
display: none;
@include mq ($from: desktop) {
.govuk-header__navigation--open {
display: block;
}
}

.js-enabled .govuk-header__navigation--open {
display: block;
}

.govuk-header__navigation--end {
@include mq ($from: desktop) {
Expand Down
17 changes: 3 additions & 14 deletions src/components/header/header.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,16 @@ examples:
- name: default
description: The standard header as used on information pages on GOV.UK
data:
homepageUrl: /
containerClasses: govuk-width-container
{}

- name: with-service-name
description: If your service is more than a few pages long, you can help users understand where they are by adding the service name.
data:
homepageUrl: /
serviceName: Service Name
serviceUrl: /components/header
containerClasses: govuk-width-container

- name: with-navigation
data:
homepageUrl: /
containerClasses: govuk-width-container
navigation:
- href: '#1'
text: 'Navigation item 1'
Expand All @@ -51,10 +46,8 @@ examples:
- name: with-service-name-and-navigation
description: If you need to include basic navigation, contact or account management links.
data:
homepageUrl: /
serviceName: Service Name
serviceUrl: /components/header
containerClasses: govuk-width-container
navigation:
- href: '#1'
text: 'Navigation item 1'
Expand All @@ -70,8 +63,6 @@ examples:
readme: false
description: An edge case example with a large number of navitation items with long names used to test wrapping
data:
homepageUrl: /
containerClasses: govuk-width-container
navigation:
- href: '/browse/benefits'
text: 'Benefits'
Expand Down Expand Up @@ -109,16 +100,14 @@ examples:
- name: full-width
readme: false
data:
homepageUrl: /
classes: govuk-header--full-width
containerClasses: govuk-header__container--full-width
navigationClasses: govuk-header__navigation--end
productName: Product Name

- name: full-width-with-navigation
readme: false
data:
homepageUrl: /
classes: govuk-header--full-width
containerClasses: govuk-header__container--full-width
navigationClasses: govuk-header__navigation--end
productName: Product Name
navigation:
Expand Down
2 changes: 1 addition & 1 deletion src/components/header/template.njk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<header class="govuk-header {{ params.classes if params.classes }}" role="banner" data-module="header"
{%- for attribute, value in params.attributes %} {{attribute}}="{{value}}"{% endfor %}>
<div class="govuk-header__container {{ params.containerClasses if params.containerClasses }}">
<div class="govuk-header__container {{ params.containerClasses | default('govuk-width-container') }}">

<div class="govuk-header__logo">
<a href="{{ params.homepageUrl | default('/') }}" class="govuk-header__link govuk-header__link--homepage">
Expand Down
4 changes: 1 addition & 3 deletions src/template.njk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@
{% endblock %}

{% block header %}
{{ govukHeader({
containerClasses: 'govuk-width-container'
}) }}
{{ govukHeader({}) }}
{% endblock %}

{% block main %}
Expand Down