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

Stories/af/fix main aria roles properties #4348

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/js/select2/dropdown/infiniteScroll.js
Expand Up @@ -75,7 +75,7 @@ define([
var $option = $(
'<li ' +
'class="select2-results__option select2-results__option--load-more"' +
'role="treeitem" aria-disabled="true"></li>'
'role="option" aria-disabled="true"></li>'
);

var message = this.options.get('translations').get('loadingMore');
Expand Down
14 changes: 10 additions & 4 deletions src/js/select2/dropdown/search.js
Expand Up @@ -9,9 +9,9 @@ define([

var $search = $(
'<span class="select2-search select2-search--dropdown">' +
'<input class="select2-search__field" type="search" tabindex="-1"' +
'<input class="select2-search__field" type="text" tabindex="-1"' +
Copy link
Member

Choose a reason for hiding this comment

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

The rest of the changes make sense, but I can't figure out why this change was needed. Can a search field not function as a combobox?

Copy link
Author

Choose a reason for hiding this comment

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

Well, the semantics of this input field is completely overridden by the applied ARIA role. Technologies that understand ARIA will get it is a "combobox" (also, currently has a role="textbox") so the type attribute is not so much relevant.
About browsers, the only difference with a text box I'm aware of is webkit special styling which Select2 resets with CSS

' autocomplete="off" autocorrect="off" autocapitalize="off"' +
' spellcheck="false" role="textbox" />' +
' spellcheck="false" role="combobox" aria-autocomplete="list" />' +
'</span>'
);

Expand All @@ -25,6 +25,7 @@ define([

Search.prototype.bind = function (decorated, container, $container) {
var self = this;
var resultsId = container.id + '-results';

decorated.call(this, container, $container);

Expand All @@ -48,7 +49,7 @@ define([

container.on('open', function () {
self.$search.attr('tabindex', 0);

self.$search.attr('aria-owns', resultsId);
self.$search.focus();

window.setTimeout(function () {
Expand All @@ -58,7 +59,8 @@ define([

container.on('close', function () {
self.$search.attr('tabindex', -1);

self.$search.removeAttr('aria-activedescendant');
self.$search.removeAttr('aria-owns');
self.$search.val('');
});

Expand All @@ -73,6 +75,10 @@ define([
}
}
});

container.on('results:focus', function (params) {
self.$search.attr('aria-activedescendant', params.data._resultId);
});
};

Search.prototype.handleSearch = function (evt) {
Expand Down
6 changes: 3 additions & 3 deletions src/js/select2/results.js
Expand Up @@ -14,7 +14,7 @@ define([

Results.prototype.render = function () {
var $results = $(
'<ul class="select2-results__options" role="tree"></ul>'
'<ul class="select2-results__options" role="listbox"></ul>'
Copy link
Member

Choose a reason for hiding this comment

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

How well does this work with a nested group role?

Part of the reason why tree was picked was because it supported nesting of options. From what we could tell, listbox only supports a single level (completely flat).

Copy link
Author

Choose a reason for hiding this comment

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

A "tree" widget is something different, from the spec:

A type of list that may contain sub-level nested groups that can be collapsed and expanded
The ARIA authoring practices describe the interaction screen readers should implement with ARIA widgets, I think using a Tree View is one of the main reasons the Select2 options are not announced. See version 1.0 and 1.1 (both working drafts):
https://www.w3.org/TR/wai-aria-practices/#TreeView
https://www.w3.org/TR/wai-aria-practices-1.1/#TreeView

On the other hand, the Combobox widget explicitly requires a listbox with options, see:
https://www.w3.org/TR/wai-aria-practices/#combobox
https://www.w3.org/TR/wai-aria-practices-1.1/#combobox

I'm afraid ARIA doesn't provide a way to emulate the semantics of optgroup or at least I wasn't able to find it. Nested groups would require further investigation and iteration. That's why I'm proposing to proceed with small steps and then iterate, if you agree :)

Testing the PR, only Firefox+NVDA announce the group "labels", IE11+JAWS skip the labels but do announce the options, Safari+VoiceOver announce just the "flat" ones.

Copy link

@fuzzbomb fuzzbomb Sep 21, 2018

Choose a reason for hiding this comment

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

I'm afraid ARIA doesn't provide a way to emulate the semantics of optgroup

Also note that plain old HTML optgroups don't get conveyed to screen readers either, so there's no great loss there

Copy link
Author

Choose a reason for hiding this comment

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

For reference, in ARIA 1.2 role=group will be allowed as child of role=listbox to emulate optgroup. See w3c/aria#782

);

if (this.options.get('multiple')) {
Expand All @@ -37,7 +37,7 @@ define([
this.hideLoading();

var $message = $(
'<li role="treeitem" aria-live="assertive"' +
'<li role="alert" aria-live="assertive"' +
' class="select2-results__option"></li>'
);

Expand Down Expand Up @@ -163,7 +163,7 @@ define([
option.className = 'select2-results__option';

var attrs = {
'role': 'treeitem',
'role': 'option',
'aria-selected': 'false'
};

Expand Down
9 changes: 7 additions & 2 deletions src/js/select2/selection/base.js
Expand Up @@ -40,6 +40,7 @@ define([

var id = container.id + '-container';
var resultsId = container.id + '-results';
var searchHidden = this.options.get('minimumResultsForSearch') === Infinity;

this.container = container;

Expand All @@ -60,7 +61,9 @@ define([
});

container.on('results:focus', function (params) {
self.$selection.attr('aria-activedescendant', params.data._resultId);
if (searchHidden) {
self.$selection.attr('aria-activedescendant', params.data._resultId);
}
});

container.on('selection:update', function (params) {
Expand All @@ -70,7 +73,9 @@ define([
container.on('open', function () {
// When the dropdown is open, aria-expanded="true"
self.$selection.attr('aria-expanded', 'true');
self.$selection.attr('aria-owns', resultsId);
if (searchHidden) {
self.$selection.attr('aria-owns', resultsId);
}

self._attachCloseHandler(container);
});
Expand Down
9 changes: 6 additions & 3 deletions src/js/select2/selection/search.js
Expand Up @@ -10,9 +10,9 @@ define([
Search.prototype.render = function (decorated) {
var $search = $(
'<li class="select2-search select2-search--inline">' +
'<input class="select2-search__field" type="search" tabindex="-1"' +
'<input class="select2-search__field" type="text" tabindex="-1"' +
' autocomplete="off" autocorrect="off" autocapitalize="off"' +
' spellcheck="false" role="textbox" aria-autocomplete="list" />' +
' spellcheck="false" role="combobox" aria-autocomplete="list" />' +
'</li>'
);

Expand All @@ -28,16 +28,19 @@ define([

Search.prototype.bind = function (decorated, container, $container) {
var self = this;
var resultsId = container.id + '-results';

decorated.call(this, container, $container);

container.on('open', function () {
self.$search.attr('aria-owns', resultsId);
self.$search.trigger('focus');
});

container.on('close', function () {
self.$search.val('');
self.$search.removeAttr('aria-activedescendant');
self.$search.removeAttr('aria-owns');
self.$search.trigger('focus');
});

Expand All @@ -56,7 +59,7 @@ define([
});

container.on('results:focus', function (params) {
self.$search.attr('aria-activedescendant', params.id);
self.$search.attr('aria-activedescendant', params.data._resultId);
});

this.$selection.on('focusin', '.select2-search--inline', function (evt) {
Expand Down