-
Notifications
You must be signed in to change notification settings - Fork 12
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
Should a11yCreateAriaValueText be required to prevent description percents on iOS? #682
Comments
Instead of requiring this maybe we could try a different default function for a11yCreateAriaValueText. Rather than just toString we could try adding an extra 'hairspace' like we did to support _a11yRepeatEqualValueText? However, if Apple has decided that the best output for sliders is to report the percentage, would it be acceptable for PhET to allow it unless it is pedagogically problematic? Or we could submit a feature request to Apple to remove it and accept it as the platform behavior for now. |
To me the subtext of this issue is that PhET is using the wrong HTML input. Ideally it would be number instead of range, since range conveys percentage semantics, at least in many cases. NumberPicker is not meant to be used in a percentage case, so I don't think that a default of allowing percentages makes too much sense. In a perfect world, we would probably be using type I tried converting the default to add a hairspace in this patch. It still gave percents in the my challenge number pickers in RAP. Index: sun/js/accessibility/AccessibleValueHandler.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/accessibility/AccessibleValueHandler.js b/sun/js/accessibility/AccessibleValueHandler.js
--- a/sun/js/accessibility/AccessibleValueHandler.js (revision faf9b4055168c78b927b2a171a8fbe33464f8f05)
+++ b/sun/js/accessibility/AccessibleValueHandler.js (date 1612208446158)
@@ -31,7 +31,8 @@
// constants
const DEFAULT_TAG_NAME = 'input';
-const toString = v => v + '';
+const hairSpace = '\u200A';
+const addHairSpace = v => v + hairSpace;
const AccessibleValueHandler = {
@@ -149,7 +150,7 @@
* to be called when the AccessibleValueHandler is reset.
* @returns {string} - aria-valuetext to be set to the primarySibling
*/
- a11yCreateAriaValueText: toString, // by default make sure it returns a string
+ a11yCreateAriaValueText: addHairSpace, // by default make sure it returns a string
/**
* Create content for an alert that will be sent to the utteranceQueue when the user finishes interacting
@@ -387,9 +388,8 @@
// Make sure that the new aria-valuetext is different from the previous one, so that if they are the same
// the screen reader will still read the new text - adding a hairSpace registers as a new string, but the
// screen reader won't read that character.
- const hairSpace = '\u200A';
if ( this._a11yRepeatEqualValueText && this.ariaValueText && newAriaValueText === this.ariaValueText.replace( new RegExp( hairSpace, 'g' ), '' ) ) {
- newAriaValueText = this.ariaValueText + hairSpace;
+ newAriaValueText = addHairSpace( this.ariaValueText );
}
this.ariaValueText = newAriaValueText;
Index: ratio-and-proportion/js/create/view/MyChallengeAccordionBox.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ratio-and-proportion/js/create/view/MyChallengeAccordionBox.js b/ratio-and-proportion/js/create/view/MyChallengeAccordionBox.js
--- a/ratio-and-proportion/js/create/view/MyChallengeAccordionBox.js (revision cdf28502ec162f2fecaf99a5e1bd5be18979962a)
+++ b/ratio-and-proportion/js/create/view/MyChallengeAccordionBox.js (date 1612208446164)
@@ -96,7 +96,7 @@
center: Vector2.ZERO,
accessibleName: ratioAndProportionStrings.a11y.leftValue,
a11yDependencies: [ targetConsequentProperty ],
- a11yCreateAriaValueText: ratioDescriber.getWordFromNumber,
+ // a11yCreateAriaValueText: ratioDescriber.getWordFromNumber,
a11yCreateContextResponseAlert: () => ratioDescriber.getTargetRatioChangeAlert( targetAntecedentProperty.value, targetConsequentProperty.value )
} );
const leftRatioSelector = new VBox( {
@@ -115,7 +115,7 @@
center: Vector2.ZERO,
accessibleName: ratioAndProportionStrings.a11y.rightValue,
a11yDependencies: [ targetAntecedentProperty ],
- a11yCreateAriaValueText: ratioDescriber.getWordFromNumber,
+ // a11yCreateAriaValueText: ratioDescriber.getWordFromNumber,
a11yCreateContextResponseAlert: () => ratioDescriber.getTargetRatioChangeAlert( targetAntecedentProperty.value, targetConsequentProperty.value )
} );
const rightRatioSelector = new VBox( {
|
I agree, that makes sense to me. Thanks for checking the hairspace workaround. In my opinion requiring a map that would often just be from number -> numeric word is a last resort. I would be more interested in maybe trying a different HTML element, if this is evidence that range input is semantically not a good fit for NumberPicker. Maybe that means having AccessibleValueHandler support more than |
This code in handleInput makes me worried that we would not be able to use too much of the same code if we went with sun/js/accessibility/AccessibleValueHandler.js Lines 658 to 664 in af5112a
but even before that, inputType:'number' is a completely different UI, which allows for typing in numbers directly (which aren't visible on the NumberPicker). I'm worried about how we would support that at all. My vote is to create an issue and defer it. What do you think @jessegreenberg? |
I agree, I don't think we can use inputType: 'number' for the reason you described. That input also has native keyboard input that I recall being tricky to prevent. I was thinking we might either create something new with a |
I really feel like the problem doesn't outweigh the cost of a solution at this time. I'll make a new issue. |
From phetsims/ratio-and-proportion#283, my findings were basically that any AccessibleValueHandler that we want to support for Interactive Description needs to pass in a a11yCreateAriaValueText that differs from the current value. In that sim-specific issue, we got around this by spelling out the number as a word. I'm not sure if this is the most maintainable, but it fixed the bug.
Here is a note from MDN https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/range:
So it is in the implementation of range that it is for percents. Part of some research done over in #681 was looking at input[type="number"], but I felt like the ability to input numbers directly is a non-starter for our NumberPicker implementation.
Basically I worry about any AccessibleValueHandler that we think of as "fully implemented" with Interactive Description if it doesn't set its own a11yCreateAriaValueText function. On iOS, we get the value stated as a percent (from min to max attribute values).
@jessegreenberg, can you think of a way that we can begin to require this feature to make sure that something like phetsims/ratio-and-proportion#283 doesn't occur again?
The text was updated successfully, but these errors were encountered: