Remove Page Title block from homepage

Applied patch and ran a fresh install. Works for me.

@imalabya it looks like if you add:

module:
- system

under dependencies, it should pass

Patch. Working on interdiff

Thank you for the patch with the fix, @benjamindamron. We'll see what the test bot thinks of it. The change looks good based on the differences suggested in the failing test.

I wonder if there's a related bug with other umami configuration based on core changes in 8.6.x or 8.7.x?

#7 applies correctly.

This to me, seems a debatable thing -
Whether to have "Home" displayed on the homepage to give users a sense of the page they're in. Also, If we see the Article & Recipe pages, these titles makes absolute sense there. Whereas on the other hand, not having "HOME" title makes sense in the context of the flow of content.

BEFORE:

AFTER:

I'd wait for others opinion on this. 👍Keeping status as is.

PS: nice find.

Adding the before/after screenshots to the issue summary.

I added @JayKandari's comment about further review/introspection to the issue summary. I'm not sure if it's appropriate to tag "Needs product manager review" at this point. Maybe best to ping one of the OotB initiative coordinators?

Removed Novice tag as I do not think there is anything more actionable to do as we have a working patch and before/after screenshots.

Discussed on initiative call. *Home* used to say *Recipes*. This was changed in a previous issue to set the page title to Home. However, the placement on the page of this element is designed to give a title to the specific section below, which is Recipes.

Therefore solution required to keep the page title as Home but have a heading reading Recipe here. Whether this is H1 / H2, and if not H1 where else we can put a H1 on the page to be discussed.

Thank you for relaying and providing the direction, @Eli-T. I think that whichever approach should use the correct semantic markup.

The Page title block may need to be disabled for the home page to do this, but another instance could be added to move it to a different region and add a heading to the Home page content to mention Recipes.

I've updated the issue summary to remove outdated screenshots and change the resolution per #13.

I think this is still a good Novice candidate. Specifically for providing a new patch that passes tests and also to provide screenshots. It's also a good opportunity to provide direction to site builders or evaluators to see how blocks can be disabled and/or reused.

If you want to continue work on this tomorrow I'll be around a bit after you on the VCD2018 schedule to review or help, @imalabya.

On the same call with @eli-t I mentioned that I don’t think it’s uncommon for sites to not display a main page title (h1) in the body of the home page. Instead, relying on an h1 around the brand/logo/site name, which is correct (perhaps) as a content hierarchy for a home page that is presenting all sorts of content types.

If we follow this approach of removing the main page title on the home page (title block), then I think a possible fix is to add the ‘Recipes’ title back in as an <h2>Recipes</h2> heading in the View header field that is currently providing the suffix text of ‘Explore recipes across every type of occasion, ingredient, and skill level.’ and simply use whatever method necessary to hide the page title on front.

I think this would ensure that the <title> element continues to display ‘Home’.

Tagging for Drupal Europe contribution days. We should update the issue summary based on @kjay's comments in #15 which followed Slack discussions last week in the #out-of-the-box.

Edit: Also update the title of the issue based on the new scope.

I'm contributing to this issue and I will be updating the issue summary based on the comment @kjay #15

Working with @bruceyboy on this

Joining @bruceyboy and @sysosmaster.

Myself, @sysosmaster, @mradcliffe and @philippze have added a patch.

It adds a function to the umami.theme file which determines if the current path is the homepage and gives the is_front variable to the hook.
source of function: https://www.drupal.org/node/2830442

This patch is an interdiff of @benjamindamron's patch in comment #7 of this issue.

We discussed weather the string in the H1 should be dynamic but the homepage always says home in the site settings so this was irrelevant. The H1 element uses the class "visually hidden" which uses display: absolute but (importantly) not visibility: hidden. This means the screen reader can pick up the H1 string as it will be relevant in the DOM.
We decided the element should be positioned underneath the logo for the sake of DOM hierarchy.

