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
FLUID-5536: Added the textfield stepper #823
Conversation
Code taken from fluid-project#726
…es using the jQuery UI slider.
Cleaned up templates, context awareness checks, and tests.
Separating functionality that will be used by the stepper.
Recreated and updated the styling for the textfield slider, by copying the stylus styles and writing new CSS for it.
Moved TextfieldControl to its own file, and renamed directory after it.
Please also correct these instructions due to the removal of jQuery UI support in the prefs framework: https://github.com/jobara/infusion/blob/FLUID-5536/tests/manual-tests/framework/preferences/assortedContent/index.html#L88 |
This is more generic and is consistent with changes in FLUID-5708
Using IoC testing framework
@cindyli I've addressed your comments and merged with master. Ready for more review. |
When using VoiceOver with firefox, safari on Mac with the demo |
Text field sliders used in these examples are not working properly. Moving slider handle doesn't trigger the change of the text field value and vice versa.
|
@cindyli I think the issue with VoiceOver announcing the initial value of the stepper is a problem with support by the screen reader and/or browser. I had the same result with this example ( http://oaa-accessibility.org/example/33/ ). I also found that NVDA + Firefox worked, but JAWS + IE 11 didn't; for both the demo and OAA example. |
<div> | ||
<label class="mpe-slider-label"></label> | ||
<div class="mpe-slider"> | ||
<input aria-labelledby="text-size-label" class="flc-textfieldSlider-slider fl-slider"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the value of aria-labelledby
at line 4, 5 be mpe-slider-label
rather than text-size-label
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, my above comment is wrong. mpe-slider-label
is actually a css class on the label element at line 2. An id
value should be added to that label element and be used as aria-labelledby
values at line 4, 5.
@cindyli thanks for catching the aria-labelledby bug. It's been fixed and is ready for more review. |
Running |
Explicitly waiting for afterRender
@cindyli there was a race condition with the tests. I've fixed that and ready for more review. |
@cindyli that's by design. For high contrast it's hard to have a "greyed" out version for the disabled button, and we were thinking that other methods like adding lines through make it appear more salient. |
Merged at 365e884 |
Updated the text size and line spacing panels to use the textfield stepper, and removed the jQuery UI version of the textfield slider.
https://issues.fluidproject.org/browse/FLUID-5536
Related Docs PR: fluid-project/infusion-docs#120