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 #210

Closed
4 tasks done
mcking65 opened this issue Dec 12, 2016 · 17 comments
Closed
4 tasks done

Review accordion example #210

mcking65 opened this issue Dec 12, 2016 · 17 comments

Comments

@mcking65
Copy link
Contributor

mcking65 commented Dec 12, 2016

Final review of the accordion example page built by completion of issue #67 .

Status of requested reviews

Required updates based on feedback

  • Put role presentation on dl element
@mcking65 mcking65 added this to the 1.1 PR milestone Dec 12, 2016
@annabbott
Copy link

The "Role, Property, State, and Tabindex Attributes" indicates a region landmark. It is not my understanding that a landmark is required. Perhaps the description for the landmark should indicate it is 'optional'.

@aruns07
Copy link

aruns07 commented Dec 21, 2016

I think, the anchor element within heading of accordion could be replaced with a button, as its implicit role=button.

@ZoeBijl
Copy link
Contributor

ZoeBijl commented Jan 5, 2017

@aruns07 while I agree that is the better route to take, the point of the APG is to show have to apply ARIA. Using semantically correct HTML with implicit roles won’t do that.

@aruns07
Copy link

aruns07 commented Jan 5, 2017

@MichielBijl the example may become a reference for developers. Developers may not get opportunity to find a better route, or read the spec https://www.w3.org/TR/wai-aria-1.1/#host_general_conflict

@mcking65
Copy link
Contributor Author

mcking65 commented Jan 7, 2017

@MichielBijl

We are demonstrating how to implement the accordion pattern, which among other things, calls for headings containing a button that has aria-expanded, and in some cases, aria-disabled. If a button element is used instead of a link with a button role, wouldn't that still be demonstrating all the essential ARIA implementation?

In this case, it seems that the suggestion to use button is inline with the principle of using the minimum amount of ARIA to achieve the objective of the pattern.

There are some patterns where we explicitly do not use the minimum amount of ARIA because it is necessary to demonstrate the appropriate use of specific roles and properties. For example, we have some grid examples that have role="grid" on a table element. In these examples, the row and gridcell roles should not be used. So, we have other grid examples where we do not use a table element, even though it requires more ARIA, because we want to demonstrate the other grid roles. And, as it turns out, that has revealed some differences in screen reader and browser behavior; grids made with tables currently reveal more screen reader or browser bugs than grids made with divs.

I think I am on board with @aruns07's suggestion

@mcking65
Copy link
Contributor Author

mcking65 commented Jan 7, 2017

@annabbott

I think you have a valid point. However, in this implementation, the accordion information is structured using a dl element and the element with role="region" is a dd element. If it did not have role="region", then it would need role="presentation". So, because of the HTML structure, in this particular implementation, a role is required on that element.

It would be possible to build this accordion using h and div in place of dt and dd. In that case, role="heading" and role="region" would not be needed. I do not know that one approach is inherently better than another. I suspect @accdc has a specific reason for his choice of HTML structure.

Instead of adding a comment about region being optional, might it be simpler and cleaner to use presentation in its place?

@mcking65 mcking65 modified the milestones: Feb 2017 Heartbeat Draft, 1.1 PR Jan 7, 2017
@accdc
Copy link

accdc commented Jan 7, 2017

I've seen public examples of both cases, where simulated active elements like divs and spans are used and when native active elements are used instead such as links and buttons.

Unfortunately the scripting is very different between these two cases, and this needs to be demonstrated separately, because native active elements require only an onClick to function properly, whereas simulated active elements need to be made focusable and must include both onClick plus a key handler to simulate a click when Enter or Space is pressed. This is important because developers all too often assume the same technique that works on native active elements will work the same on simulated active elements, which causes ARIA widgets to break.

In the accordion example here, I already included the scripting for both techniques so it won't matter at all if you change the link into a button or into a focusable div or span instead, so these bases are already covered by reading the JS comments.

I'm not opposed to making one of these examples use a button instead of a link, in which case role="button" is not required on the active element.

However I don't think we need to literally cover all markup possibilities when doing this, since these examples are meant to be downloaded and experimented with as well, so we can also include instructions and comments in the markup to show developers how they can successfully make these changes themselves without breaking anything.

Regarding the use of named regions, this remains an important aspect of accordions that should be preserved as a best practice, otherwise it will be very difficult to locate the boundaries of multiple or nested accordions within the same page.

@Veyfeyken
Copy link

VoiceOver (MacOS Sierra / Safari 10) recognises 4 regions instead of 3. "Personal information" is read as "button 1 of 4". Can't explain this behaviour unfortunately.