There was a problem with describing the front page in the if statement which is why we decided to add the function (see link above). This is useful for other elements using twig templates with the block hook (see umami.theme in the patch). Alternatively this can be used to preprocess other hooks.

Here is the said patch from above...

I verify that patch #23 hides the title block from the content area while still keeping an invisible (visually-hidden) H1 title in the page.

Not putting into RTBC until tests and screenshots get ready.

This adds the Test to the #22
work done by @Sysosmaster, @bruceyboy

Thank you for updating the issue summary, @philippze! I think we're not solving the #2: We display "Recipes" above this view instead. We achieve this with changing the view header in this issue, but in #2996661: [PP-1] Add a heading & subheading for the Promoted section. And then we should remove the Needs Issue Summary Update tag when done.

--

@bowen111 is looking to create before and after screenshots:

1. A before screenshot without the new markup.
2. 2 after screenshots on the home page one displaying the new markup and one displaying no Home title below
3. 2 after screenshots on a non-home page one displaying that no new markup is there and one displaying the main page title below.

1. Before, home page, title displayed incorrectly

   

2. After, home page, title removed, visibly hidden h1 added at top of page

   

3. after, articles page, page title shows correctly

   

Thank you for the patch @sysosmaster, @bruceyboy. I think general approach works here, but there are some styling issues I've noted below in the most recent patch. To fix these I think it would be fine to fix them only in the combined patch.

