Skip to content

Conversation

@simurai
Copy link
Contributor

@simurai simurai commented Feb 19, 2020

This is a first attempt to make our .form-group component more accessible.

Problem

In our docs we suggest using a "definition list" (<dl>) for the .form-group component. We even "bake it in" and make it a requirement by using selectors like dl.form-group > dd. The problem is that it can be "too noisy" for screen readers when there is a <dt> with just one <dd>.

From https://github.com/github/github/issues/132195 by @jscholes
When a screen reader user encounters repeated list containers with only a single item, it significantly increases the speech verbosity of page navigation. This is because the software will announce the fact that the cursor is entering and leaving each list.

Fix

This PR replaces...

  • <dl class="form-group"> 👉 <div class="form-group">
  • <dt> 👉 <div class="form-group-header">
  • <dd> 👉 <div class="form-group-body">

in the docs.

Before

<dl class="form-group">
  <dt>
    <label for="example-text">Example Text</label>
  </dt>
  <dd>
    <input class="form-control" type="text" value="Example Value" id="example-text" />
  </dd>
</div>

After

<div class="form-group">
  <div class="form-group-header">
    <label for="example-text">Example Text</label>
  </div>
  <div class="form-group-body">
    <input class="form-control" type="text" value="Example Value" id="example-text" />
  </div>
</div>

For now, the CSS selectors e.g dl.form-group > dd are unchanged. We'll have to wait to deprecate them until the next major release. Also, first we'd have to replace all the markup everywhere on dotcom.

Alternative

We might not even need those "wrapper" elements and could reduce the markup down to:

<div class="form-group">
  <label  class="form-group-label" for="example-text">Example Text</label>
  <input class="form-group-input form-control" type="text" value="Example Value" id="example-text" />
</div>

It would probably require some more testing and maybe refactoring on dotcom? 🤔

/cc @primer/ds-core

@vercel
Copy link

vercel bot commented Feb 19, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/primer/primer-css/q3v2t0610
✅ Preview: https://primer-css-git-accessible-form-groups.primer.now.sh

@simurai simurai changed the base branch from master to release-14.3.0 March 24, 2020 01:53
Co-Authored-By: Mu-An 慕安 <me@muanchiou.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants