Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text content for keyboard shortcuts dialog #100

Closed
terracoda opened this issue Nov 15, 2017 · 26 comments
Closed

Text content for keyboard shortcuts dialog #100

terracoda opened this issue Nov 15, 2017 · 26 comments
Assignees

Comments

@terracoda
Copy link

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.

@terracoda terracoda self-assigned this Nov 15, 2017
@terracoda
Copy link
Author

terracoda commented Nov 15, 2017

Draft 1
Represented by a single list like it is in the visual design.

  • Adjust slider with Left and Right arrow keys or Up and Down arrow keys.
  • Adjust in smaller steps by holding Shift plus Left or Right arrow key, or by holding Shift plus Up and Down arrow key.
  • Adjust in larger steps with Page Up and Page Down keys.
  • Jump to minimum with Home key.
  • Jump to maximum with End key.

Questions:

  1. Wondering if a sublist would be helpful for users:
  2. Any tweaks to wording?

Draft 2

  • Adjust slider:
    • Left and Right arrow keys, or
    • Up and Down arrow keys.
  • Adjust in smaller steps:
    • Hold Shift plus Left or Right arrow key, or
    • Hold Shift plus Up or Down arrow key.
  • Adjust in larger steps with Page Up or Page Down key.
  • Jump to minimum with Home key.
  • Jump to maximum with End key.

@terracoda
Copy link
Author

@amanda-phet , @arouinfar , @oliver-phet , @emily-phet
Comment when you have time.

For reference here's the visual design:
screen shot 2017-11-15 at 11 33 20

@arouinfar
Copy link

arouinfar commented Nov 17, 2017

@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 arouinfar removed their assignment Nov 17, 2017
@terracoda
Copy link
Author

@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.

@arouinfar
Copy link

@terracoda thanks for the clarification! Draft 2 seems convenient, so I think it's definitely worth some user testing.

@terracoda
Copy link
Author

If all goes as planned in phetsims/a11y-research#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 #100 (comment)

@terracoda to provide html snippets in the a11y-research repo

@jessegreenberg
Copy link
Contributor

@terracoda what is the status of this issue, are the accessible descriptions in this dialog ready for implementation?

@terracoda
Copy link
Author

@jessegreenberg, yes :-) I think so. It's been a long discussion.

Based on @arouinfar's comment (#100 (comment)) 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 phetsims/balloons-and-static-electricity#320 (comment)

@jessegreenberg
Copy link
Contributor

@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?

@jessegreenberg
Copy link
Contributor

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>

@terracoda
Copy link
Author

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>

@terracoda
Copy link
Author

@jessegreenberg, I also posted more code examples in phetsims/scenery#686 (comment) in case that is helpful for creating a robust API.

@jessegreenberg
Copy link
Contributor

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>

?

@terracoda
Copy link
Author

terracoda commented Jan 31, 2018

@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.

@jessegreenberg
Copy link
Contributor

Both h3 and p are fine with me. My question in #100 (comment) was about the difference between span and no tag name (as was in the original mock up), not span and h3.

@terracoda
Copy link
Author

@jessegreenberg, honestly, I do not think there is any difference.

@jessegreenberg
Copy link
Contributor

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?

@terracoda
Copy link
Author

@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.

@terracoda terracoda reopened this Feb 1, 2018
@terracoda
Copy link
Author

terracoda commented Feb 1, 2018

@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.

@terracoda terracoda assigned emily-phet and arouinfar and unassigned arouinfar Feb 1, 2018
@terracoda
Copy link
Author

@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.

@emily-phet
Copy link

Looks good to me!

@terracoda
Copy link
Author

Thanks @emily-phet.

@arouinfar, any comment? Ready for implementation?

@arouinfar
Copy link

Looks good to me too @terracoda! I'd say it's ready for implementation.

@terracoda
Copy link
Author

@arouinfar, thanks!

@terracoda terracoda removed their assignment Feb 2, 2018
@terracoda
Copy link
Author

@jessegreenberg, Ohm's Law Keyboard Dialog update is ready implementation.

@jessegreenberg
Copy link
Contributor

Alright, change made in the above commit @terracoda! Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants