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

Issue #158 Add Optional Keyboard Commands to Accordion Example #397

Merged
merged 1 commit into from
Jun 18, 2017

Conversation

gerardkcohen
Copy link
Contributor

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.

Also refactored JS and CSS to be simpler to understand.

@mcking65
Copy link
Contributor

@gerardkcohen, thank you! This looks excellent. I really like the refactoring, especially the use of data attribs to control the expand/collapsing behaviors and the css icons.

On windows, Ctrl+pageUp/Down are not working for me; the browser is grabbing them. Also it looks like you are not filtering out modifier keys when not applicable, e.g., ctrl+home/end do same as home/end. Should be ignoring all modifiers on those.

Nonetheless, I don't want to keep this out of the publication we are working on for this week, so I am merging and opening a separate review issue to collect this and other feedback.

@mcking65 mcking65 merged commit f6e1121 into w3c:master Jun 18, 2017
mcking65 added a commit that referenced this pull request Jun 18, 2017
Given the significance of the updates made in pull request #397 by @gerardkcohen,
This commit adds  a link to a review issue to gather feedback on the updates and collect issues.
@gerardkcohen gerardkcohen deleted the 158-accordion-with-optional-keys branch June 20, 2017 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants