Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The homepage has an empty block title with an empty h1.
Steps to reproduce
- Install drupal and create content
- Visit the homepage.
- Notice that the homepage block exists and has an empty h1
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | 3177231-21.patch | 494 bytes | markdorison |
| #7 | Снимок экрана 2020-10-21 в 12.20.39.png | 335.47 KB | kostyashupenko |
| #6 | 3177231-6.patch | 473 bytes | kostyashupenko |
| #3 | olivero_3177231.png | 23.9 KB | nitesh624 |
| #3 | 3177231-3.patch | 393 bytes | nitesh624 |
Comments
Comment #2
nitesh624working on it will update in few hours
Comment #3
nitesh624After applying the patch
Comment #4
mherchelComment #5
mherchelThere's also the empty page title block div, along with the shortcut wrapper. However if we remove the page title block div, the entire layout shifts, which is undesirable. We may need to make some CSS changes too.
Comment #6
kostyashupenkoRight now margins added for block level, not for page_title, and block level now exist after this patch still, but empty. I have checked - we have the same issue in Claro theme, page title is empty with empty h1 inside. So... If we can keep empty div block-page-title, then this patch is enough. But.. If there is no real title, then block-page-title shouldn't be rendered, correct? If yes, then looks like this is issue of core, not really of Olivero.
Comment #7
kostyashupenkoComment #8
mherchelThe best way to handle this is to change the configuration to remove the title block from the frontpage. This would negate the need for the preprocessing, and it would also allow the site-builder to re-enable the title on the frontpage if they wish.
Unfortunately, this will require a configuration change, which I'm unsure if we can do... although it may be possible before the first release.
I talked with @lauriii in Drupal slack, and he said to post a patch.
Note that the changes in the CSS are to compensate for the block div not being present (with its margin), and also note in preprocess the
shortcut-wrapperdiv is not needed (pretty sure that was copied over from another theme while initially setting up the scaffolding).Comment #9
mherchelThe previous patch unintentionally removes the title block on the "Welcome to Drupal" getting started page.
This patch fixes that by inserting the title into the template (and fixing the h1 font and compensating for the increased margin).
Comment #10
mherchelComment #11
mherchelTalked with @lauriii a bit more in Drupal slack. The approach in #9 is a little weird, because if a site-builder edits the frontpage view to create a title, it still will not show. In addition that solution doesn't solve the problem for other views that might have a blank title.
Comment #12
mherchelI'd really like to prevent the block and region markup from showing. However, I keep on running into #953034: [meta] Themes improperly check renderable arrays when determining visibility.
Unfortunately, it's not easy to check if data exists without having unintended consequences. I think the best solution at this point is #6.
So, RTBC #6
Comment #13
xjmGenerally one should reupload the intended patch as the final patch to avoid committers accidentally committing the wrong thing. For now, I'm at least hiding the newer patches.
Comment #14
lauriiiEven though the approach in #6 isn't ideal, it's probably the pragmatic way to go. I think we should use the same approach with Umami which is
{% if title|render|striptags|trim %}.Comment #15
ayushmishra206 CreditAttribution: ayushmishra206 at OpenSense Labs commentedMade the change suggested in #14.
Comment #16
mherchel@ayushmishra206 we need to go with the approach in #6 with what @lauriii mentioned in #14.
Comment #17
ayushmishra206 CreditAttribution: ayushmishra206 at OpenSense Labs commentedComment #18
mherchel@ayushmishra206 See my previous comment. I appreciate the work that you're doing, but you're working on an approach that will not be accepted. Please read through my comments before posting a patch.
Comment #19
markdorisonHiding the patches from #15 and #17 to avoid confusion.
@mherchel You marked this issue as RTBC in reference to the #6 patch. Is that the case from your perspective?
Comment #20
mherchelCorrect, however @lauriii (in comment #14) mentioned that we should use
{% if title|render|striptags|trim %}instead of|render. Should be a quick change.Comment #21
markdorisonComment #22
walangitan CreditAttribution: walangitan at Chromatic commentedPatch in #21 applies cleanly in my testing. Tests also passed, marking this as RTBC.
Comment #23
mherchel+1 on the RTBC!
Comment #26
lauriiiCommitted cee06eb and pushed to 9.2.x and 9.1.x. Thanks!
Comment #27
jibran{% if title|render|striptags|trim %}should these three function calls be converted to a helper method? It is pure drupalism and one should not have to remember this.