So at the moment there is no way in D7 to make the heading invisible through the Manage Display interface:
admin/structure/types/manage/blog/display

You only have the choice of: Above, Inline or

Now I'd prefer if just added an element-invisible class to the heading. However, to be consistent it might be more flexible to add a option so that you could remove the HTML as required.

Either way, core should have some way to provide a list of links with a contextual heading so that it is easier for navigation for screen readers.

Files: 
CommentFileSizeAuthor
#59 Screen Shot 2013-10-24 at 5.12.06 PM.png140.86 KBmgifford
#59 Screen Shot 2013-10-24 at 5.36.09 PM.png110.72 KBmgifford
#57 taxonomy_terms_need_headings-1106344-57.patch1.68 KBBarisW
PASSED: [[SimpleTest]]: [MySQL] 59,071 pass(es).
[ View ]
#49 taxonomy_terms_need_headings-1106344-49.patch1.66 KBmgifford
PASSED: [[SimpleTest]]: [MySQL] 58,961 pass(es).
[ View ]
#48 taxonomy_terms_need_headings-1106344-48.patch1.66 KBBarisW
PASSED: [[SimpleTest]]: [MySQL] 56,200 pass(es).
[ View ]
#43 taxonomy_terms_need_headings-1106344-43.patch900 bytesmgifford
PASSED: [[SimpleTest]]: [MySQL] 55,627 pass(es).
[ View ]
#40 interdiff-19-28.txt1.43 KBmgifford
#39 interdiff-19-28.txt1.43 KBmgifford
#28 taxonomy_terms_need_headings-1106344-28.patch900 bytesmgifford
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy_terms_need_headings-1106344-28.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#25 taxonomy_terms_need_headings-1106344-25.patch880 bytesmgifford
PASSED: [[SimpleTest]]: [MySQL] 31,447 pass(es).
[ View ]
#22 taxonomy_terms_need_headings-1106344-22.patch810 bytesmgifford
PASSED: [[SimpleTest]]: [MySQL] 29,403 pass(es).
[ View ]
#19 taxonomy_terms_need_headings-1106344-19.patch776 bytesmgifford
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy_terms_need_headings-1106344-19.patch.
[ View ]
#17 taxonomy_terms_need_headings-1106344-17.patch540 bytesmgifford
PASSED: [[SimpleTest]]: [MySQL] 29,389 pass(es).
[ View ]
#10 title_attributes_1106344.png82.06 KBJeff Burnz

Comments

yched’s picture

Status:Active» Postponed (maintainer needs more info)

"it might be more flexible to add a option so that you could remove the HTML as required"

I don't get this. That's what the 'hidden label' option currently does.

Besides, I don't think I see how "more flexible options for field display" qualifies as a bug rather than a feature request ?

Jeff Burnz’s picture

I think what Mike is saying is that Hidden removes the label, i.e. it does not print, and that perhaps we should have a label display option that merely hides the heading. So we'd have 4 options:

- Above
- Inline
- Hide
- Remove

In this scenario Hide merely adds the .element-invisible class, whereas Remove would act like Hidden does now.

yched’s picture

Title:Manage Display Removes List Heading» Add a 'label display' option that only adds 'element-invisible' class
Category:bug» feature
Status:Postponed (maintainer needs more info)» Active

OK, makes more sense. Retitling.

I do fear that the difference between 'hidden' and 'not there' will be confusing for most end-users, though.

Also, changed to 'feature request', until the point is made that this is a real accessibility bug.

mgifford’s picture

Title:Accessibility : missing header for taxonomy fields» Add a 'label display' option that only adds 'element-invisible' class
Component:markup» field system
Status:Needs review» Active

It's the same issue as #364219: Navigation menus should be preceded by headings of the appropriate level (usually <h2>).

or http://webaim.org/standards/wcag/checklist#g1.3

I was proposing keeping with the hidden/invisible framework that's used in CSS could be re-used for consistency.

So this is an example right now with the header hidden.

<div class="field field-name-taxonomy-vocabulary-3 field-type-taxonomy-term-reference field-label-hidden clearfix">
    <ul class="field-items">
          <li class="field-item even">
        <a href="/html5" typeof="skos:Concept" property="rdfs:label skos:prefLabel">HTML5</a>      </li>
          <li class="field-item odd">
        <a href="/topic/accessibility" typeof="skos:Concept" property="rdfs:label skos:prefLabel">Accessibility</a>      </li>
      </ul>
</div>

There should be an option like this:

<div class="field field-name-taxonomy-vocabulary-3 field-type-taxonomy-term-reference field-label-inline clearfix clearfix">
      <h3 class="element-invisible">Topic</h3>
    <ul class="field-items">
          <li class="field-item even">
        <a href="/html5" typeof="skos:Concept" property="rdfs:label skos:prefLabel">HTML5</a>      </li>
          <li class="field-item odd">
        <a href="/topic/accessibility" typeof="skos:Concept" property="rdfs:label skos:prefLabel">Accessibility</a>      </li>
      </ul>
</div>

There are no LABELs involved as such. It's labeling a list, but wanted to make that minor clarification.

I set this back as a bug as it's really no different than any other list of links in core. You have to be able to understand the context of the group of links.

Jeff Burnz’s picture

Category:feature» bug

OK, so we would have Above, Inline, Invisible and Hidden? As an end user I don't really know the difference between Invisible and Hidden, conceptually they are too close for me to differentiate what they might actually do.

If we take the route that less options is better and the developer always takes the hit (advocate for the low skilled user):

1. Most users won't care if the label is hidden or removed, having the label not display is the objective.
2. Its easier to remove the label in code than implement element-invisible logic.

Therefor I would think that the current "Hidden" option should add element-invisible class instead of removing the label. Advanced themers who care about structure/semantics can override the field output and remove the label.

So we do not add a new option - stick with Above, Inline and Hidden and merely make hidden add the class instead of removing the output - thoughts?

yched’s picture

If this is also about being able to pick the tag (h2, h3...) that wraps the label, then I'm sorry but this is definitely a feature request. Something like "semantic cck" in core.

About making "hidden" use element-invisible class : why not, but I guess there are also cases where you would want the label to be absent even for screen readers ?

Jeff Burnz’s picture

Not about picking heading level for labels.

I guess there are also cases where you would want the label to be absent even for screen readers ?

Yes good point, will let Mike and Everette chime in on that one. From theme perspective I see no issues with using element-invisible.

yched’s picture

I mean : currently field labels are displayed within a div, styled as strong through CSS. Letting the user choose a different tag (other than through theme override, of course) would be a feature request.

Jeff Burnz’s picture

There will be some performance overhead added here, since check_plain will always be called on the field label no matter what the display option.

Also there is a problem with labels we really don't want such as "body", even for accessibility that is going to be a pita to have in the source.

I ran into some weirdness with title_attributes in theme_field, I can get the new class into the $variables array but some some odd reason it won't print, not important now, need to discuss the first two things.

Jeff Burnz’s picture

StatusFileSize
new82.06 KB

Here is what I mean by title_attributes weirdness...

title_attributes_1106344.png

mgifford’s picture

So in it's simplest form it could just be:

function theme_field($variables) {
  $output = '';

  // Render the label, if it's not hidden.
  if (!$variables['label_hidden']) {
    $output .= '<div class="field-label"' . $variables['title_attributes'] . '>' . $variables['label'] . ':&nbsp;</div>';
  } else {
    $output .= '<div class="elementi-invisible"' . $variables['title_attributes'] . '>' . $variables['label'] . ':&nbsp;</div>';
  }

However there is no need to include a hidden label for anything other than a list. There's no need to have this setting do anything different than what it does for the body or title elements. Although I'm not sure how to select for $element['#field_type'] & pick out only those fields where the output requires a heading inorder to group the items together in context.

EDIT: Jeff, that's definitely an odd output. What code did you use to manipulate the array?

Jeff Burnz’s picture

Any field type or label display checking can to be pushed back to template_preprocess_field, the logic is quite simple to do there, but I think it comes down to special casing in core for one field type, cost/benefits etc, when this is quite easy to do (in your own theme) since we have template suggestions for field types.

mgifford’s picture

It's certainly something to think about. Perhaps pushing it back to the theme layer is more appropriate so that this code could be stripped out of a super lean site.

However, by by default we need to aim to have a site which is at least WCAG 2.0 A. We can be confident if it is something that has be be added in by individual themes and isn't in core (even just the core themes) that it isn't something that the vast majority of people will do.

yched’s picture

"However there is no need to include a hidden label for anything other than a list"

I'm not sure I get what this means exactly :
- "list" as in "ul / li" ? The default theme for fields only displays divs, so dealing with 'invisible label' or 'no label' would be up to the specific theme override that introduces 'ul / li's.
- "list" as in "any field having multiple values" ? It would be highly confusing to make the 'invisible vs absent label' behavior depend on the field being multiple. As a site builder / themer, I'd probably scratch my head for a long time before I get that the two are correlated.
Plus: what about a field that is 'multiple' (accepts several values), but happens to only have 1 value for a given node ?

mgifford’s picture

When I look at managing fields of a particular content type, say admin/structure/types/manage/blog/fields

The Term reference field is a UL list which needs a HEADING label for accessibility.

I don't think that the other field options (which include for me Boolean, Date, Datestamp, Datetime, Decimal, Email, File, Float, Image, Integer, Link, List, Longtext, Longtext & summary & Text) would produce LI's (either ordered or unordered) so would never need to have a H2, H3, H4 or similar heading label present for accessibility reasons.

Any UL or OL list (whether it has one item or 50) should be have a heading to provide context for screen readers. That could be invisible or not.

yched’s picture

It's bartik that overrides the field theme specifically for taxonomy fields so that they're displayed as ul/li.
bartik_field__taxonomy_term_reference()

As I said above, the default, theme-agnostic output for field values only involves divs.
theme_field().

So, should this be left to bartik to add the 'element-invisible' class when the label is configured as 'hidden' (instead of not printing it) ?

mgifford’s picture

Status:Active» Needs review
StatusFileSize
new540 bytes
PASSED: [[SimpleTest]]: [MySQL] 29,389 pass(es).
[ View ]

So then we should at the very least patch bartik, include the themable theme_field__taxonomy_term_reference() function into other core themes & include this as part of the process for themes for D8.

yched’s picture

Status:Needs review» Needs work

Typo : class="elementi-invisible" :-)

