Alternative header layout #1278

maxgds · 2020-02-04T12:13:17Z

What

Adds a new layout alternative to the layout header.
https://trello.com/c/vMiHP2yB/194-build-header-layout-for-search-on-left

Why

This is needed for pages in whitehall such as https://www.gov.uk/government/how-government-works

Notes: The layout change meant that the skeleton of the html needed to change. To avoid repeating blocks they are now in partials. I tried to do this with content blocks but it broke the component's examples page as each render of the content block appended itself to the previous ones before displaying.

This is being pushed back into a non-master branch so that individual chunks of work can be reviewed and monitored on Trello rather than pushing to one huge PR.

Screenshot

kr8n3r · 2020-02-04T16:18:56Z

is it known that the nav menu is not visible on mobile?

maxgds · 2020-02-05T07:55:48Z

is it known that the nav menu is not visible on mobile?

No, it wasn't... it was working previously I must have broken something. Looking into it.

maxgds · 2020-02-05T08:36:20Z

is it known that the nav menu is not visible on mobile?

Fixed now, I'd accidentally optimised away the div that set the relative position.

andysellick

A few initial visual discrepancies (I haven't dived into the code yet). The live header has the mobile menu on the right, this has it on the left:

Between 768px and somewhere around 640px the mobile menu's icon collapses beneath it:

maxgds · 2020-02-06T12:14:54Z

A few initial visual discrepancies (I haven't dived into the code yet).

The layout at mobile is different anyway - the search bar is full width deliberately rather than half width to some arbitrary point where it gets hidden on live. Likewise the menu items don't try to cram into 2 columns. I thought it made more sense for the menu button to appear on the left than on the right if we're not putting the search over there. If you think it should be on the right, why?

I'll look into the separated icon, though.

andysellick · 2020-02-06T13:11:28Z

I'm not sure which side it should be on, I was just pointing out the difference.

Another slightly odd behaviour - if you expand the menu on mobile then switch to desktop the whole of the menu is pushed down slightly. Obviously this is a niche use case and doesn't represent a barrier to anything, it's just a little odd.

app/views/govuk_publishing_components/components/docs/layout_header.yml

app/assets/stylesheets/govuk_publishing_components/components/_layout-header.scss

app/views/govuk_publishing_components/components/_layout_header.html.erb

app/views/govuk_publishing_components/components/docs/layout_header.yml

app/views/govuk_publishing_components/components/layout_header/_header_logo.html.erb

andysellick

Seems good, I have a few comments, mostly me being nitpicky, but a couple of things that might need looking at.

app/views/govuk_publishing_components/components/_layout_header.html.erb

app/views/govuk_publishing_components/components/layout_header/_header_logo.html.erb

app/views/govuk_publishing_components/components/layout_header/_navigation_items.html.erb

app/assets/stylesheets/govuk_publishing_components/components/_layout-header.scss

andysellick · 2020-02-06T13:32:11Z

app/assets/stylesheets/govuk_publishing_components/components/_layout-header.scss

+    }
+
+  }
+


Nitpick: I'd remove this blank line and line 83.

app/assets/stylesheets/govuk_publishing_components/components/_layout-header.scss

app/views/govuk_publishing_components/components/_layout_header.html.erb

app/views/govuk_publishing_components/components/docs/layout_header.yml

app/views/govuk_publishing_components/components/layout_header/_header_logo.html.erb

kr8n3r · 2020-02-06T14:24:05Z

app/views/govuk_publishing_components/components/layout_header/_header_logo.html.erb

+<div class="govuk-header__logo gem-c-header__logo">
+  <a href="/" class="govuk-header__link govuk-header__link--homepage">
+    <span class="govuk-header__logotype">
+      <svg role="presentation" focusable="false" class="govuk-header__logotype-crown" xmlns="http://www.w3.org/2000/svg" viewbox="0 0 132 97" height="32" width="36">


role=presentation is being replaced with aria-hidden=true
see alphagov/govuk-frontend#1724

This adds a new layout alternative that allows for search on the left of the header and navigation items on the right to support some layouts seen in whitehall pages. It also corrects a stray div that was breaking the with-nav option without a searchbox.

