Add comments for skip link and main-content anchor in templates

main-content is the anchor used for the "Skip to main content" link. This is to make it easier for keyboard only usrs to move focus to the beginning of the main content.

.element-invisible is a system class that makes elements invisible, but keeps them available for screen-reader users.

@droplet

Are their guidelines for when comments are required in templates?

.element-invisible is well documented

The anchor is pretty self explanatory for people familiar with HTML. I suppose the only missing connection is what brings focus to the anchor.

From modules/system/html.tpl.php

    <div id="skip-link">
      <a href="#main-content" class="element-invisible element-focusable"><?php print t('Skip to main content'); ?></a>
    </div>

Should this be documented in page.tpl.php?

Clarifying title.

I don't really think that this needs comments, but I'm happy to review any patch that is submitted.

I think it would be worth documenting the purpose of the skip link (and the target anchor) with inline comments in the template files.

As Droplet suggests, themers can remove it carelessly. See #1393062: Should the "Skip to main content" link be removed?.

In that issue, a site owner who didn't know the purpose of the link deliberately commented it out of the template.

Affects Bartik and system.module too, not just Seven theme.

@andrew - can you come up with a patch?

Isn't this just in this system file now? /core/modules/system/templates/html.html.twig

I couldn't find html.html.twig in a Core theme.

Could be just adding it to the other documentation on top of the page. That would probably be the easiest. At this point there isn't any description of what skip-links are or how they work.

How about extending it like this:

<a id="main-content" tabindex="-1"></a>{# Used by link is in html.html.twig for skip-link accessibility #}

Basically we don't want themers just deleting this line. Providing context will help him see it stays there.

provided a patch with a comment as proposed in #11.

This looks good, other than accessibility was spelled incorrectly. It should be:

{# keyboard navigation / accessibility link to main content section in page.html.twig #}

Hi mgifford,
thanks for checking, I'm not native speaker, so missed that. Corrected patch attached.

Looks good then.

I think because this is docs it should be a proper sentence (start with a capital and end with a period) per https://www.drupal.org/node/1354#drupal.

Also the comment goes over 80 characters, it should probably be a block-type comment in the Twig template, see https://www.drupal.org/node/1823416#comments.

Thanks!

Hi Scott,
thanks for your guidance, much appreciated, hopefully I got it right this time...

The patch in #17 meets twig coding standards for long comments and applies cleanly. Looks good, thanks heliogabal.

The equivalent templates in the System module and Stable themes would also benefit from this doc comment.
Let's do those in the same patch.

Adding standard issue summary for whoever reviews + commits this.

Based on #17 comment, create patch.
Add the comment to:
core/modules/system/templates/html.html.twig
core/themes/classy/templates/layout/html.html.twig
core/themes/stable/templates/layout/html.html.twig

Looks good, latest patch documents the skip link in all versions of html.html.twig, as I suggested. Applies cleanly to 8.1.x and 8.2.x branches.

Since this now updates several themes, I'm changing the component to theme system, and tagging so stable + classy maintainers still see it.

Stable and Classy don't normally accept changes, but this is pure documentation so it should be OK.

Assigning for committer (and Stable maintainer) review.

Sorry for the delayed review.

The spaces around the slash threw me off, I did a bit of digging and based on https://www.drupal.org/drupalorg/style-guide/content (which you can get to via https://www.drupal.org/node/1354#drupal) and http://www.apvschicago.com/2011/06/slashes-uses-and-restrictions.html I don't think it makes sense to have spaces on either side of the slash here. They're not lines of poetry or open compounds so let's remove the extra spaces :)

Since 8.2.x has diverged in the surrounding code we will need two patches: One for 8.2.x and one for 8.1.x.

Removing space around slash as mentioned by @Cottser
Update:
Ignore this patch. Accidentally committed wrong file, updating another patch

Removing space around slash as mentioned by @Cottser
Please ignore patch in #24
This patch contains interdiff from #20

Re-rolling patch for 8.2.x dev

Very straightforward, thanks for the latest patches. Both apply cleanly to their branches.

So, RTBC for the patches in #25 and #26.

Attaching interdiff from 20 to 26 for completeness.

Committed and pushed efad394 to 8.2.x and 381321e to 8.1.x. Thanks!