Problem/Motivation

The header of Claro is always rendered even when the regions (page.breadcrumb & page.header) are have no content in it.

Because of this the background and the background color of the header is always rendered which is odd.

Steps to reproduce

Assign no content to the breadcrumb and header regions in Claro, you'll see the background is show on screen.

Proposed resolution

Add a check to see if one of the regions (breadcrumb or header) is filled, if not don't render the header at all.

CommentFileSizeAuthor
#10 Afer MR 9340.png160.55 KBbebalachandra
#9 Block layout for claro theme.png251.05 KBbebalachandra
#9 Before MR9340.png150.36 KBbebalachandra
#6 after-patch.png8.28 KBprashant.c
#6 before-patch.png10.33 KBprashant.c
#3 header-is-always-shown-in-claro-even-when-regions-in-it-are-empty-3472319-3.patch833 bytesf0ns
Scherm­afbeelding 2024-09-05 om 14.33.59.png2.62 KBf0ns

Issue fork drupal-3472319

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

f0ns created an issue. See original summary.

f0ns’s picture

f0ns commented
Title: Header is always shown in Claro even when are empty. » Header is always shown in Claro even when regions in it are empty
f0ns’s picture

f0ns commented
StatusFileSize
new header-is-always-shown-in-claro-even-when-regions-in-it-are-empty-3472319-3.patch833 bytes

This patch fixes this behaviour for me

cilefen’s picture

cilefen commented
Version: 10.3.x-dev » 11.x-dev
Component: theme system » Claro theme
Status: Active » Needs work
Issue tags: +Needs merge request

prashant.c made their first commit to this issue’s fork.

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented
Status: Needs work » Needs review
Issue tags: -Needs merge request
StatusFileSize
new before-patch.png10.33 KB
new after-patch.png8.28 KB
  • @f0ns Assigning no regions, do you mean from the themes info file or removing the content assigned to the regions?
  • Because I replicated this by removing/disabling the blocks and content to all the regions and not by removing regions.
  • The Submitted path does solve the issue, creating MR using the same.

Before:
Before patch

After:
After patch

However, I think appropriate conditions could also be added for the remaining part as well for the {{ page.pre_content }}{{ page.highlighted }}<code> and {{ page.content }}:

<div class="layout-container">
      {{ page.pre_content }}
      <div class="page-content clearfix">
        {{ page.highlighted }}
        {% if page.help %}
          <div class="help">
            {{ page.help }}
          </div>
        {% endif %}
        {{ page.content }}
      </div>
  </div>

prashant.c opened merge request !9430

f0ns’s picture

f0ns commented
Issue summary: View changes
bebalachandra’s picture

bebalachandra commented
StatusFileSize
new Before MR9340.png150.36 KB
new Block layout for claro theme.png251.05 KB

Fixes verified on 11.x-dev.

I was able to reproduce the issue on my local environment. After applying fix from the MR!9340 issue resolved.

Steps followed for reviewing:
1. Installed 11.x-dev on local.
2. Navigate to admin > structure > Block layout
3. Make sure you are on claro theme tab for block placement.
4. Disable all the blocks which are placed in Header and Breadcrumb region.
5. Navigate to home page and check for the header.

After the MR!9340 header is not shown when the region is empty.

Attached screenshots for reference.

Issue mentioned is resolved after the MR. I suggest move this issue to RTBC

bebalachandra’s picture

bebalachandra commented
StatusFileSize
new Afer MR 9340.png160.55 KB

Missed uploading after MR screenshot in previous comment

markconroy’s picture

markconroy commented
Status: Needs review » Reviewed & tested by the community

Tested on Drupal 11, and is working as expected with this patch.

---
Thanks to The Confident for sponsoring my time to work on this.

f0ns’s picture

f0ns commented

Thank you for testing, looks good to go.

Concerning comment #6, personally I don’t see the need to add extra control statements to other regions as these elements have no containers that have any css markup in core.

nod_ closed merge request !9430

nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 5360e3bdb08 to 11.x and 946c580a097 to 11.0.x and bca9ac9378f to 10.4.x and 0a4577802ac to 10.3.x. Thanks!

  • nod_ committed 0a457780 on 10.3.x 
    Issue #3472319 by prashant.c, f0ns, bebalachandra, markconroy: Header is...

  • nod_ committed bca9ac93 on 10.4.x 
    Issue #3472319 by prashant.c, f0ns, bebalachandra, markconroy: Header is...

  • nod_ committed 946c580a on 11.0.x 
    Issue #3472319 by prashant.c, f0ns, bebalachandra, markconroy: Header is...

  • nod_ committed 5360e3bd on 11.x 
    Issue #3472319 by prashant.c, f0ns, bebalachandra, markconroy: Header is...
nod_’s picture

nod_
Primary language French
Location Lille
commented

.

Status: Fixed » Closed (fixed)

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