Closed terracoda closed 6 years ago
Draft 1 Represented by a single list like it is in the visual design.
Questions:
Draft 2
@amanda-phet , @arouinfar , @oliver-phet , @emily-phet Comment when you have time.
For reference here's the visual design:
@terracoda wording looks good, but I could use some clarification about the functional difference between the drafts. It looks like screen reader users would be able navigate through draft 2 a bit faster if they decide to skip over the sublist items. Is that the case? If so, draft 2 may be a bit more efficient to navigate, particularly because screen reader users would likely be familiar with many of the keyboard shortcuts for standard components.
@arouinfar, yes with Draft 2 screen reader users would get a clear indication that there are 2 ways to adjust, and two ways to adjust in smaller steps based on the sub-listed structure. I'm not exactly sure how screen reader users navigate nested lists, however. My intuition is that there would be a benefit, but with such a small amount of content, the benefit may be small. It would be nice to ask users about this.
@terracoda thanks for the clarification! Draft 2 seems convenient, so I think it's definitely worth some user testing.
If all goes as planned in https://github.com/phetsims/a11y-research/issues/62 (i.e., we keep the action text on left and key icons on right) then the descriptive content for the non visual experience would be as in https://github.com/phetsims/ohms-law/issues/100#issuecomment-344618056
@terracoda to provide html snippets in the a11y-research repo
@terracoda what is the status of this issue, are the accessible descriptions in this dialog ready for implementation?
@jessegreenberg, yes :-) I think so. It's been a long discussion.
Based on @arouinfar's comment (https://github.com/phetsims/ohms-law/issues/100#issuecomment-345576042) and the resolution in phetsims/a11y-research#62, I think we decided to go with descriptive action text first and description for key icons second as in Draft 2 content and structure.
Here's the HTML:
<h1>Keyboard Shortcuts</h1>
<h2>Slider Controls</h2>
<ul>
<li>Adjust slider:
<ul>
<li>Left and Right arrow keys, or</li>
<li>Up and Down arrow keys.</li>
</ul>
</li>
<li>Adjust in smaller steps:
<ul>
<li>Hold Shift plus Left or Right arrow key, or</li>
<li>Hold Shift plus Up or Down arrow key.</li>
</ul>
</li>
<li>Adjust in larger steps with Page Up or Page Down key.</li>
<li>Jump to minimum with Home key.</li>
<li>Jump to maximum with End key.</li>
</ul>
<h2>General Navigation</h2>
<ul>
<li>Move to next item with Tab key.</li>
<li>Move to previous item with Shift plus Tab key.</li>
<li>Exit a dialog with Escape key.</li>
</ul>
For the General Navigation, I checked consistency with https://github.com/phetsims/balloons-and-static-electricity/issues/320#issuecomment-346495639
@terracoda I have just about wrapped this up, but there is one problem with the above HTML. Scenery currently doesn't support this structure.
<li>Adjust slider:
<ul>
Scenery currently doesn't support mixing Text
and Element
child nodes. Would it be acceptable to wrap "Adjust slider:" with a span
?
Done in the above commit, here is the HTML now:
<h1 tabindex="-1" id="248-842-847-846-844">Keyboard Shortcuts</h1>
<h2 tabindex="-1" id="248-842-847-846-245-244-175-171">Slider Controls</h2>
<ul tabindex="-1" id="248-842-847-846-245-244-175-174-173">
<li tabindex="-1" id="container-248-842-847-846-245-244-175-174-173-42"><span tabindex="-1" id="label-248-842-847-846-245-244-175-174-173-42">Adjust slider</span>
<ul tabindex="-1" id="248-842-847-846-245-244-175-174-173-42">
<li tabindex="-1" id="248-842-847-846-245-244-175-174-173-42-40-23">Left and Right arrow keys, or</li>
<li tabindex="-1" id="248-842-847-846-245-244-175-174-173-42-40-38">Up and Down arrow keys.</li>
</ul>
</li>
<li tabindex="-1" id="container-248-842-847-846-245-244-175-174-173-107-106"><span tabindex="-1" id="label-248-842-847-846-245-244-175-174-173-107-106">Adjust in smaller steps</span>
<ul tabindex="-1" id="248-842-847-846-245-244-175-174-173-107-106">
<li tabindex="-1" id="248-842-847-846-245-244-175-174-173-107-106-105-89-62">Hold Shift plus Up or Down arrow key.</li>
<li tabindex="-1" id="248-842-847-846-245-244-175-174-173-107-106-101-77">Hold Shift plus Left or Right arrow key, or</li>
</ul>
</li>
<li tabindex="-1" id="248-842-847-846-245-244-175-174-173-136">Adjust in larger steps with Page Up or Page Down key.</li>
<li tabindex="-1" id="248-842-847-846-245-244-175-174-173-153">Jump to minimum with Home key.</li>
<li tabindex="-1" id="248-842-847-846-245-244-175-174-173-170">Jump to maximum with End key.</li>
</ul>
<h2 tabindex="-1" id="248-842-847-846-245-244-243-239">General Navigation</h2>
<ul tabindex="-1" id="248-842-847-846-245-244-243-242-241">
<li tabindex="-1" id="248-842-847-846-245-244-243-242-241-192">Move to next item with Tab key.</li>
<li tabindex="-1" id="248-842-847-846-245-244-243-242-241-221">Move to previous item with Shift plus Tab key.</li>
<li tabindex="-1" id="248-842-847-846-245-244-243-242-241-238">Exit a dialog with Escape key.</li>
</ul>
Great idea @jessegreenberg to add an HTML tag in there. I was going to suggest a similar approach, but use a heading <h3>
or at least a <p>
instead of a <span>
. A <span>
provides no meaningful or functional benefit whereas headings and paragraphs do.
<h1>Keyboard Shortcuts</h1>
<h2>Slider Controls</h2>
<ul>
<li><h3>Adjust slider</h3>
<ul>
<li>Left and Right arrow keys, or</li>
<li>Up and Down arrow keys.</li>
</ul>
</li>
<li><h3>Adjust in smaller steps</h3>
<ul>
<li>Hold Shift plus Left or Right arrow key, or</li>
<li>Hold Shift plus Up or Down arrow key.</li>
</ul>
</li>
<li>Adjust in larger steps with Page Up or Page Down key.</li>
<li>Jump to minimum with Home key.</li>
<li>Jump to maximum with End key.</li>
</ul>
<h2>General Navigation</h2>
<ul>
<li>Move to next item with Tab key.</li>
<li>Move to previous item with Shift plus Tab key.</li>
<li>Exit a dialog with Escape key.</li>
</ul>
@jessegreenberg, I also posted more code examples in https://github.com/phetsims/scenery/issues/686#issuecomment-361933984 in case that is helpful for creating a robust API.
Thanks @terracoda. I can add an h3 in there, but that adds more information than your original mockup. I thought you didn't want extra information around that text. Is there any difference between
<li><span>Adjust slider</span>...</li>
and
<li>Adjust slider...</li>
?
@jessegreenberg, yes a <span>
is completely meaningless, and yes an <h3>
provides heading semantics.
How do you feel about a <p>
instead of a <span>
? In general, I try to avoid using<span>
's as they provide no meaning at all.
Both h3
and p
are fine with me. My question in https://github.com/phetsims/ohms-law/issues/100#issuecomment-362007511 was about the difference between span and no tag name (as was in the original mock up), not span
and h3
.
@jessegreenberg, honestly, I do not think there is any difference.
Thanks @terracoda, good to know, wasn't sure if it changed anything with the virtual cursor. In the above commit I changed the span
to p
. @terracoda is there anything remaining for this issue?
@jessegreenberg, it might affect the virtual cursor, and it might be browser dependent. Both <li>
and <p>
are block-level elements whereas is in-line. It is always fine to use in-line elements inside block-level, but not the other way around.
I think this is issue is done. Thanks @jessegreenberg.
@jessegreenberg, @emily-phet, and @arouinfar, I just tested Dev 20.
I find the nested structure in the Keyboard Shorts dialog to be a bit too structure heavy, getting in the way of the actual information.
I suggest removing the nested list structure and going with this:
<h1>Keyboard Shortcuts</h1>
<h2>Slider Controls</h2>
<ul>
<li>Adjust slider with Left and Right arrow keys, or Up and Down arrow keys.</li>
<li>Adjust in smaller steps with Shift plus Left or Right arrow key, or Shift plus Up or Down arrow key.</li>
<li>Adjust in larger steps with Page Up or Page Down key.</li>
<li>Jump to minimum with Home key.</li>
<li>Jump to maximum with End key.</li>
</ul>
<h2>General Navigation</h2>
<ul>
<li>Move to next item with Tab key.</li>
<li>Move to previous item with Shift plus Tab key.</li>
<li>Exit a dialog with Escape key.</li>
</ul>
I mean (edited )only updated the content in the first and second list items - the nested ones.
@emily-phet and @arouinfar, I am comfortable removing the nested list structure, now that I heard what it sounds like in context.
Can you please comment wording change? Thanks.
Looks good to me!
Thanks @emily-phet.
@arouinfar, any comment? Ready for implementation?
Looks good to me too @terracoda! I'd say it's ready for implementation.
@arouinfar, thanks!
@jessegreenberg, Ohm's Law Keyboard Dialog update is ready implementation.
Alright, change made in the above commit @terracoda! Closing.
Just noticed that this content may not have been provided along side the visual design.
@terracoda to spell it out in this issue very soon.