@MarioBatusic
Copy link

#210
In the accordion example there is no semantics for the whole accordion structure. Screen reader users see each button seqentially but it is not easy for them to conclude the semantics of the complete structure.
The same problem: namly the absence of an outer semantic widget container prevents screen readers from allowing all optional keyboard shortcuts within the widget (arrow keys, control+page up and so on). This is unfortunately not testable with this example, because it misses the keyboard implementation.
I have prefered the earlier accordion simulation by a special use of roles tablist, tab, tabpannel.

The possible approvement could be the introduction of a role "accordion" for the outer container. That would signal for scren readers and other ATs to allow in-widget keyboard.

@mcking65
Copy link
Contributor Author

@MarioBatusic wrote:

In the accordion example there is no semantics for the whole accordion structure. Screen reader users see each button seqentially but it is not easy for them to conclude the semantics of the complete structure.

First, thank you for your thoughtful feedback.

The task force did discuss these types of concerns when we made the decision to set aside the tab-based approach to the accordion in favor of the current markup. Here is some of our thinking.

  1. Accordions are semantically equivalent to a set of collapsed headings.
  2. Perception of the nature of the information architecture rendered by an accordion is efficient and easy with screen reader heading navigation.
  3. Adding the accordion concept to the typical screen reader user's lexicon does not really add any substantive value.
  4. The optional keys in the accordion pattern are primarily for the benefit of keyboard users. With the heading/button semantics, those optional keys do not add value for the typical screen reader user.

The same problem: namly the absence of an outer semantic widget container prevents screen readers from allowing all optional keyboard shortcuts within the widget (arrow keys, control+page up and so on).

I am guessing that you mean, since there is not a reason for windows screen readers to automatically switch to application or focus mode that keys are not passed through. This is completely intentional because:

  1. Very often, accordions are used to structure information that screen readers users need to read rather than operate. So, staying in reading mode is actually a good thing.
  2. The heading structure of the accordion allows screen reader users to navigate among the accordion sections in reading mode, making the additional keys superfluous for screen reader users.

This is unfortunately not testable with this example, because it misses the keyboard implementation.

