Improve details summary polyfill #57
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 8 commits
January 27, 2015 15:07
This wasn’t correctly detecting the “open” attribute. This is a quick fix to get feedback from Leonie and *needs* refactoring and tests. For now, it has two checks - first for native support, which uses the boolean ‘open’ attribute. Then for non-native support, which checks for an ‘open’ attribute with an empty value, otherwise ‘open’ is undefined (not present) and the details content should be closed. This check is also used to either create an open or closed arrow (pointing right or down) depending on the initial ‘open’ state.
In Chrome, this was triggering twice - both for click and “space”. Removing the keydown event and adding a mouseup event, ensures this now triggers when the enter or space key is pressed. Anywhere `console.log` appears is commented out (rather than removed) while this is tested further.
Simplify test page before asking for feedback from Leonie
JAWS reads “black down pointing triangle”, since using role=“button” indicates that the button is expanded or collapsed, hide any description of the arrow character.
The summary wasn’t accessible via the keyboard in IE6 & 7, so couldn’t be tabbed to. The bug is described here: http://www.saliences.com/browserBugs/tabIndex.html Using .tabIndex = 0, rather than .setAttribute('tabindex', '0') fixes this.
|
Really interesting summary. Thanks. |
|
This is a great improvement and the summary of why the changes are needed is brilliant. 👍 |
gemmaleigh
force-pushed
the
improve-details-summary-polyfill
branch
2 times, most recently
from
737c3c0 to
e4487e2
Compare
added 2 commits
January 29, 2015 15:39
When the space key is used to open `<details>` the page scrolls. This cancels the event and stops the page from scrolling.
In IE9 and below the `event` object [does not have a `preventDefault` method](http://help.dottoro.com/ljwolcsp.php). Use `event.returnValue = false;` for IE instead.
tombye
added a commit
that referenced
this pull request
Jan 30, 2015
Improve details summary polyfill
|
I've re-read the spec and my original presumption was incorrect so it's worth explaining why, mainly because it also explains why using In our use-case, the markup we tried for the arrow is: The ▼ is a text node and a child of the "...the user agent MUST expose content and descendant elements that do not have an explicit or inherited role of presentation." The ▼ text node has a role of "text" so it will be exposed. This explains why the arrow was still being spoken when the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current polyfill wasn't correctly detecting the "open" attribute and was firing twice in Chrome,
when the summary was clicked.
For now, it has two checks - first for native support, which uses the boolean "open" attribute. Then for non-native support, which checks for an "open" attribute with an empty value, otherwise "open" is
undefined (not present) and the details content should be closed.
For non-native support, an open or closed arrow (pointing right or down) is added, and styled to look like the native arrow in Chrome - depending on the initial "open" state. This arrow is hidden from assistive technology using
role="presentation". Read the W3C Recommendation here.A detailed explanation from Léonie:
<details role="group">The implicit role for
<details>should be "group". Since most browsers haven't implemented support for the element (let alone accessibility support), we need to apply this role manually – so that assistive technologies know that it's a container for a group of related content.<summary role="button" aria-controls="details-content-0" tabindex="0" aria-expanded="false">…</summary>[1] role="button *
The implicit role for
<summary>should be "button", but as with<details>we need to provide the accessibility support explicitly using the role attribute.[2] aria-controls="details-content-0"
aria-controlsis not consistently supported by assistive technologies.Those that do support it usually provide a keyboard shortcut that lets the user move focus to the start of the element being controlled. It is not expected behaviour for a screen reader to automatically start to speak the content of the disclosure widget when it opens.
aria-controlsin any case.[3] tabindex="0" *
Without basic browser support for
<summary>it can't be focused on with the keyboard. Using tabindex="0" places the element into the natural tab order of the page, so as someone using a keyboard tabs through the page they'll be able to focus on it in order to activate it.[4] aria-expanded="false" *
The aria-expanded attribute informs assistive technologies that the
<summary>element triggers the expansion/collapse of something. When focus first moves to the<summary>element, its state is announced as "collapsed", and once the button is activated it changes to "expanded".