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

Fix aria-role description in grabDrag interaction #152

Closed
zepumph opened this issue Dec 4, 2018 · 14 comments
Closed

Fix aria-role description in grabDrag interaction #152

zepumph opened this issue Dec 4, 2018 · 14 comments

Comments

@zepumph
Copy link
Member

zepumph commented Dec 4, 2018

No description provided.

@zepumph zepumph self-assigned this Dec 4, 2018
@terracoda
Copy link
Contributor

@zepumph, I started a little example walk-through for the aria-roledescription in the Grab Button Alerts section and will be adding some ideas there.

This is just in case we need it. We may not.

@zepumph
Copy link
Member Author

zepumph commented Dec 11, 2018

I added this above. Please have a look.

@terracoda
Copy link
Contributor

@zepumph, I am hearing the aria-roledescription text again, but I am hearing things in a strange way.

I hear the following:

  1. On focus: "Chemistry book, web application"
  2. On grab: "Chemistry book, move in four directions", Alert: "Grabbed. Lightly on Physics book ..."
  3. On release: "Chemistry book, web application", Alert: "Released."

Number 2. Is what I expect to hear with aria-roledescription in place.

I thought I should hear "Grab Chemistry book, button" on focus, and possibly on release?

What I think might be good is if it goes like this:

  1. On focus: "Chemistry book, button" (no aria-roledescription needed)
  2. On grab: "Chemistry book, move in four directions", Alert: "Grabbed. Lightly on Physics book ..."
  3. On release:
    a. "Chemistry book, re-grab to move", Alert: "Released."
    b. And since focus should end up on the button, one would likely hear, "Grab Chemistry book, button" again.

It seems focus is not being placed on the button on release, because after I release the Chemistry book, and if I press the Tab key, my focus goes to the same object again (e.g. the Chemistry book that I just released).

I like the idea of not hearing the refocused button's label after a release, but I think that's how it is actually supposed to work.

Questions for you?

  • Where is "Grab Chemistry book, button"?
  • Where are you placing focus after a release?
  • Is possible to use a different aria-roledescription upon release, e.g., "Chemistry book, re-grab to move"?

@terracoda
Copy link
Contributor

@zepumph, I will complete the walk-through in the design document and discuss with @emily-phet today.

@terracoda
Copy link
Contributor

@zepumph, I discussed the grab/release walk-through in the design document with @emily-phet.

Is it possible to have difference aria-roledescription text on grab and release? Here's what we think will work nicely if this is possible:

  1. On grab: Chemistry book, move in four directions
  2. On release: Chemistry book, grab to move

And on release, please return focus to the Grab Chemistry book, button.

@zepumph, let me know if this is possible and/or if you have any questions.

@zepumph
Copy link
Member Author

zepumph commented Dec 13, 2018

I'm glad you had a listen! I did quite a few bug fixes in relation to this topic. Here is what I fixed, please take a look:

  1. Supported proper aria-label updating so you should hear the "Grab *" now when you release.
  2. Added the aria-roledescription you asked for for the button.
  3. A few more behind the scenes things to make sure that the above two work.

A note about your above comment:
On grab: Chemistry book, move in four directions
On release: Chemistry book, grab to move
The accessible name of the button is "Grab Chemistry book", so you will probably hear "Grab Chemistry book, grab to move." I hear on NVDA "button, Grab Chemistry book" and it sounds nice to me!

Let me know what else you want for this. Perhaps we should align some time to talk so that we can get on the same page if my commits are in the wrong direction.

@terracoda
Copy link
Contributor

terracoda commented Dec 16, 2018

@zepumph, I had a listen and it sounds good when interacting, but possibly problematic within VoiceOver's help text, as aria-roledescriptin replaces the name/label all together.

I think for AT that supports "aria-roledescription", the text within the attribute becomes the name and function for the object (I'll check this). Thus, in VoiceOver's help text hint system, the object is referred to like this,

You are currently on a grab to move..."
You are currently on a move in four directions..."

I should have made this more clear in the design document, but I think that the name/label of the object needs to be included in the aria-roledescription as well. The examples online do include name and function in the aria-roledescription. So, in our case, it would be somewhat more like this:

<button>Grab Chemistry book</button>
<div role="application" aria-roledescription="Chemistry book, move in four directions">Chemistry book</div>

And then on release

<button>Grab Chemistry book</button>
<div role="application" aria-roledescription="Chemistry book, grab to move">Chemistry book</div>

Or maybe it would work on release like this:

<button>Grab Chemistry book</button>
<div role="application" aria-roledescription="Chemistry book">Chemistry book</div>

I am having trouble finding the exact HTML with the inspector. Is it possible to paste in the HTML for me for these three states?

  1. On focus of the button
  2. On grab - application is launched
  3. On release - application is closed and focus is returned to the button

I could be wrong about this, but I think we might be having a little trouble because we are dealing with two elements, a normal button and a div role="application" all within the same object.

Edited: removed "not" in fist sentence, and added "function"

@terracoda
Copy link
Contributor

@zepumph, maybe the word, "moveable" is better than all this business of "move in in four directions" and "grab to move". I'll work on this in the design doc and assign aback to you when it is ready.

@terracoda
Copy link
Contributor

@zepumph, I think I have a couple of options for us:

Current code for Grab Chemistry book button:

<button data-focusable="true" 
id="332-415-678-437" 
data-trail-id="332-415-678-437" aria-label="Grab Chemistry book" 
aria-roledescription="grab to move">Grab Chemistry book</button>

Current code for grabbed Chemistry book div:

<div tabindex="0" data-focusable="true" 
id="332-415-678-437" data-trail-id="332-415-678-437" 
aria-label="‪Chemistry‬ book" 
aria-roledescription="move in four directions" 
role="application">‪Chemistry‬ book</div>

Proposed code options for Grab Chemistry book button:

  1. Option without aria-label & without aria-roledescription:
<button data-focusable="true"
 id="332-415-678-437" 
data-trail-id="332-415-678-437">Grab Chemistry book</button>
  1. Option with aria-roledescription, if the attribute is required on the button:
<button data-focusable="true" 
id="332-415-678-437" 
data-trail-id="332-415-678-437" 
aria-roledescription="Grab Chemistry book button">Grab Chemistry book</button>

Note: If an aria-label is required in option 2, use aria-label="Grab Chemistry book", but I don't think an aria-label should be needed with a native HTML button. However, the use of the aria-roledescription attribute might mean an aria-label is needed.

Proposed code options for grabbed Chemistry book div:

  1. Option with new generalized aria-roledescription text
<div tabindex="0" 
data-focusable="true" 
id="332-415-678-437" 
data-trail-id="332-415-678-437" 
aria-label="‪Chemistry‬ book" 
aria-roledescription="Moveable Chemistry book" 
role="application">‪Chemistry‬ book</div>
  1. We could even try "Moveable Chemistry book" all around, and see what kind of duplication we get with the different AT (if any duplication).
<div tabindex="0" 
data-focusable="true" 
id="332-415-678-437" 
data-trail-id="332-415-678-437" 
aria-label="Moveable Chemistry book" 
aria-roledescription="Moveable Chemistry book" 
role="application">‪Moveable Chemistry book</div>

Summary & Walk-through from Design Doc
With this solution, the aria-roledescription="Moveable Chemistry book" stays the same on grab and on release! And I find that the grab alert text, the release alert text, and grab button label, all seem to go along nicely with "Moveable Chemistry book":

  • User Tabs to grab button
    • Grab Chemistry book, button
  • User presses Space key
    • Moveable Chemistry book
    • without support: “Chemistry book, web application”
  • Grab alert fires:
    • Grabbed. Lightly on Physics book. Use W, A, S, or D keys to move book. Space to release.
  • User presses S or Down Arrow
    • Down. Rub fast or slow.
  • User rubs back and forth with A and D or Left and Right Arrow keys
    • Left, Right, Jiggling more warmer.
    • Jiggling faster, now hotter.
    • Jiggling even faster, even hotter.
    • Very hot. Atoms breakaway from Chemistry book.
  • User presses Space key to release:
    • Moveable Chemistry book
    • without support, “Chemistry book, web application”
  • Release alert fires:
    • Released.
  • Somewhere during the release, focus returns to Grab button:
    • Grab Chemistry book, button
  • During the release and refocus, Cooling alerts continue :
    • Jiggling less, cooler.
    • Jiggling even less, even cooler.
    • Temperature cool. Atoms jiggle a tiny bit.

@terracoda
Copy link
Contributor

terracoda commented Dec 17, 2018

@zepumph, I successfully fished out the code from the inspector and have provided the two options above. Assigning back to you. Please let me know if you have any questions. I think the changes, especially option 2 for the button, are straight forward.

Note that options are kind of interchangeable. Ideally, we want the simplest code with the fewest number of attributes that sounds still sounds good in all AT.

@terracoda
Copy link
Contributor

And @zepumph, thanks for all your work in #152 (comment)

I think we are very close to closing this issue!

@zepumph
Copy link
Member Author

zepumph commented Dec 18, 2018

For button, we will get rid of aria-label and aria-roledescription:

<button data-focusable="true"
 id="332-415-678-437" 
data-trail-id="332-415-678-437">Grab Chemistry book</button>

and the second option for the draggable

<div tabindex="0" 
data-focusable="true" 
id="332-415-678-437" 
data-trail-id="332-415-678-437" 
aria-label="Moveable Chemistry book" 
aria-roledescription="Moveable Chemistry book" 
role="application">‪Moveable Chemistry book</div>

zepumph added a commit to phetsims/scenery that referenced this issue Dec 18, 2018
zepumph added a commit that referenced this issue Dec 18, 2018
… strings, A11yGrabDragNode should set accessibleName for draggable and grabbable, #152
zepumph added a commit to phetsims/scenery-phet that referenced this issue Dec 18, 2018
… strings, A11yGrabDragNode should set accessibleName for draggable and grabbable, phetsims/friction#152
zepumph added a commit to phetsims/scenery-phet that referenced this issue Dec 18, 2018
@zepumph
Copy link
Member Author

zepumph commented Dec 18, 2018

The above PDOM has been implemented. In general I really like all of the changes above. I moved some code from usages into the grab/drag js file. @terracoda let me know what you think!

@terracoda
Copy link
Contributor

@zepumph, this is working great in VoiceOver!
I do not hear the aria-roledescription on release which is what I expected. I think not hearing it on release is a good thing. On release, as focus is placed back on the button, I hear the button label, the release alert and then any remaining cooling alerts. This sounds very good to me.

On grab, I like the way the aria-roledescription text, "Moveable Chemistry book" sounds with the alert, "Grabbed. Lightly on Physics book..." and on release I think the button label, "Grab Chemistry book, button" sounds fine with the release alert, "Release."

VoiceOver Hint Text describes the grab button as a button (no name, but that's fine) and the grabbed objects with the appropriate aria-roledescription, e.g., "You are currently on a Moveable Chemistry book".

Here's a couple screen shots to document VoiceOver's custom Hint Text:
vo-hint-initial-focus-zoomed-in-book
vo-hint-grabbed-zoomed-in-book

Thanks for all your work on a generalizable grab button!
Closing this issue with delight!

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

No branches or pull requests

2 participants