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

Review accordion example updated with implementation of optional keyboard interactions #401

Closed
mcking65 opened this issue Jun 18, 2017 · 17 comments
Labels
Example Page Related to a page containing an example implementation of a pattern

Comments

@mcking65
Copy link
Contributor

mcking65 commented Jun 18, 2017

@gerardkcohen refactored the
accordion example
to simplify the code and add support for the optional keyboard interactions, including Up Arrow, Down Arrow, ctrl-PageUp, ctrl-PageDown, Home, and End.

This issue is to capture feedback on the updated example.

@mcking65 mcking65 added Example Page Related to a page containing an example implementation of a pattern Needs Review labels Jun 18, 2017
@mcking65 mcking65 added this to the 1.1 Rec milestone Jun 18, 2017
mcking65 added a commit that referenced this issue 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.
@DavideTriso
Copy link

The implementation actually does not prevent all accordions to be collapsed at the same time, as stated in the description. Click on the expanded accordion collapses it.

@carmacleod
Copy link
Contributor

The accordion example has odd mouse behavior, in that clicking on either the Accordion-title or Accordion-icon span inside the Accordion-trigger button do not trigger the open/close action.
Unfortunately, the title text or the drop-down arrow icon are the 2 most likely places that a mouse user will click to open/close the panel. (This behavior seen on Windows Chrome).

@LaurenceRLewis
Copy link

LaurenceRLewis commented Jul 11, 2017

@carmacleod

The only way I can work around this is to use a fake button. The reason being the nested elements do not inherit the properties of the parent button element.

<dt role="button" aria-expanded="true" class="Accordion-trigger" aria-controls="sect1" id="accordion1id" tabindex="0"><h3 class="Accordion-title">Personal Information</h3></dt>

Then for the toggle icons use
#accordionGroup .Accordion-trigger.open:before {
font-family: FontAwesome;
content: "\f077";
float: right;
color: #fff;
}
#accordionGroup .Accordion-trigger:before {
font-family: FontAwesome;
content: "\f078";
float: right;
color: #fff;
}

Or if the preference is to not use FontAwesome just the <span class="Accordion-icon"></span> back in with the current styling :-)

@gerardkcohen
Copy link
Contributor

@carmacleod I have a fix coming soon

@Malvoz
Copy link

Malvoz commented Aug 11, 2017

@carmacleod

The accordion example has odd mouse behavior, in that clicking on either the Accordion-title or Accordion-icon span inside the Accordion-trigger button do not trigger the open/close action.
Unfortunately, the title text or the drop-down arrow icon are the 2 most likely places that a mouse user will click to open/close the panel. (This behavior seen on Windows Chrome).

@Decrepidos

The only way I can work around this is to use a fake button.

@gerardkcohen

I have a fix coming soon

A solution that worked for me is setting pointer-events to 'none' like so:

.Accordion-title,
.Accordion-icon {
pointer-events: none;
}

The only downside is pointer-events aren't fully supported in all browsers (canIuse states 94% browser support atm) and as far as I know this solution doesn't have a negative effect on accessibility (?).

@LaurenceRLewis
Copy link

@gerardkcohen pointer-events is a new one for me. Thanks for mentioning it.

@gerardkcohen
Copy link
Contributor

@Malvoz @Decrepidos Waiting on #434 to get merged

@accdc
Copy link

accdc commented Sep 21, 2017

Since all of my code was taken out, the accordion triggering elements are now broken in IE11. Confirmed on Win7.

@gerardkcohen
Copy link
Contributor

@accdc This is caused initially by using Array.from() which is not available in IE 11. It would be easy enough to fix, but hoping someone (cc @mcking65?) can provide clarification on browser support. I figured ARIA 1.1 would only be supported on newer browsers anyway, so there was flexibility use newer JS features. I can certainly roll back for compatibility.

@mcking65
Copy link
Contributor Author

@gerardkcohen, sorry, I am long overdue to write up the browser support statements for the authoring practices examples.

What our task force has agreed on is that the examples need only work in recent versions of each browser at the time the example is developed. We are, though, including IE in the list. So, IE 11 does need to work along with recent versions of Chrome on both Win and Mac, Firefox on Win, and Safari on Mac. For a few examples, we are also testing on mobile Safari, but we have intentionally punted on a lot of mobile issues until at least release 2.

@gerardkcohen
Copy link
Contributor

@mcking65 Great, thanks for the confirmation! I will work it out.

gerardkcohen added a commit to gerardkcohen/aria-practices that referenced this issue Sep 24, 2017
@gerardkcohen
Copy link
Contributor

@accdc @mcking65 I have removed any JS not supported by IE11 and tested on both WIN7 and WIN8 VM's.

@mcking65 mcking65 modified the milestones: 1.1 Rec, 3Q17 Working Draft Oct 10, 2017
mcking65 added a commit that referenced this issue Oct 11, 2017
Per feedback in issue #401, make js compatible with IE 11.
@finalborgo
Copy link

Don't know if you're aiming to include Edge for the browser support as well. I just gave it a try with the accordion example (Edge 15, Windows 10). The problem is that the content (besides for the first element) won't appear when triggering the close/open by clicking the apprpriate accordion label/icon. However, it seems that an empty

is created when clicking open one of the accordion elements. Strangely, the content will appear when hovering the appropriate html with the developers tools within the DOM explorer

@gerardkcohen
Copy link
Contributor

@finalborgo Thanks for the heads up. Looks like part of your message is missing, so not sure what empty XXXX is created. In any case, I will look to test on Edge 15 and Windows 10.

@mcking65
Copy link
Contributor Author

We have not done any work to support Edge. That could potentially come in a later release depending on Edge support levels for ARIA, assistive technology support for Edge, and importance in the market place.

The goal of practices is not to demonstrate how to engineer code that works in every browser but to focus on demonstrating best use of ARIA in support of assistive technologies. That said, if our examples do not work in the most common browsers on the most common platforms, then we can'reach our intended audiences. So, we want code that will work in at least one browser that people in our audience are likely to have.

@gerardkcohen
Copy link
Contributor

@finalborgo @mcking65 I issued PR #502 that addresses Edge bugs. Not vital, but it was an easy fix.

mcking65 added a commit that referenced this issue Oct 30, 2017
modified examples/accordion/accordion.html:
Remove link to review issue #401.
@mcking65
Copy link
Contributor Author

All issues related to this change addressed; closing review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Example Page Related to a page containing an example implementation of a pattern
Development

No branches or pull requests

8 participants