Besides, the comment a couple lines above (// Render the label, if it's not hidden.) should be updated accordingly.

mgifford’s picture

Status:Needs work» Needs review
StatusFileSize
new776 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy_terms_need_headings-1106344-19.patch.
[ View ]

Arg. There are times... vi.. Sorry..

Anyways, here's a new patch.

The comment also probably needs some help:
// Render the label either as a visible list or make it invisible for accessibility.

Then I'll work on patches for Seven & Stark. Or just Stark? Why would there be a Seven admin theme in D8?

yched’s picture

Title:Add a 'label display' option that only adds 'element-invisible' class » Accessibility : missing header for taxonomy fields
Component:field system» Bartik theme

Retitle, recategorize.

+ I've posted a comment on the issue that introduced the specific theme override in Bartik (#819996: Fix taxonomy term displays) to get the right people to comment.

Status:Needs review» Needs work

The last submitted patch, taxonomy_terms_need_headings-1106344-19.patch, failed testing.

mgifford’s picture

Status:Needs work» Needs review
StatusFileSize
new810 bytes
PASSED: [[SimpleTest]]: [MySQL] 29,403 pass(es).
[ View ]

Man... Hope this gets it. And thanks @yched for posting on the Fix taxonomy term display issue.

Jeff Burnz’s picture

Mike, is there a specific reason why we're switching markup here, from h3 to div?

mgifford’s picture

Nope. Sorry Jeff. I think that was a cut/paste error. Let me re-roll that one.

mgifford’s picture

StatusFileSize
new880 bytes
PASSED: [[SimpleTest]]: [MySQL] 31,447 pass(es).
[ View ]

Seemed to make more sense to consolidate it as:

  // Render the label either as a visible list or make it invisible for accessibility.
  $label_class = (!$variables['label_hidden']) ? 'field-label' : 'element-invisible';
  $output .= '<h3 class="' . $label_class . '">' . $variables['label'] . ': </h3>';

Since it's all the same other than the class.

mgifford’s picture

Everett Zufelt’s picture

Component:Bartik theme» markup
mgifford’s picture

StatusFileSize
new900 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy_terms_need_headings-1106344-28.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Updating patch.

mgifford’s picture

Title:Add a 'label display' option that only adds 'element-invisible' class » When displaying a term reference field headers aways renedered (invisible or not)
Component:field system» markup
Status:Active» Needs review
Issue tags:+Bartik

Not sure why Bartik has decided to deviate on this.

Jeff Burnz’s picture

There needs to be an easy way to completely remove headers if you do not want them. Removing point and click control over output is not what we need to be doing more of in Drupal.

This issue reminds me the breadcrumb heading which is hidden by default and requires a theme function override to remove element-invisible, which is just is a tad crazy. The same thing is going to happen here, if I read this issues correctly, that you will need to override a theme function to remove a heading.

mgifford’s picture

Status:Needs review» Needs work

The last submitted patch, taxonomy_terms_need_headings-1106344-28.patch, failed testing.

mgifford’s picture

Status:Needs work» Needs review
Issue tags:+accessibility, +Bartik
mgifford’s picture

Issue tags:+a11ySprint

@Jeff as usual you raise good points.

We don't set up WCAG's guidelines, but do need to try to work with them where we can. List elements need to have a header so that screen reader users can navigate between them.

Now, I'm not exactly sure how to deal with your bigger issue (which should be it's own issue). If you want to show the element it is a bit silly to have to write a theme function.

