New regions needs to be added in order to implement CSS for the design properly in this task: https://www.drupal.org/node/2764233.

New regions should be:
'brand'
'top_navigation'
'primary_navigation'
'search'
'secondary_navigation'
'messages'
'footer_bottom'

Comments

adoo created an issue. See original summary.

adoo’s picture

Issue summary: View changes
adoo’s picture

Issue summary: View changes
adoo’s picture

StatusFileSize
new2.63 KB

I added regions.

adoo’s picture

StatusFileSize
new2.63 KB
adoo’s picture

Issue summary: View changes
StatusFileSize
new2.65 KB

Change copyright region to footer_bottom

adoo’s picture

StatusFileSize
new3.65 KB

Change order of the regions, add container wrappers.

adoo’s picture

Status: Active » Needs review
davidhernandez’s picture

+++ b/templates/page.html.twig
@@ -51,36 +51,77 @@
 <div class="layout-container">

Most of these changes are fine. We can probably get rid of the main layout-container. That came with the template from Classy, but if it isn't needed we can remove it.

+++ b/templates/page.html.twig
@@ -51,36 +51,77 @@
-  {% if page.footer_first or page.footer_second or page.footer_third %}
+  {% if page.footer_first or page.footer_second or page.footer_third or footer_bottom %}
     <footer role="contentinfo">

I talked to Adrian about this and we agree that we could get rid of the 'if' but it might be good to leave it as an example.

adoo’s picture

StatusFileSize
new3.37 KB

Remove layout-container div, change the markup and add container to the menu and brand.

davidhernandez’s picture

Assigned: adoo » Unassigned
Status: Needs review » Reviewed & tested by the community

This looks fine. There are a couple things I'd like to do different, but we can follow up with that. Right now I want to get the regions in so we can work on the CSS.

Also, @adoo, I'm not too found of these 'if's for adding classes.

+++ b/templates/page.html.twig
@@ -48,18 +48,55 @@
+  <section class="header-wrapper
+    {% if page.header_first %} with-header-first{% endif %}
+    {% if page.header_second %} with-header-second{% endif %}
+    {% if page.header_third %} with-header-third{% endif %}">

...but we can leave them for now. One thing I don't like is that this leaves a lot of extra white space in the class attribute. You can see it in the page source. I added dashes in the if to remove the extra whitespace. You can see that after it is committed.

I also updated to comment block with the new regions.

  • davidhernandez committed e93ab84 on 8.x-1.x
    Issue #2764857 by adoo, davidhernandez: Add regions for the header...
davidhernandez’s picture

Status: Reviewed & tested by the community » Fixed
adoo’s picture

Status: Fixed » Needs review
StatusFileSize
new456 bytes

Change a class in the template.

adoo’s picture

Status: Needs review » Fixed

Will open new issue for the the class change.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.