mdn / yari

The platform code behind MDN Web Docs
Mozilla Public License 2.0
1.19k stars 509 forks source link

Rethink using <h4> in notecards #4394

Closed tannerdolby closed 3 years ago

tannerdolby commented 3 years ago

I tested quite a few pages in my work for #4060, but I didn't stumble across pages that had a .note notecard container with a nested <h4>. Most of my Lighthouse audit tests were on pages with notes using a <p><strong>Note:</strong> foo bar</p> structure, any page with notecards that still use an <h4> will be causing a11y issues by skipping heading levels and creating a non sequentially-descending heading order.

Page with <h4> in notecard:

Page with <p> and <strong> (no h4):

Screen Shot 2021-08-02 at 11 08 13 PM
tannerdolby commented 3 years ago

We need to make the notecards more consistent. Currently they are be utilized in two ways:

Without an h4 and just using <p><strong></strong></p>

<div class="notecard note">
  <p><strong>Note:</strong> Class <strong>expressions</strong> are subject to the same hoisting restrictions as described in the <a href="#class_declarations">Class declarations</a> section.</p>
</div>

and with an <h4 id="note> which is causing a11y issues by breaking the heading order

<div class="notecard note">
  <h4 id="note">Note</h4>
  <p>Until 2019, the <a href="/en-US/docs/Glossary/W3C">W3C</a> published a competing HTML5 standard with version numbers. In MDN docs and elsewhere on the web therefore, you may find references to other HTML versions, HTML5.1 for example. Since <a href="https://www.w3.org/blog/news/archives/7753" class="external" rel=" noopener">28 May 2019</a>, the WHATWG Living Standard was announced by the W3C as the sole version of HTML.</p>
</div>

I reckon we just swap the h4 out for a <strong id="note"> (to utilize the same styles/look) where its still being used and just keep the other notes as is which already use <p><strong>Note:</strong>foo bar</p>. Therefore, eliminating the non sequentially-descending heading order a11y issue and also aiding in the work done in splitting sections by subheadings (#4388).

If we need to bump the font-weight/size etc by using <strong id="note"> (in notecards where the h4 tags are being used) instead of h4 to mimic the same look, that won't be a problem.

cc @peterbe @schalkneethling

schalkneethling commented 3 years ago

@tannerdolby I believe we are doing away with heading elements in the new notes.

peterbe commented 3 years ago

First of all, @wbamberg has confirmed that div.notecard h4 ceases to be a thing when the source is a Markdown.

In the en-US content alone, there are roughly 600 div.notecard h4 selectors:

▶ rg '<h4>' | wc -l
     626

With a lot of elbow grease, we could possibly clean that up over a couple of weeks. But who knows what madness will still exist in the translated-content. Another option is to use cheerio to force all div.notecard h4 into a div.notecard p strong. Wouldn't be horribly hard, and if done, it should free up https://github.com/mdn/yari/pull/4388 to treat all <h4> tags as individual headings.

One thing to watch out for is that it might hard to use cheerio to turn:

<div class="notecard note">
  <h4>TEXT HERE</h4>
  <p>MORE TEXT HERE</p>
</div>

into...:

<div class="notecard note">
  <p><strong>TEXT HERE</strong> MORE TEXT HERE</p>
</div>

because then you're not just messing with the tags, you're messing with the content too.

wbamberg commented 3 years ago

So back in the day there weren't very strong guidelines around the markup that should appear in note or warning cards, but generally people used <strong> at the start. Then in January 2021 the recommendation was changed to use headings: https://developer.mozilla.org/en-US/docs/MDN/Contribute/Changelog#january_2021.

Personally I disagree with this change because it breaks the idea that headings implicitly section a page. But that's by the by.

In Markdown we redid the representation of notes and warnings: https://developer.mozilla.org/en-US/docs/MDN/Contribute/Markdown_in_MDN#notes_warnings_and_callouts - they are an GFM blockquote whose first paragraph starts with **Note:** or **Warning:** or **Callout:**.

In the meantime we have to tell the h2m converter how to recognize notes and warnings, given the rather wide range of ways people currently mark them up. Since the most popular option in the JS docs was still <p><strong>Note:</strong></p> (https://github.com/mdn/content/pull/4504#issuecomment-827258409), I updated the notes and warnings to follow that pattern consistently (https://github.com/mdn/content/pull/4504), and that's what h2m recognizes.

So I'm currently manually removing <h4> elements inside notes in en-US content (as well as removing lots of other inconsistencies in the way people write notes). At some point it might be worth automating this. This is especially true if other areas of the site, like Web/API, use the <h4> style more than the JS docs do (and I think this is probably the case).

So given this:

<div class="note notecard">
<h4>Note:</h4>
<p>stuff...

...then it's probably pretty safe to auto-convert it to:

<div class="note notecard">
<p><strong>Note:</strong> stuff...

...at least given a manual review of the changes.