- Concatenates the appropriate header classes before implementing them more simply - Reduces repitition in code where the search_left layout branches from the standard

This spacing that was added during development isn't doing anything but introduced a minor bug when resizing the browser with the nav open, so this removes it.

- Removes spurious space - Adds a description of where this variant of header is used

vanitabarrett · 2020-02-12T10:05:25Z

app/assets/stylesheets/govuk_publishing_components/components/_layout-header.scss

 }

+


Additional new lines here

app/views/govuk_publishing_components/components/layout_header/_header_logo.html.erb

vanitabarrett · 2020-02-12T10:09:22Z

Is this placement of the menu link here correct? Could the spacing below the Menu link match the spacing above, as it looks quite close to the blue border?

vanitabarrett · 2020-02-12T10:10:04Z

Not sure if this is a problem which existed before, but it looks like a search bar on it's own needs some bottom spacing on smaller devices so it doesn't touch the blue bar

injms · 2020-02-12T11:47:03Z

There are a couple of false positives with the automated accessibility tests caused by the component being repeated on the example page - it's not a problem with the component, as this is (hopefully) something that wouldn't happen in the real world use.

We can turn off the checks by adding duplicate-id-aria and form-field-multiple-labels to the accessibility_excluded_rules in the layout header's YAML file.

An alternative fix for the form-field-multilple-labels problem is to make the id of the search component unique by appending #{SecureRandom.hex(4)} to the id - which is what's done on the accordion component.

maxgds · 2020-02-12T13:25:12Z

Is this placement of the menu link here correct? Could the spacing below the Menu link match the spacing above, as it looks quite close to the blue border?

Fixed the spacing issues and the stray new lines introduce/overlooked previously.

maxgds · 2020-02-12T13:26:52Z

There are a couple of false positives with the automated accessibility tests caused by the same components being repeated on the example page

I've ignored those checks. The accordion example is different - you could potentially have multiple accordions on a single page, but you shouldn't have multiple headers at any time, so it needs the randomising approach whereas we don't here.

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 4, 2020 12:13 Inactive

maxgds force-pushed the alternative-header-layout branch from 7ddd914 to 10ae31d Compare February 4, 2020 12:31

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 4, 2020 12:32 Inactive

maxgds force-pushed the alternative-header-layout branch from 10ae31d to 4cd8250 Compare February 4, 2020 12:34

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 4, 2020 12:34 Inactive

maxgds marked this pull request as ready for review February 4, 2020 15:13

maxgds force-pushed the rebased-add-layout branch from bcde082 to 51fc63a Compare February 4, 2020 15:36

maxgds force-pushed the rebased-add-layout branch from 51fc63a to 64e79a8 Compare February 5, 2020 10:05

maxgds force-pushed the alternative-header-layout branch from 4cd8250 to 993b99c Compare February 5, 2020 13:59

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 5, 2020 13:59 Inactive

maxgds changed the title ~~Alternative header layout~~ [WIP] [broken rebase - fixing] Alternative header layout Feb 5, 2020

maxgds force-pushed the alternative-header-layout branch from 993b99c to e1b8fec Compare February 5, 2020 14:38

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 5, 2020 14:38 Inactive

maxgds force-pushed the alternative-header-layout branch from e1b8fec to 7ab881b Compare February 5, 2020 14:43

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 5, 2020 14:44 Inactive

maxgds force-pushed the alternative-header-layout branch from 7ab881b to 8d12349 Compare February 5, 2020 14:49

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 5, 2020 14:49 Inactive

maxgds changed the title ~~[WIP] [broken rebase - fixing] Alternative header layout~~ Alternative header layout Feb 5, 2020

maxgds force-pushed the alternative-header-layout branch from 8d12349 to da1f566 Compare February 5, 2020 15:40

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 5, 2020 15:40 Inactive

andysellick reviewed Feb 6, 2020

View reviewed changes

vanitabarrett suggested changes Feb 6, 2020

View reviewed changes

andysellick reviewed Feb 6, 2020

View reviewed changes

