No table header on install requirements

Yes, having table headers here would sure be useful. Currently there's 3 columns. 1) Status Icon, 2) Requirement title, 3) Requirement description.

<table class="system-status-report"><tbody><tr class="ok"><td class="status-icon"><div title="OK"><span class="element-invisible">OK</span></div></td><td class="status-title">Web server</td><td class="status-value">Apache/2.2.17 (Ubuntu)</td></tr>
...
<tr class="error"><td class="status-icon"><div title="Error"><span class="element-invisible">Error</span></div></td><td class="status-title">Default settings file</td><td class="status-value">The default settings file does not exist.<div class="description">The Drupal installer requires that the <em>./sites/default/default.settings.php</em> file not be modified in any way from the original download.</div></td></tr>

So you'd like the order of these items to be restructured so that the outstanding items are more readily available.

Visually each row with an error message is red if it has an error. Furthermore there's an icon in the first column that is provided with an alternate text for the screen reader user. It's obvious to a sighted user (I think) what the problems are. I'm not sure how to best give an equivalent experience to a blind user.

Reporting the error is the first start. I'm changing the title so that it's clear what needs to happen.

I think there might be some UX issues in changing the order of the the messages. I also think that the new TH's will also be visible by default which will change the UI, so tagging accordingly.

Seems like this is just a bug. There is no table header, and there is no clear reason why we don't have them here.

I am not really excited about moving errors on top. People debugging, will refresh the page and fixate on the location where they previously saw the bug. If that location is variable, as errors move down and up from the top it will be hard to quickly scan this table if you resolved the bug.

Potentially we could use a status box for this, but for sighted users this would be a pain as it would just push the content they want down below the screen.

However, I suppose we could have an error that's available on focus to allow folks to jump right to the row with the error.

Ok, so here's a patch & a screen shot.

Could you expand on those suggestions, I don't really understand what they mean in actual changes to the UI.

@falcon03 Both :D. 1. Does that mean we change the position of the rows, depending on if its an error or not? 2. Like an actual drupal error message.

Don't we have a solution for this, in all tables?

#8: system-requirements-headers-1811128-8.patch queued for re-testing.

Hi All,

So, the patch works.

Unfortunately, it adds a visible header to the table... which would make this a UI change. So, we've added the "element-invisible" class to hide it from visibility, while keeping this accessible.

Sadly, this results in webkit rendering the table incorrectly. So, CSS has been modified in the "Seven" theme. This renders the border around the table in a slightly different way. In fact, "Seven" theme makes reference to a known issue with webkit rendering tables incorrectly:
/**
* Exception for webkit bug with the right border of the last cell
* in some tables, since it's webkit only, we can use :last-child
*/
tr td:last-child {
border-right: 1px solid #bebfb9; /* LTR */
}

...this patch removes the need for that.

Lol @sebsebseb123, I created a patch almost exactly the same at the same time. :P

Does it make sense to use Status, Component, and Details as the table header text? I feel like those describe the contents of the table a bit better than Status, Title, and Description. Thoughts?

That was weird... re-testing.

#16: system-requirements-headers-1811128-16.patch queued for re-testing.

Re-rolled with new column headers.

@falcon03 do you have access to a Windows box where you could test this for NVDA or something like that?

Would like to have some other folks look over this and see if we can figure out what the issue is. Hopefully it was just a glitch somewhere.

EDIT - I did try this in a fresh install and VO gave me the required headings without difficulty. Status, Components & Details were read to me no problem.

#22: statusreport_headers-1811128-22.patch queued for re-testing.

@falcon03 I'm marking this RTBC as it is at least a semantic improvement. With my testing with VoiceOver (both in November & today), I had no trouble seeing the headers. I've got a very basic use of VoiceOver, so perhaps I'm missing something.

Anyways, I'm attaching an image with the visuals that VoiceOver provides of the headings. When we can replicate the problem, I hope we can re-address it. Might just be a ghost in the machine though.

The code here is better semantically and works for me.

Looks sensible. Committed/pushed to 8.x.

excellent! Thanks @catch

Woot, my second Drupal core patch! Thanks everyone!

Removing the top border on the table element is too generic, needs to target only where needed. Where the patch does this:

-  border: 1px solid #bebfb9;
+  border-right: 1px solid #bebfb9;
+  border-bottom: 1px solid #bebfb9;

is on the element selector "table". Screenshot attached of an effected table (configure format filter) and it likely effected others.

Think someone fixed that already in the latest git:

table {
  width: 100%;
  font-size: 0.923em;
  margin: 0 0 10px;
  border-right: 1px solid #bebfb9;
  border-bottom: 1px solid #bebfb9;
}

#mgifford that is from the patch here from #22 that was just committed, that I am reporting about in #31, quoting the diff from the patch that *removed* the border-top, which I found to break at least the table in my screenshot.

Can you write a patch to fix it?

