Dialog: Fixes for #7862 - dialog accessibility #704

Closed
wants to merge 7 commits into from
View
2 demos/dialog/animated.html
@@ -28,7 +28,7 @@
});
$( "#opener" ).click(function() {
- $( "#dialog" ).dialog( "open" );
+ $( "#dialog" ).dialog( "open", $(this));
return false;
});
});
View
2 demos/dialog/modal-form.html
@@ -109,7 +109,7 @@
$( "#create-user" )
.button()
.click(function() {
- $( "#dialog-form" ).dialog( "open" );
+ $( "#dialog-form" ).dialog( "open");
});
});
</script>
View
30 ui/jquery.ui.dialog.js 100644 → 100755
@@ -110,16 +110,22 @@ $.widget("ui.dialog", {
})
.appendTo( "body" ),
+ // Add a document wrapper inside the dialog for a11y.
@scottgonzalez
jQuery Foundation member

This will be the first place that we set document or application role in jQuery UI. Can you explain why this is needed and whether it affects all AT or just specific ones?

Hi Scott
Everett Zufelt said this was needed to sandbox the screen reader to the modal dialog.
He's the best one to answer this.
LR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ uiDialogDocument = options.modal ? ( this.uiDialogDocument = $( "<div>" ) )
+ .attr( "role", "document")
+ .addClass( "ui-dialog-document" )
+ .appendTo( uiDialog) : null,
+
uiDialogContent = this.element
.show()
.removeAttr( "title" )
.addClass( "ui-dialog-content ui-widget-content" )
- .appendTo( uiDialog ),
+ .appendTo( options.modal ? uiDialogDocument : uiDialog ),
uiDialogTitlebar = ( this.uiDialogTitlebar = $( "<div>" ) )
.addClass( "ui-dialog-titlebar ui-widget-header " +
"ui-corner-all ui-helper-clearfix" )
- .prependTo( uiDialog ),
+ .prependTo( options.modal ? uiDialogDocument : uiDialog ),
uiDialogTitlebarClose = $( "<a href='#'></a>" )
.addClass( "ui-dialog-titlebar-close ui-corner-all" )
@@ -148,9 +154,15 @@ $.widget("ui.dialog", {
.addClass( "ui-dialog-buttonset" )
.appendTo( uiDialogButtonPane );
+ if (options.modal) {
@scottgonzalez
jQuery Foundation member

Is there anything we need to do for non-modal dialog to provide context?

Everett noted that ideally the modal would be a sibling to its triggering element but obviously thats not achievable.
He was happy with this removed from non-modal dialogs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ // We should only sandbox user to dialog if modal (a11y).
+ uiDialog.attr({
+ role: "dialog"
+ });
+ }
uiDialog.attr({
- role: "dialog",
- "aria-labelledby": uiDialogTitle.attr( "id" )
+ "aria-labelledby": uiDialogTitle.attr( "id" ),
+ "aria-describedby": uiDialogContent.attr( "id" )
@scottgonzalez
jQuery Foundation member

What is the benefit of providing the entire content as the description? Do users not just interact with the contents of the dialog right away since that's where focus should be?

Again, Everett said this meant the screen reader read the contents to the user once the dialog was active.
(I'm not much help here, just the coder - Everett's the expert!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
});
uiDialogTitlebar.find( "*" ).add( uiDialogTitlebar ).disableSelection();
@@ -252,6 +264,12 @@ $.widget("ui.dialog", {
$.ui.dialog.maxZ = maxZ;
}
+ // return the focus to the triggering element for a11y.
+ if (this.triggerEl) {
+ this.triggerEl.focus();
+ this.triggerEl = false;
+ }
+
return this;
},
@@ -299,6 +317,10 @@ $.widget("ui.dialog", {
return;
}
+ if (document.activeElement) {
+ this.triggerEl = $( document.activeElement )
@scottgonzalez
jQuery Foundation member

this.document[0].activeElement for cross-window support. Also, have you run into cases where there is no active element? That should only occur if the document hasn't loaded yet.

Yeah, document.activeElement does work if no element is focussed, ie if I just click the trigger. But seeing as though this is for a11y, in most cases we'd assume they were tabbing and focussing anyway. Happy to do it another way if there's a better approach.

@scottgonzalez
jQuery Foundation member

What browsers are you seeing this happen in? Can you provide a demo page showing this? Browsers should report <body> when no element is focused, assuming the document has loaded. If you click on a button to open the dialog, then the button may not have focus, but that's fine because as a mouse user, you don't care if focus goes back to the body. If you tab to the button and press enter to open the dialog, then the button should be the active element, which is what we want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+
var hasFocus,
options = this.options,
uiDialog = this.uiDialog;