1. +++ b/core/profiles/demo_umami/tests/src/Functional/DemoUmamiProfileTest.php
   @@ -169,4 +169,37 @@ public function testDemonstrationWarningMessage() {
   +   * Test that validates that the 'h1' element in system-branding is present on
   

   The first line of the dock block should be one line.

   Maybe something like "Tests the presence of an 'h1' element on pages."

   Also the second sentence is greater than 80 characters.

2. +++ b/core/profiles/demo_umami/tests/src/Functional/DemoUmamiProfileTest.php
   @@ -169,4 +169,37 @@ public function testDemonstrationWarningMessage() {
   +   *
   +   * -- Note
   +   * This test should be moved (together with some of the other tests in here)
   +   * into a separated file since they are not testing the 'installation' of the
   +   * profile but the internal workings of the profile.
   

   We usually add an @todo markup for this, but that requires a follow-up issue. I think it would be okay to remove this comment now.

3. +++ b/core/profiles/demo_umami/tests/src/Functional/DemoUmamiProfileTest.php
   @@ -169,4 +169,37 @@ public function testDemonstrationWarningMessage() {
   +    //Load in frontpage
   ...
   +    //test if we only have 1 h1 element.
   ...
   +    //test if h1 element has the 'visually-hidden' class.
   ...
   +    //test if we only have 1 h1 element.
   ...
   +    //load login page
   ...
   +    //test if h1 element has the 'visually-hidden' class.
   

   Comments should have a space between // and the first letter, have a capital letter and end in a full stop (period).

4. +++ b/core/profiles/demo_umami/tests/src/Functional/DemoUmamiProfileTest.php
   @@ -169,4 +169,37 @@ public function testDemonstrationWarningMessage() {
   +//    $this->drupalGet("/");
   

   We should remove this commented out code.

5. +++ b/core/profiles/demo_umami/themes/umami/templates/components/branding/block--system-branding-block.html.twig
   @@ -19,6 +19,9 @@
   +    <h1 class="visually-hidden">Home</h1>
   

   The text "Home" should be run through the twig t function.

   {{ 'Home'|t }}

Updated the Test code and the missing t function call.
done by @Sysosmaster

I've confirmed the changes in the patch. I think that we could reword the doc block comment below:

+++ b/core/profiles/demo_umami/tests/src/Functional/DemoUmamiProfileTest.php
@@ -171,35 +171,25 @@
+   * This test validates that  the 'h1' element in system-branding is present on
+   * the front page.
+   * And absent on the other pages.

Let's try to combine this into one paragraph.

   * This test validates that  the 'h1' element in system-branding is present on
   * the front page, and absent on the other pages. It also validates that it
   * has the visually hidden class.

change the comment.
@Sysosmaster

forgot to set "needs review"

If you think the issue affects screen reader users, please remember to add the accessibility tag

I don't think this is a good approach for accessibility. If you don't think a sighted user needs a heading called "home" halfway down the page, what makes you think a screen reader user needs it nearer the top of the page?

The page title should remain on the front page for screen readers and search engines.

Reading the issue, I don't see how you've arrived at this. It seems to me there's been an assumption that you should always do this, but the reasons been misunderstood.

For sure, visually-hidden headings can help a screen reader user enormously. Typically they are useful to help a visually impaired user find a region of the page, that a sighted user would be able to find by some visual affordance (size, layout, etc.). For example, a breadcrumb navigation typically has a distinct visual presentation which is easy to home in on (one line, full-width, whitespace above and below, visible links, and some arrow separators). A visually-hidden heading provides an alternative prominent way to find it. Another example would be a shopping cart info block, which is often visually presented as a cart icon link with a number, in the top corner. A visually-hidden heading and text alternatives make it easy to locate and understand with a screen reader.

However... it's OK to remove a heading completely, if no-one needs it!

So whenever you put a visually-hidden heading in place, consider why it's necessary. What benefit does it provide for screen reader users; what visual affordance does it tie in with? Turning the question around, why doesn't a sighted user need a heading called "home" in the branding block, how do they know it's the home page?

• A sighted user can see the word "home" in window/tab title, from the <title> element. This is also apparent to screen reader users. This title is often the first thing that gets announced on page load (maybe depending on screen reader preferences), and screen readers can announce the window title on demand too. The title is also available to screen reader users when switching between window/tabs or applications.
• Sighted users can tell it's the home page by looking at the browser URL bar, which has no path/query after the domain. This fact is also equally available to screen reader users.
• The lack of a prominent breadcrumb is another clue that this is the homepage, but it may not be immediately obvious to sighted users or screen reader users. The absence of a breadcrumb is more obvious when you've just come come from a previously a deeper page which had one. Breadcrumbs have their own visually-hidden H2 and properly labelled landmark region, so it has a comparable level of prominence (or abscence...) to screen reader users.
• Sighted users CANNOT tell it's the home page just by looking at the branding block. So there's no visual affordance that a visually-hidden "home" heading would make up for.
• Sighted users can quickly tell that it's the homepage by lookig at the main menu, because the "home" link is picked out as the active one. But only at wide breakpoint, they'd have to look inside the burger menu to learn this at the narrow breakpoint. Screen reader users can't tell this from the main menu. Using aria-current="page" on the main menu link would help here. It's well supported, and has a direct equivalence to the sighted experience. I'll file a core feature request for that, all Drupal sites would benefit form it, it's already mentioned on #2893640: Modernize ARIA usage, in line with ARIA 1.1 and the ARIA Authoring Practices guide..
• You could look at the whole header region at once, and argue that the active menu link is a visual affordance which needs a screen reader fallback somewhere in the header region. I might buy this as an idea, but I don't think it has much weight. The previous suggestion to use aria-current is a more robust approach.

Proposed resolution:
[...]
2 We wrap the brand/logo/site name in the system-branding block on the front page into an h1, and put the page title ("Home") there as a correct semantic element following accessibility standards.

This sounds confused. It suggests you've considered using the site name as a H1, but that isn't what the patch in #33 does. Standards-wise, it's not technically a WCAG failure to omit a H1 from a page, though technique G141: Organizing a page using headings (and some testing tools) strongly point you in that direction.

If there's going to be a H1 on the homepage, "home" isn't very useful - the site name would be better. I do recall a recent conversation on Slack about making a conditional H1 on the branding block, but my assumption was that this would be the site name, not "home". Having said that, we don't have a conditional heading for the site branding in Standard profile for D7 or D8. So why would we depart from this in Umami?

The whole branding block could be simplified into H1 (homepage only) > A > IMG (alt = Umami Food magazine). This will be easier once #2780293: Add GUI to configure the site's logo alt attribute is in.

Aside: the visually-hidden site-name link should be removed. It's a failure of WCAG "Focus Visible". Never put the visually-hidden class around operable controls as a rule-of-thumb. Do we need a separate issue about this? We can do it very quickly here by unchecking the site name in config, but it's not in the current issue scope.

Accessibility aside, does SEO actually need a H1 to indicate "home"? The text is already in the <title>, and the lack of a path component in the URL is a very strong indicator of the home page.

The original issue title was a better plan. Don't keep a low-quality heading around just because you think you're supposed to provide one.

Only provide a visually-hidden heading as an alternative means to locate or identify a section of the page that's visually evident to a sighted user. You don't need a heading to locate the start of the page.

I think that we could have three different outcomes here based on @andrewmacpherson's accessibility review:

The whole branding block could be simplified into H1 (homepage only) > A > IMG (alt = Umami Food magazine).

If the h1 is important, then I think having the h1 element wrap the site logo anchor element makes sense. Then the scope of the issue would continue to be about the branding block / logo and so I think that we could remove the visually-hidden site-name link from config as well. Does doing the first part still have value for evaluation and demo purposes if the recommended approach would be #2780293: Add GUI to configure the site's logo alt attribute?

If it does not, then the scope of the issue returns to the original proposed resolution (solved with the patch in #7). Also @eli-t and @kayj mentioned that there may not be any value in any additional tests for umami.

Removing Novice tag until scope review is finished.

I worked with @kjay yesterday to hash out the scope of the issue based on @andrewmacpherson's accessibility review.

1. Hide the page_title block on the front page using the patch in #7.
2. Make sure that the "Recipes" heading is in the same location as a h2. This was originally proposed as #2996661: [PP-1] Add a heading & subheading for the Promoted section, but @kjay mentioned that issue as described would not work for the proposed design, and would like that to be solved in this issue.
3. Create a follow-up issue to address accessibility review of the site branding block if having a h1 element on the front page is necessary for SEO.

To do #2, @kjay suggests "adding a tiny bit more content (a title) to the existing View Header content field."

Working on this at DrupalEurope mentored sprint in Darmstadt, Friday 14 September 2018

I'm working with Michael on that issue.

I've just updated an issue summary to delete out of date requirement to replace 'Home' with 'Articles' (see #15 for more information).

Worked on with @sysosmaster and @madmaxDK
during contribution day at drupalEurope 2018

We worked on todo #2 as suggested by @mradcliffe in comment #39.

We added "Recipes" to the view header on the homepage view header test area.

The title is an H2 and the blurb of text has been positioned below with the same CSS class to align centre. The text has also been wrapped in an H3 to keep design consistent, as requested in issue https://www.drupal.org/project/drupal/issues/2996661 by @imalabya.

Changed status to needs review

Hi bruceyboy - we are working on this as well, can you come find us at DrupalEurope? We are in the center of the mentored sprint room I have a mentoring ribbon wrapped around my head :)

Can we get a first time sprinter onto this now to provide screenshots and test the patch applies correctly. Would definatley be a nice RTBC and live commit to core.

I have tested the patch #44

Screenshot before:

Screenshot after:

There is h1 tag inside:

There is only one small thing to fix.

You need to delete '.' in the end of subheader.
Line 45 in patch #44.

Because, I think, there are no periods in headers and subheaders.

I can confirm the H1 appears in the branding block for the front page only. I found an empty <h2> in the new Recipes views header, stemming from a badly closed <h2> tag.
I've fixed the closing tag on line 45 and am attaching an updated patch. I've also now removed the period.

The old:

value: "<h2 class=\"text-align-center\">Recipes<h2/>\r\n<h3 class=\"text-align-center\">Explore recipes across every type of occasion, ingredient, and skill level.</h3>"

The new:

value: "<h2 class=\"text-align-center\">Recipes</h2>\r\n<h3 class=\"text-align-center\">Explore recipes across every type of occasion, ingredient, and skill level</h3>"

The issue with "." was fixed:

Thanks for fixing broken h2 tag!

Thank you for testing and reviewing the patch, @Yaremchuk. It would be great to embed the screenshots that you took in #48 and in #51 when you make the comment.

I've gone ahead and done that.

Thanks for following up on the patch, @bruceyboy, @anthonylindsay. I found some issues in the current patch below.

1. +++ b/core/profiles/demo_umami/tests/src/Functional/DemoUmamiProfileTest.php
   @@ -169,4 +169,26 @@ public function testDemonstrationWarningMessage() {
   +   * Test to validate proper hidden `h1` working for the title element.
   ...
   +   * This test validates that  the 'h1' element in system-branding is present on
   ...
   +    // Load in the `front` page.
   ...
   +    // Load the `login` page.
   

   I wasn't sure if backticks were allowed in the API Documentation, but I couldn't find a specific thing against it. And I did find that it was used in a couple of other tests to quote things so I think that this will help make it readable.

   This is OK.

2. +++ b/core/profiles/demo_umami/themes/umami/umami.theme
   @@ -50,6 +50,17 @@ function umami_preprocess_block(&$variables) {
   +   // Set a variable based on the path.matcher.
   

   There is an extra leading space on this line that needs to be removed.

I've updated the issue summary Remaining tasks to reflect the status of the issue.

Addressing the issues from #53
- Using single quotes instead of backticks
- Removed unwanted whitespace for comment

@micheallenehan you got there seconds before me! :)

Patch looks good to me. Back-ticks and whitespace are gone.

Thank you, @michaellenahan, @anthonylindsay.

Hang on, re: #54/56, which patch is the RTBC for?

The issue was re-scoped in #39 but the issue summary doesn't reflect that, and neither do the patches in #54 and #56.

Per #39, We're NOT doing anything with the site branding block in this issue...

3. Create a follow-up issue to address accessibility review of the site branding block if having a h1 element on the front page is necessary for SEO.

This conflates SEO with accessibility, which isn't helpful. There are certainly accessibility issues with the branding block, which have nothing to do with SEO. Let's treat those as separate follow-up issues please.
There was a lot of discussion on Slack about whether the homepage needs a H1 in the branding block at all, let alone a visually-hidden one called "home". I'm against it.

Updated the proposed resolution in the issue summary.

TODO: update remove all the stuff relating to H1 in the branding block.

+ value: "<h2 class=\"text-align-center\">Recipes</h2>\r\n<h3 class=\"text-align-center\">Explore recipes across every type of occasion, ingredient, and skill level</h3>"

This H3 isn't introducing any content, because it's trumped by a H2 right afterwards in the recipe card. A paragraph tag would do here.

The "recipes" H2 is supposed to introduce the series of recipe cards, so these should be a level below.

As of #56, the outline is:

H Recipes
    H +1 Explore recipes ... (but no content under this level)
H Fiery chili sauce
H Vegan chocolate brownies
...
H Recipe collections

This is what the visual design implies though:

H Recipes
    P Explore recipes ... (but no content under this level)
    H +1 Fiery chili sauce
    H +1 Vegan chocolate brownies
...
H Recipe collections

updating tasks, about the branding block.

Tagging for upcoming contribution days.

Updated patch remove all h1-related code per #36/#59 and address the hierarchy issues brought up in #60.

@andrewmacpherson I see that you created the issue for site branding accessibility issues in #3000724: Fix accessibility problems in Umami's branding block. I can't find an issue for whether we need an h1 on the homepage. Has that been created and if not do we still need to have that discussion? (Personally, once #2780293: Add GUI to configure the site's logo alt attribute lands, I would wrap the logo in an h1 and call it a day. But opinions may vary.)

If we've created all the necessary follow-up issues I was going to cross that off the remaining tasks so that no one creates any duplicate issues. (Not that I almost did that or anything.)

Tagging for DrupalNorth 2019

Hi,

h1 not visible, "Recipes" on h2 and the sub-text just in

. All is fine for me.
Patch tested on drupal-8.8.x-dev.

Hi I post a comment yesterday but I don't see it now, So I post it again.
All seem fine to me on drupal-8.8.x-dev.

@fabienly Since you tested and approved the patch, I'm changing the status from "Needs review" to "Reviewed & tested by the community" so that this issue can move forward. Thanks for testing!

After discussing this issue with @smaz and @markconroy, we recommend that @kjay will find a (minimal?) way to change the design/layout/structure of the homepage to address all the comments in this issue and create new issues if needed in order to have a clear path to a complete solution.

How about following the work in #56 which drops the h1 title and further removing the 'Recipes' title and just styling up the text intro to suit the presentation on the front page. This removes the need for hierarchy changes.

As per the attached (including screenshot).

@kjay Thank you!
The patch #73 works great.

Looking at the code, there's a small change I think we should make -
The frontpage view, has a disabled 'Feed' display, which seems like a clone of the 'Page' display,
/en/admin/structure/views/view/frontpage/edit/feed_1

We should update the Global Text area in the 'Feed' display as well, changing <h2> to <p> .

Following @shaal's observation, new patch attached. This time I've exported the view instead of editing the patch!

Patch #73 looks good to me. Its works fine. Thanks

@Gnanasampandan Velmurgan Thank youfor #76!

For a more complete review, can you please provide more details on the review?
(steps you took to test it, screenshots of how it looks after the patch, etc)

After writing that, if it works as you expected, you can change the status (under "issue metadata") from "Needs Review", to "Reviewed & tested by the community" (RTBC)

Thanks again for all your work!

"Home" displayed on the homepage to give users a sense of the page they're in. Also, If we see the Article & Recipe pages, these titles makes absolute sense there. Whereas on the other hand, not having "HOME" title makes sense in the context of the flow of content.

@shaal
As per Review the Patch looks like
1) "Home" title displayed on the homepage to give users a sense of the page.
2) The Article & Recipe pages, not having "HOME" title makes sense in the context of the flow of content. Also attached the screenshot for References

#75 no longer applies.

Rerolled the patch #75.

Start testing at DrupalCon Amsterdam

I'm not sure how it's actually meant to be. How it is at the moment:
- Homepage: No H1 title on this page and no h2 title before the recipies teasers
- Article/Recipies Pages: H1 is in place
Hope this helps.

Thank you @quaxi for the review.

According to @andrewmacpherson in #59

There was a lot of discussion on Slack about whether the homepage needs a H1 in the branding block at all, let alone a visually-hidden one called "home". I'm against it.

So the plan for this patch was to remove H1 from homepage, and keep it in any page where it make sense (which are all pages that are not homepage in that case...)

Changing the status to RTBC.

I'm not sure this is RTBC because a core committer might be confused by the issue summary. I was confused by the issue summary mentioning the h1 code changes be removed, but the latest patch by @shashikant_chauhan removes the h1 as demonstrated in the screenshots by @quaxi because the page title block is removed from the home page. I tried to reword this, and updated the screenshots from @quaxi to the issue summary.

Also I think we still need to create the follow-up issues before this is RTBC, right?

Fixes screenshot HTML.

I'm adding this issue to the agenda for our next OOTB call (Tuesday) to discuss which follow-up issues are still relevant.

Updating the issue summary to reflect the work in the latest patch. Will discuss follow up issues and review the patch in the next OOTB call.

This issue was discussed during last week's OOTB call and we agreed that no further follow-up tasks on SEO are required since we believe that this patch does not present any 'required' new tasks.

Updating the issue summary accordingly and marking as RTBC.

Thanks everyone :)

Committed and pushed d7d6c95777 to 9.0.x and 1f6428fdf3 to 8.9.x. Thanks!