The only reason I just reported and did not write a patch, is I figured you guys who had worked on it were already familiar with the issue details, and I would have to spend time discovering what you already had been through, so I assumed it might be easier for the original author. It's not just put the border back, it's make it work for the original issue and not step on tables unseen from that screen.

I'm spinning up a Drupal 8 site now... I'll see if I can come up with a patch to fix that.

Yay, thanks mparker17. Just from looking at the previous patch, it seems just keep the original border on the generic table element and isolate all the border changes to table.system-status-report

Hmm... that CSS was written to get rid of a double-border at the top of the table. The table header row is invisible except to screen-readers, but there's a border on both the top of the table and the top of each regular row in the table: hence the double-border. See @sebsebseb123's comments in #16.

I'm too tired to come up with an effective solution right now, but I'll endevour to tackle this in the morning.

So, on my local D8 install, I reverted all the CSS changes in the patch from #22 and I get something that looks like the attached screenshot in Chromium 24 and Opera 12.13. It works fine in Firefox 19; haven't tested in IE 9 or 10 yet.

I don't know why the border shows up on the first column and not any other.

I tried a number of things... moving the element-invisible class to the thead element, moving it to the td elements, no change.

Deleting the table header entirely makes it work, but the whole point of this issue is to add a table header to make the table more accessible to users of screen readers, so that's not acceptable.

I could get the border to reappear around the whole table again by adding table.system-status-report { border-collapse: separate; }, but that removes the lines between rows in the table in all browsers.

It seems implausible that we've found a bug because it happens in two rendering engines (Webkit and Presto).

Any suggestions would be greatly appreciated!

#22: statusreport_headers-1811128-22.patch queued for re-testing.

Often with the installation is kept pretty light so that it doesn't depend on leveraging the rest of Drupal. I'm assuming that the theme_table() just hasn't been loaded in the installation.

Unfortunately the <tr> row is visible. That might need to be <tr class="element-invisible"> so that the row doesn't display.

I could get this to read out in VO, but if that works, then we can probably remove some of the others:
$output .= '<thead class="element-invisible"><tr>';

#47: statusreport-fix_tableheader-46.patch queued for re-testing.

#47: statusreport-fix_tableheader-46.patch queued for re-testing.

Related link in problems with VoiceOver:
http://www.accessibleculture.org/articles/2010/10/voiceover-and-tables-w...

fixing element-invisible in last patch.

We'll have to mark it as "needs review" in order for Testbot to notice it.

True, but that's waiting on #2002336-14: Introduce a CSS class to hide borders of fieldset elements

If you have a chance, please review that patch.

#52: statusreport-fix_tableheader-52.patch queued for re-testing.

In #2099761: Introduce a class to hide borders for tables I proposed adding a .table-unboxed class that emulated what was done in #2002336: Introduce a CSS class to hide borders of fieldset elements.

It's also worth noting the move to this twig file:
/core/modules/system/templates/status-report.html.twig

But this still needs testing to verify that it is an improvement for screen readers.

Patch needs re-roll.

This can be closed

My fault, the commits were not related to the patch.

@BarisW I took a look at this and the whole structure of the status-report.html.twig seems to have changed to:

    <tr class="system-status-report__entry system-status-report__entry--{{ requirement.severity_status }} color-{{ requirement.severity_status }}">
      {% if requirement.severity_status in ['warning', 'error'] %}
        <th class="system-status-report__status-title system-status-report__status-icon system-status-report__status-icon--{{ requirement.severity_status }}">
          <span class="visually-hidden">{{ requirement.severity_title }}</span>
      {% else %}
        <th class="system-status-report__status-title">
      {% endif %}
        {{ requirement.title }}
      </th>
      <td>
        {{ requirement.value }}
        {% if requirement.description %}
          <div class="description">{{ requirement.description }}</div>
        {% endif %}
      </td>
    </tr>

I suspect that this would override any heading information for that table related to the columns below.

This is an old screenshot, but helps focus on what we're trying to address in this long thread.

I had a look at this just now, and it appears that the structure has changed yet again, and no longer uses tables. @mgifford are the issues in this ticket still relevant given the current state of the template?

{% for group in grouped_requirements %}
  <h3 id="{{ group.type }}">{{ group.title }}</h3>
  {% for requirement in group.items %}
    <details>
      <summary role="button">
        {% if requirement.severity_title  %}
          <span class="visually-hidden">{{ requirement.severity_title }}</span>
        {% endif %}
        {{ requirement.title }}
      </summary>
      {{ requirement.value }}
      {% if requirement.description %}
        <div>{{ requirement.description }}</div>
      {% endif %}
    </details>
  {% endfor %}
{% endfor %}

Here is a screenshot:

Thanks @ultrabob - it is nice to be able to close this issue.

Removing the tables fixes the accessibility problem here.

Thanks for checking the latest code.