-
Notifications
You must be signed in to change notification settings - Fork 356
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
Issue #158 Add Optional Keyboard Commands to Accordion Example #308
Issue #158 Add Optional Keyboard Commands to Accordion Example #308
Conversation
Thank you for your contribution @gerardkcohen, much appreciated. We’ll review this soon™. |
Introduce an enhancement for the layout grid example review in issue w3c#227. The first layout grid provides a NUX to help new keyboard users understand that arrow keys can be used to navigate. This change will cause the NUX to be displayed only if the grid receives focus via the tab key.
Focus may move inside the grid as a result of a mouse click or screen reader call to an accessibility API. This change adds a check for the location of focus before processing navigation key events.
1. Adds js for visual scrolling when using keyboard navigation in a listbox with scrollHeight > clientHeight. 2. Adds third example implementation with larger set of options to demonstrate scrolling.
For issue w3c#246, add javascript and content for an example feed.
For issue w3c#245, modified feed pattern section in aria-practices.html to correct multiple typos and make minor editorial revisions to wording of the description.
The review process is now complete for this example page. modified examples/link/link.html: Removed paragraph with link to issue w3c#228, which was the review issue for this example.
modified examples/disclosure/disclosure-faq.html: Corrected two misspellings of the word "hidden" identified by @annabbott in issue w3c#265.
…delines and Example Template For issue w3c#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.
For issue w3c#230, made following changes to examples/slider/slider-2.html to make it consistent with editorial guidelines and the template for examples: 1. Changed title to "Slider Examples with aria-orientation and aria-valuetext" 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 horizontal slider link and description. 6. Removed region role from example container. 7. Removed an H2 heading before the horizontal sliders and made all three sliders part of a single example container. 8. Added missing separator elements that mark the start and end of the example container for screen reader users. 9. Made editorial revisions to the keyboard table. 10. Made text of H2 for roles, states, and properties consistent with template. 11. Added a row for tabindex to the attributes table and made other editorial revisions and corrections. 12. Fixed href value of design pattern link in nav footer.
For issue w3c#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.
Modified slider design pattern section in aria-practices.html: 1. Changed keyboard list to put one key per list item per editorial guidelines. 2. Made example link names and descriptions consistent with the updated example pages.
For issue w3c#311: 1. Update js so space bar now toggles the expanded/collapsed state of the disclosure. 2. Update documentation.
… Pages Updated code generation script to: 1. exclude container element 2. Include comments 3. fixed problem that excluded script element content.
Modified examples/button/button.html: 1. Add link to review issue 316. 2. Fix href of links to button design pattern. 3. Revise wording of introduction. 4. Add links to menubutton examples. 5. Add elements from template that provide separators that mark start and end of example for screen reader users. 6. Remove two H2 headings for the buttons and replace with a single H2 for the example section. 7. Editorial revisions to keyboard table. 8. Editorial revisions to states and properties table.
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.
Gerard, thank you for these excellent improvements! Nicely done!!
I made a couple of changes, and would like it if you could fix one issue.
First, note that I pushed 2 commits to the 158-accordion-optional-keys branch of your fork. So, please pull those changes in before making any further changes. The two changes I amde:
- I ran
npm test
and found 10 JSCS errors. I fixed them all automatically withnpm run jscs-fix
. Please see the readme for info about our use of jscs and runnpm test
to check the format of the your js. - I made some editorial revisions to the keyboard table in the html file. It was all nit-pick kind of stuff for consistency with our guidelines ... no worries there. You did fab job of updating the table.
One issue to fix: Ctrl+PageUp and Ctrl+PageDown are still performing their default actions in the browser. So, if in Firefox, the script is processed but then the browser changes focus to another browser tab. We want the default browser action to occur if focus is not in the accordion, but we do not want it if focus is in the accordion.
If you add another commit to your 158-accordion-optional-keys branch and push it, it will automatically update this pull request. Then, I can squash and merge all the commits.
…cator For issues w3c#229 and w3c#230, In response to feedback from @mcking65 logged in issue w3c#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.
The modal dialog example development for issue w3c#103 is complete. The following changes start the review process. Modified aria-practices.html to replace link to issue 103 with a link to the new example page. Modified examples/dialog-modal/dialog.html to add link to review issue 321.
1. Add link to new aria-activedescendant menu button example. 2. Correct HREF attributes for existing menu button example links. 3. Revise descriptions of menu button examples.
Modified aria-practices.html: 1. Added guidance for aria-controls stating that using aria-controls is optional and how to set it. 2. For issue w3c#362, added guidance for aria-expanded: set true when menu displayed and remove when not displayed, alternatively set false when menu is hidden.
…owing changes to the modal dialog pattern. Change 1: Replaced "If a dialog is limited to interactions that either provide additional information or continue processing, it might set focus to the element deemed to be most frequently desired, such as a “OK” or “Continue” button." with "If a dialog is limited to interactions that either provide additional information or continue processing, it may be advisable to set focus to the element that is likely to be most frequently used, such as an “OK” or “Continue” button." Change 2: Replaced "When a dialog closes, focus typically returns to the element that had focus before the dialog was invoked. This is often the control that opened the dialog. In circumstances where that element no longer exists, focus is set on an element that supports a logical work flow." with "When a dialog closes, focus is set on an element that supports a logical work flow. If the element that had focus before the dialog was invoked still exists, that element is usually the most appropriate choice." Also made the following additional revisions and editorial consistency corrections. Change 1: Replaced "The window under a modal dialog is typically inert; users cannot interact with content outside the dialog window. If the background window is not inert, then interaction with elements in the background window cause the modal window to close." with "Windows under a modal dialog are inert; users cannot interact with content outside an active dialog window. The inert content outside an active dialog is typically visually obscured or dimmed so it is difficult to discern, and in some implementations, attempts to interact with the inert content cause the dialog to close." Change 2: Added a third note to the keyboard section notes: "3. It is strongly recommended all dialogs include a visible element with role button that closes the dialog, such as a close icon or cancel button, and that the element is in the Tab sequence." Changed 3: Replaced "All controls required to operate the dialog are child nodes of the element which has role set to dialog." with "All elements required to operate the dialog are descendants of the element that has role dialog." Change 4: Replaced "The aria-describedby property can be set on the element with the dialog role to indicate which element or elements in the dialog contain content that describes the primary purpose or message of the dialog." with "Optionally, the aria-describedby property is set on the element with the dialog role to indicate which element or elements in the dialog contain content that describes the primary purpose or message of the dialog." Changed 5: Replaced "If the dialog has aria-modal set to true and content outside the dialog is completely inert and visually obscured to an extent that is intentionally unreadable, it is no longer necessary to set aria-hidden to true on each element containing a portion of the inert layer. However, if you do still use aria-hidden set to true, the dialog container element cannot be a descendant of any element that has aria-hidden set to true and the content outside the modal dialog must not be visually discernable." with two notes: * Because marking a dialog modal by setting aria-modal to true can prevent users of some assistive technologies from perceiving content outside the dialog, users of those technologies will experience severe negative ramifications if a dialog is marked modal but is not modal for other users. So, mark a dialog modal only when: 1. Application code prevents all users from interacting in any way with content outside of it. 2. Visual styling obscures the content outside of it. * The aria-modal property introduced by ARIA 1.1 replaces aria-hidden for informing assistive technologies that content outside a dialog is inert. However, in legacy dialog implementations where aria-hidden is used to make content outside a dialog inert for assistive technology users, it is important that: 1. aria-hidden is set to true on each element containing a portion of the inert layer. 2. The dialog element is not a descendant of any element that has aria-hidden set to true.
Slightly revised note 3 in the keyboard subsection.
1. For issue w3c#232, in the complementary aside element for related w3c specs, Changed HREF attributes and some link text. 2. Changed region name from "Related W3C Specs" to "related W3C Documents".
In footers, changed "Authoring Practices Guide Working Group" to "Authoring Practices Task Force".
…#387) In disclosure.js, tabindex was unnecessarily being set to 0 on the button that controls the disclosure. Removed that line of code.
I was reminded that some focusable elements are not in the tab sequence so the description of the keyboard behavior is not clear because it says that tab moves focus to next focusable element. So, for issue w3c#325, made the following changes. Added the following paragraph to the beginning of the keyboard subsection: > In the following description, the term “tabbable element” refers to any element with a tabindex value of zero or greater. > Note that values greater than 0 are strongly discouraged. Added the following to the keyboard interaction description as the first item in the list: > When a dialog opens, focus moves to an element inside the dialog. See notes below regarding initial focus placement. In the subsequent bullet items in the keyboard description, replaced the word "focusable" with "tabbable".
For issues w3c#223, w3c#224, w3c#225, and w3c#226, made the following changes. 1. In the `tree` row of the states and properties table, change "an tree" to "a tree". 2. In the `tree` row of the states and properties table, removed unnecessary bullet that defines the tree role. 3. In the `tree` row of the states and properties table, reworded the bullet about the roving tabindex focus management. 4. In the source code section of the navigation treeview examples, removed link to no longer existent treeitemClick.js file.
For issue w3c#384: 1. Per feedback from @a11ydoer, fixed broken link in footer nav. 2. Per feedback from @annabbott, changed spelling of "lable" to "label".
Per feedback from @annabbott in issue w3c#384, the optional behavior for the space bar of activating a menu item is not implemented. Removed this key from the table of keys supported in the menu.
…sue w3c#326 Per feedback from @annabbott in issue w3c#326, revised documentation for enter and space to fix subject/verb agreement.
…section Per feedback from @a11ydoer in issue w3c#325, changed: > Because marking a dialog modal by setting aria-modal > to true can prevent users of some assistive technologies from perceiving content outside the dialog, > users of those technologies will experience severe negative ramifications if a dialog is marked modal but is not modal for other users. To: > Because marking a dialog modal by setting aria-modal > to true can prevent users of some assistive technologies from perceiving content outside the dialog, > users of those technologies will experience severe negative ramifications if a dialog is marked modal but does not behave as a modal for other users.
Per feedback from @annabbott on issue w3c#325 in the May 1, 2017 task force meeting, changed: > So, mark a dialog modal <strong>only when:</strong> To add the word both: > So, mark a dialog modal <strong>only when both:</strong>
…abel" (pull w3c#392) 1. To fix bug raised by @racer2207 in issue w3c#384, removed event support for LEFT and RIGHT arrow keys. 2. Changed "lable" to "label" in keyboard table.
The button example page includes the menu button examples in its list of similar examples. In pull w3c#375, the directory structure and file names for menubutton examples were changed, which broke those links. This commit updates the list of similar examples to include all 3 menu buttons with appropriate descriptions and correct href attributes.
Modified aria-practices.html per feedback from @jared-w-smith in issue w3c#384 to add optional up arrow key behavior for menubutton: (Optional) <kbd>Up Arrow</kbd>: opens the menu and moves focus to the last menu item.
@mcking65 I made the change requested and followed the nice instructions you provided, but I think something happened... this PR looks a mess, which I think you predicted. Maybe we close this PR and I start over? |
Closing in favor of #397 |
For Initial Review
This is a first attempt to contribute to this project, so this is initially for (code) review and process analysis/ exploration. There is a functional contribution here as well.
Added the following (optional) keyboard commands based on issue #158:
Up/ Down: Cycle focus through accordion toggle controls/ headings
Home/ End: Focus first/ last accordion toggle control/ heading
Control + Page Up/ Down: Move focus from accordion panel to accordion heading, and then cycle through headings.
Additionally, the accordion toggles were changed from anchors with
role="button
to native<button>
's. This required some minor updates to the CSS.Lastly, there was an issue with the
forEach
helper method when iterating over objects that included inherited prototype members instead of only instance. I encountered a few bugs due to this.