kr8n3r reviewed Feb 6, 2020

View reviewed changes

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 10, 2020 10:29 Inactive

maxgds force-pushed the alternative-header-layout branch from 21f2b16 to c345be1 Compare February 12, 2020 08:35

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 12, 2020 08:35 Inactive

maxgds force-pushed the rebased-add-layout branch from 64e79a8 to f279af1 Compare February 12, 2020 09:35

maxgds added 11 commits February 12, 2020 09:49

Add alternative header layout

9a2a113

This adds a new layout alternative that allows for search on the left of the header and navigation items on the right to support some layouts seen in whitehall pages. It also corrects a stray div that was breaking the with-nav option without a searchbox.

Linting fixes

7ae936b

Add space to prevent button icon from wrapping

32c5683

Improve legibility

9333ed2

- Concatenates the appropriate header classes before implementing them more simply - Reduces repitition in code where the search_left layout branches from the standard

Remove unnecesarry spacing

4e2a207

This spacing that was added during development isn't doing anything but introduced a minor bug when resizing the browser with the nav open, so this removes it.

Remove unnecessary layout class

9ecbfaa

Improve yml file

574aa5b

- Removes spurious space - Adds a description of where this variant of header is used

Fix linting on partials

8f2df27

Fix nav layout for small screens

1fb6c98

Clarify odd looking fallback code

3845226

Update method for hiding presentational image from AT

3f71d08

maxgds force-pushed the alternative-header-layout branch from c345be1 to 3f71d08 Compare February 12, 2020 09:49

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 12, 2020 09:49 Inactive

vanitabarrett reviewed Feb 12, 2020

View reviewed changes

Linting

2e954c5

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 12, 2020 11:29 Inactive

maxgds added 2 commits February 12, 2020 12:24

Improve spacing at mobile width

159347b

Ignore accessibility warnings caused by duplicate examples on a page

a16736c

bevanloon temporarily deployed to govuk-publis-alternativ-hvqbzz February 12, 2020 13:20 Inactive

vanitabarrett approved these changes Feb 12, 2020

View reviewed changes

maxgds merged commit 73fe8df into rebased-add-layout Feb 12, 2020

maxgds deleted the alternative-header-layout branch February 12, 2020 14:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alternative header layout #1278

Alternative header layout #1278

maxgds commented Feb 4, 2020 •

edited

Loading

kr8n3r commented Feb 4, 2020

maxgds commented Feb 5, 2020

maxgds commented Feb 5, 2020

andysellick left a comment

maxgds commented Feb 6, 2020

andysellick commented Feb 6, 2020

andysellick left a comment

andysellick Feb 6, 2020

kr8n3r Feb 6, 2020 •

edited

Loading

vanitabarrett Feb 12, 2020

vanitabarrett commented Feb 12, 2020

vanitabarrett commented Feb 12, 2020

injms commented Feb 12, 2020 •

edited

Loading

maxgds commented Feb 12, 2020

maxgds commented Feb 12, 2020

Alternative header layout #1278

Alternative header layout #1278

Conversation

maxgds commented Feb 4, 2020 • edited Loading

What

Why

Screenshot

kr8n3r commented Feb 4, 2020

maxgds commented Feb 5, 2020

maxgds commented Feb 5, 2020

andysellick left a comment

Choose a reason for hiding this comment

maxgds commented Feb 6, 2020

andysellick commented Feb 6, 2020

andysellick left a comment

Choose a reason for hiding this comment

andysellick Feb 6, 2020

Choose a reason for hiding this comment

kr8n3r Feb 6, 2020 • edited Loading

Choose a reason for hiding this comment

vanitabarrett Feb 12, 2020

Choose a reason for hiding this comment

vanitabarrett commented Feb 12, 2020

vanitabarrett commented Feb 12, 2020

injms commented Feb 12, 2020 • edited Loading

maxgds commented Feb 12, 2020

maxgds commented Feb 12, 2020

maxgds commented Feb 4, 2020 •

edited

Loading

kr8n3r Feb 6, 2020 •

edited

Loading

injms commented Feb 12, 2020 •

edited

Loading