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.

Sign up for GitHub

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

Jump to bottom

Review updates to horizontal slider example #229

Closed
7 tasks done
mcking65 opened this issue Dec 14, 2016 · 10 comments
Closed
7 tasks done

Review updates to horizontal slider example #229

mcking65 opened this issue Dec 14, 2016 · 10 comments
Assignees
@jongund @shirsha @annabbott
Labels
bug Code defects; not for inaccurate prose editorial Changes to prose that don't alter intended meaning, e.g., phrasing, grammar. May fix inaccuracies. Example Page Related to a page containing an example implementation of a pattern
Milestone
Jan 2017 Clean Up

Comments

@mcking65
Copy link
Contributor

mcking65 commented Dec 14, 2016 •
edited
Loading

The following updates have been made to the
horizontal slider example page:

  • Change page title to "Horizontal Slider Example".
  • Fix page title tag.
  • Revise to use current example template
  • Write an introductory description.

Reviews Requested as of March 11, 2017
@mcking65 mcking65 added bug Code defects; not for inaccurate prose editorial Changes to prose that don't alter intended meaning, e.g., phrasing, grammar. May fix inaccuracies. needs edits labels Dec 14, 2016
@mcking65 mcking65 added this to the 1.1 PR milestone Dec 14, 2016
@mcking65 mcking65 modified the milestones: Feb 2017 Heartbeat Draft, 1.1 PR Jan 6, 2017
@jongund
Copy link
Contributor

jongund commented Feb 21, 2017

These should be fixed now with the latest pull request, including JSCS issues

ZoeBijl pushed a commit that referenced this issue Feb 22, 2017
@jongund
Slider example update to fix issue #229 and #230
3b8c118 
- Update focus style
- Update page title
- Update description
- Fix JSCS bugs
@ZoeBijl
Copy link
Contributor

ZoeBijl commented Feb 22, 2017

Thanks Jon! This issue is fixed in: 3b8c118

mcking65 added a commit that referenced this issue Mar 11, 2017
@mcking65
Horizontal Slider Example: Revised for Consistency with Editorial Gui…
a7cca9c 
…delines and Example Template

For issue #229, modified examples/slider/slider-1.html and examples/slider/css/slider.css:
1. Changed title from "Color Picker Using Slider Widgets" to "Horizontal Slider Example".
2. Made H1 consistent with title element.
3. Added link to review issue.
4. Added link to design pattern in intro paragraph and made editorial revisions and corrections.
5. Made editorial revisions and corrections to vertical slider link and description.
6. Moved stylles for idColorBox and idColorInfo divs to slider.css from `<head>` element so they are viewable by the reader as part of the example.
7. Changed the heading inside of the example to H3 from H2 so it fits in the structure of the page.
8. Added missing screen reader separator element that marks the end of the example code.
9. Fixed H2 for roles, states, and properties to be consistent with template.
10. Added a row for tabindex to the attributes table and made other editorial revisions and corrections.
11. Fixed href value of design pattern link in nav footer.
@mcking65 mcking65 changed the title Updates needed to horizontal slider example Review updates to horizontal slider example Mar 11, 2017
@mcking65 mcking65 added Example Page Related to a page containing an example implementation of a pattern Needs Review labels Mar 11, 2017
@mcking65
Copy link
Contributor Author

@jongund, did you have a reason for adding tabindex in the JavaScript rather than putting it into the HTML?

mcking65 added a commit that referenced this issue Mar 12, 2017
@mcking65
Horizontal Slider Example: Update Similar Example Link
132777b 
For issue #229, modified examples/slider/slider-1.html:
changed the name and description of the link to the thermostat slider example.
The name of that example was changed when working on issue 230.
@jongund
Copy link
Contributor

jongund commented Mar 13, 2017

I updated the source code to include tabindex in the html code and also improved visual focus styling by including style to the rail

@annabbott
Copy link

Looks good.

mcking65 pushed a commit that referenced this issue Mar 15, 2017
@jongund @mcking65
Slider Examples: Include tabindex in HTML and Improve Rail Focus Indi…
039ee3f 
…cator

For issues #229 and #230, In response to feedback from @mcking65 logged in issue #229, made the following changes for both slider example pages:

1. Include tabindex for thumbs in HTML; was previously added with javascript during instantiation.
2. Remove note about dynamic addition of tabindex from roles, states, and properties tables.
3. Improve focus styling of rail.
4. Removed un used CSS.
@shirsha
Copy link

shirsha commented Mar 16, 2017

Working fine with keyboard, JAWS 18 in IE11 and NVDA screen reader in Firefox.

Can you add aria-valuetext=“STRING” so that screen reader can know what slider they are on while changing the values?
Ex: aria-valuetext="RED RGB value".

@mcking65
Copy link
Contributor Author

Thank you @shirsha.

Can you add aria-valuetext=“STRING” so that screen reader can know what slider they are on while changing the values?
Ex: aria-valuetext="RED RGB value".

That would be redundant with the slider label. The valuetext property is for making the value itself meaningful. In this case, the number 0 to 255 is meaningful.

Screen reader users can hear the label of the focused item at any time with their screen reader command for reading the current element.

mcking65 added a commit that referenced this issue Mar 16, 2017
@mcking65
Close Review of Horizontal Slider Example
10f0a8e 
The authoring practices task force review process is complete for this example.
Modified examples/slider/slider-1.html to remove link to review issue #229.
@mcking65
Copy link
Contributor Author

Team, we have completed updates and the review process for this example. It is now ready for the next heartbeat publication. Thank you!

@ctomczyk
Copy link

ctomczyk commented Jun 7, 2017

It doesn't work with Talkback/Android 6.0.1. Basically, I can tap on slider, but using up/down volume to change the slider has no effect. Same reported by someone else on iOS.

@mathalete
Copy link

@ctomczyk
Same on VoiceOver iOS 10.3.2 (14F89).
The role of slider and corresponding attributes seem to be exposed correctly to the Accessibility tree. Whilst the instruction is to swipe up and down to move the slider is announced, VO doesn't facilitate this.
Looks like a iOS VO bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jongund jongund

@shirsha shirsha

@annabbott annabbott

Labels
bug Code defects; not for inaccurate prose editorial Changes to prose that don't alter intended meaning, e.g., phrasing, grammar. May fix inaccuracies. Example Page Related to a page containing an example implementation of a pattern
Projects
None yet
Milestone
Jan 2017 Clean Up
Development

No branches or pull requests
7 participants
@mathalete @jongund @ctomczyk @ZoeBijl @mcking65 @shirsha @annabbott