We plan to add the optional keys to the example (see issue #158).

I have prefered the earlier accordion simulation by a special use of roles tablist, tab, tabpannel.
The possible approvement could be the introduction of a role "accordion" for the outer container. That would signal for scren readers and other ATs to allow in-widget keyboard.

An accordion role with an accompanying accordionheader role could do that. However, we may still be able to accomplish that end without more roles by offering an alternative pattern using vertical tabs for scenarios where the accordion content is primarily widgets. Generally, though, members of the task force, including all the screen reader users, see the heading/button approach is more helpful to both developers and users.

Thank you for your review. It is important all these patterns are well vetted as we are focused on creating a viable product that can support ARIA 1.1 when it becomes a recommendation this spring. We have a long way to go just to get to a minimally viable deliverable. That is why we have pushed off some of the optional work. We welcome not only reviews but contributions.

@mcking65
Copy link
Contributor Author

The task force will discuss this review in its February 6 teleconference, specifically addressing:

  1. Should this example page use native button or link with button role?
  2. Should the best-practice structure include named regions for the panels? If so, should the pattern be updated accordingly?

@jasonkiss
Copy link

Great to see this example coming together. A few questions:

  1. The dl used to structure the accordions has default list semantics in accessibility APIs, and some required owned elements. But the dt and dd elements have their roles overwritten. Doesn't that make the dl effectively an empty definition list?

  2. I can understand the benefit in certain cases of grouping an accordion's content and setting a name for that group by reference to the relevant heading. But I wonder if the region role is not too strong as the default role for this grouping: it could result in a flood of landmarks on a page with many accordions. Wouldn't the group role be better for this purpose, where appropriate? Otherwise, isn't the relationship between the content and the heading already established by DOM order?

@mcking65
Copy link
Contributor Author

mcking65 commented Feb 2, 2017

@jasonkiss wrote:

The dl used to structure the accordions has default list semantics in accessibility APIs, and some required owned elements. But the dt and dd elements have their roles overwritten. Doesn't that make the dl effectively an empty definition list?

Yes, thanks! The dl should have role="presentation". Amazed I missed this considering my Jan 6 comments on the dd element role.

  1. I can understand the benefit in certain cases of grouping an accordion's content and setting a name for that group by reference to the relevant heading. But I wonder if the region role is not too strong as the default role for this grouping: it could result in a flood of landmarks on a page with many accordions. Wouldn't the group role be better for this purpose, where appropriate? Otherwise, isn't the relationship between the content and the heading already established by DOM order?

In general, I do not find the group role helpful as a content structure. It usually creates distracting verbosity rather than adding clarity. If region is not used, then, as you suggest, the heading structure and DOM order are adequate.

I hope that a page with more than one accordion is not common. Sounds like UI asking for rework. That said, if there were lots of accordions, using region could create landmark overload.

I agree that if we decide to include region in the pattern, that it should be an optional feature accompanied with appropriate guidance to avoid landmark overload.

mcking65 added a commit that referenced this issue Feb 16, 2017
modified:   examples/accordion/accordion1.html to address feedback in issue #210 from @jasonkiss:
1. Added role `presentation` to the `dl` element.
2. Added a row to the states and properties table for the presentation role.
3. Revised the `heading` row of the states and properties table.
4. Revised the `aria-level` row of the states and properties table.
mcking65 added a commit that referenced this issue Feb 16, 2017
modified aria-practices.html To address feedback from @annabbott and @jasonkiss in issue #210.
Added the following text to the roles, states, and properties section of the accordion pattern:

> Optionally, each element that serves as a container for panel content has role region and aria-labelledby with a value that refers to the button that controls display of the panel.
> * Avoid using the region role in circumstances that create landmark region proliferation, e.g., in an accordion that contains more than approximately 6 panels that can be expanded at the same time.
> * Role region is especially helpful to the perception of structure by screen reader users when panels contain heading elements or a nested accordion.
@mcking65
Copy link
Contributor Author

Thank you everyone for your feedback and assistance with this example. I have taken the following actions to address the feedback and am closing this review. Of course, feel free to reopen this issue if I have not adequately resolved a concern you have raised or create a new issue if you discover other problems.

To address feedback related to guidance about using the region role on accordion panels and the possibility of landmark proliferation raised by @annabbott and @jasonkiss, in commit e07aa78, I added guidance in the states and properties section of the accordion design pattern about the option to make the panel into a landmark region.

To address @jasonkiss feedback regarding the dl with no dd and dt elements, I added role presentation to the dl in commit c63e7aa.

The task force has decided to keep the current implementation with a elements that have the button role. However, to address @aruns07's recommendation to use of a button element instead, I have updated the objectives listed in issue #158.

To address feedback from @MarioBatusic regarding deficiencies in the semantic structure, I raised issue w3c/aria#530 for the ARIA specification.

With respect to @Veyfeyken's results on Mac, I tested with voiceOver on Sierra in Chrome and Safari, I can not reproduce your experience. There is no position in set information announced at all because it is not part of this pattern. VO does announce the heading levels and the number of items inside of the region, so it is possible those are the messages you are hearing. It appears all the correct information is announced, and when you look at the counts in the rotor menu (ctrl-opt-u), everything is correct.

@Veyfeyken
Copy link

@mcking65 I can't reproduce the VoiceOver issue in the current version either. The buttons are no longer announced as 'n' of 4.

I do experience another, bigger VoiceOver-problem in the current version. Tested in Mac OS Sierra with Safari 10 and Firefox 51. It's not a problem in Chrome 56. To reproduce using the VO-keys:

  • the expanded accordion header 'personal information' receives focus and is read.
  • VO + down the panel with the form for personal information is read
  • VO + down closed accordion header 'BILLING ADDRESS' receives focus and is read.
  • ENTER personal information panel closes and billing address panel opens
  • VO + downthe panel with the form for billing address is skipped completely and the accordion header 'SHIPPING ADDRESS' receives focus and is read. You have to navigate backwards to get to the billing address form.

Opening a panel sets the focus after the panel content?

I cannot reproduce the problem in iOS, nor in Win10 with Firefox and NVDA or IE11 and Jaws.

@accdc
Copy link

accdc commented Jun 12, 2017 via email

@plweil
Copy link

plweil commented May 3, 2018

Examples that rely on aria roles instead of html elements that already have the necessary native semantics could be sending the wrong message to developers; e.g., examples that contradict the "first rule of aria use":

If you can use a native HTML element [HTML51] or attribute with the semantics and behavior you require already built in, instead of re-purposing an element and adding an ARIA role, state or property to make it accessible, then do so.

Demonstrating how roles could be used to achieve the desired results is understandable as an educational exercise. But they ought to be accompanied by examples where aria roles are either minimized or eliminated altogether; i.e., best practices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

9 participants