There's really now way to override it with CSS? Hmm.. Maybe we need something with Javascript. Not sure.

Adding headers to lists was a step ahead for some users, but apparently not for all.

If we get this in core then the D8 is more consistent. Then maybe someone can write a simple module that collects references to element-invisible & can remove them via some UI.

I'm not exactly sure how that would work, but want to make sure I understand.

mgifford’s picture

mgifford’s picture

Title:When displaying a term reference field headers aways renedered (invisible or not)» Taxonomy term reference field headers always should be rendered with a HTML header (invisible or not)

Changing the title for clarity (I hope).

mgifford’s picture

mgifford’s picture

mgifford’s picture

StatusFileSize
new1.43 KB

Going back to comment #19 for the interdiff I'm posting here to the latest in #28.

mgifford’s picture

StatusFileSize
new1.43 KB

Going back to comment #19 for the interdiff I'm posting here to the latest in #28.

mgifford’s picture

Status:Needs review» Needs work
Issue tags:+accessibility, +Bartik, +a11ySprint

The last submitted patch, taxonomy_terms_need_headings-1106344-28.patch, failed testing.

mgifford’s picture

Status:Needs work» Needs review
StatusFileSize
new900 bytes
PASSED: [[SimpleTest]]: [MySQL] 55,627 pass(es).
[ View ]

Reroll for bartik.theme filename change.

oresh’s picture

Status:Needs review» Reviewed & tested by the community

i just understand why not add field-label class anyway,
but works fine.
Tiny change anyway.

catch’s picture

Status:Reviewed & tested by the community» Needs work

If the label is hidden by field settings, it'll be NULL when it gets here:

http://api.drupal.org/api/drupal/modules!field!field.module/function/tem...

mgifford’s picture

@catch, sorry, but I don't understand why.

The patch in #43 just leverages $variables['label_hidden']

I can't see how it would affect the template_preprocess_field()'s:

  $variables['label_hidden'] = ($element['#label_display'] == 'hidden');
  $variables['label'] = $variables['label_hidden'] ? NULL : check_plain($element['#title']);

Is this a bigger problem with the label_hidden? What am I missing?

mgifford’s picture

tagging. Hopefully some clarity will help move this issue forward.

BarisW’s picture

Status:Needs work» Needs review
StatusFileSize
new1.66 KB
PASSED: [[SimpleTest]]: [MySQL] 56,200 pass(es).
[ View ]

I've updates template_preprocess_field() as well, and changed the logics in bartik_field__taxonomy_term_reference(). If the field is hidden, we just add the 'element-invisible' tag instead of replacing the 'field-label' class. Hidden fields now get the following output: <h3 class="field-label element-invisible">

mgifford’s picture

StatusFileSize
new1.66 KB
PASSED: [[SimpleTest]]: [MySQL] 58,961 pass(es).
[ View ]

Ok, so if I set this up so that the Label is Hidden (changed from Above) here:
admin/structure/types/manage/article/display

I should then be able to just add an article with a taxonomy. Then realized the patch needs to be updated to use "visually-hidden" which is now in Core.

Here's a re-roll.

mgifford’s picture

Status:Needs review» Needs work

The last submitted patch, taxonomy_terms_need_headings-1106344-49.patch, failed testing.

mgifford’s picture

Status:Needs work» Needs review
Issue tags:+Needs issue summary update, +accessibility, +Bartik, +a11ySprint
mgifford’s picture

Prior test failed with:

The test did not complete due to a fatal error. Completion check TrackerUserUidTest.php 33 Drupal\tracker\Tests\Views\TrackerUserUidTest->testUserUid()

Which seemed like it could be a bot issue, especially with this simple code.

mgifford’s picture

Status:Needs review» Needs work

The last submitted patch, taxonomy_terms_need_headings-1106344-49.patch, failed testing.

BarisW’s picture

Status:Needs work» Needs review
Issue tags:+Needs issue summary update, +accessibility, +Bartik, +a11ySprint
BarisW’s picture

StatusFileSize
new1.68 KB
PASSED: [[SimpleTest]]: [MySQL] 59,071 pass(es).
[ View ]

Another re-roll. HEAD moves fast nowadays.

mgifford’s picture

mgifford’s picture

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new110.72 KB
new140.86 KB

I tested this and it works as expected. When setting the Label as hidden

Before patch
Before patch

After patch:
Invisible header

webchick’s picture

Issue summary:View changes
Status:Reviewed & tested by the community» Fixed

Committed and pushed to 8.x. Thanks!

Status:Fixed» Closed (fixed)

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