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

A few ARIA updates #732

Merged
merged 6 commits into from
May 11, 2017
Merged

A few ARIA updates #732

merged 6 commits into from
May 11, 2017

Conversation

aleventhal
Copy link
Contributor

@aleventhal aleventhal commented May 7, 2017

Put important important ARIA on the <li> element and fix bugs in aria-expanded.

aleventhal added 4 commits May 7, 2017 12:00
…ed to be moved, because Fancytree treats leaf nodes as expanded in the data model
…odel, sometimes not. So let's make the logic robust and first child whether something is a leaf node. If so, remove aria-expanded. Once we know it's not a leaf node, we can use hasChildren as that is reliable.
@@ -3647,14 +3647,16 @@ $.extend(Fancytree.prototype,
if( node.expanded ){
cnList.push(cn.expanded);
if(aria){
$ariaElem.attr("aria-expanded", true);
if (hasChildren) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be if ( hasChildren !== false ) since hasChildren() returns undefined for lazy, unexpanded nodes. And we want lazy nodes to have aria-expanded set to false?

$ariaElem.attr("aria-expanded", true);
}
if( aria ){
if (hasChildren || (hasChildren === undefined && node.lazy)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment on the /src folder: Here we could use hasChildren !== false instead, since undefined is returned for lazy unloaded nodes.
note: This file change is part of the /dist folder and overwritten in the build process. We only need to edit /src/... files

}
// if(aria){
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just cleanup

if( node.key && opts.generateIds ){
node.li.id = opts.idPrefix + node.key;
}
node.span = document.createElement("span");
node.span.className = "fancytree-node";
if( aria && !node.tr ) {
$(node.span).attr("role", "treeitem");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting on the li will allow level, posinset and setsize to be computed automatically by the browser. However, I am working on a fix for Chrome as it's adding descendant text from the child tree items into the auto-computed name. This is a very important bug that it brought to light. In the meantime, the buggy behavior is not bad in Chrome, so I think we should go with this.

tabindex = opts.titlesTabbable ? " tabindex='0'" : "";

nodeTitle = "<span " + role + " class='fancytree-title'" +
id + tooltip + tabindex + ">" +
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little cleanup

@mar10 mar10 merged commit 76b8a21 into mar10:master May 11, 2017
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