Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: options.js

Issue 29332808: Issue 2408 - Improved accessibility of checkboxes in options page (Closed)
Patch Set: Created Jan. 19, 2016, 3:10 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « options.html ('k') | skin/options.css » ('j') | skin/options.css » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: options.js
===================================================================
--- a/options.js
+++ b/options.js
@@ -23,6 +23,7 @@
var recommendationsMap = Object.create(null);
var filtersMap = Object.create(null);
var collections = Object.create(null);
+ var maxLabelId = 0;
function Collection(details)
{
@@ -64,19 +65,19 @@
for (var i = 0; i < arguments.length; i++)
{
var item = arguments[i];
- var text = item.title || item.url || item.text;
var listItem = document.createElement("li");
listItem.appendChild(document.importNode(template.content, true));
listItem.setAttribute("data-access", item.url || item.text);
- listItem.querySelector(".display").textContent = text;
- if (text)
- listItem.setAttribute("data-search", text.toLowerCase());
+ var labelId = "label-" + (++maxLabelId);
+ listItem.querySelector(".display").setAttribute("id", labelId);
var control = listItem.querySelector(".control");
if (control)
{
+ // We use aria-labelledby to avoid triggering the control when
+ // interacting with the label
+ control.setAttribute("aria-labelledby", labelId);
control.addEventListener("click", this.details[j].onClick, false);
- control.checked = item.disabled == false;
}
this._setEmpty(table, null);
@@ -84,6 +85,7 @@
table.insertBefore(listItem, table.childNodes[this.items.indexOf(item)]);
else
table.appendChild(listItem);
+ this.updateItem(item);
}
}
return length;
@@ -100,12 +102,53 @@
{
var table = E(this.details[i].id);
var element = table.childNodes[index];
+
+ // Element gets removed so make sure to handle focus appropriately
+ var control = element.querySelector(".control");
+ if (control && control == document.activeElement)
+ {
+ if (!focusNextElement(element.parentElement, control))
+ {
+ // Fall back to next focusable element within same tab or dialog
+ var tab = element.parentElement;
saroyanm 2016/01/25 14:45:43 Detail: This is not only tab already, so maybe mak
Thomas Greiner 2016/01/25 18:04:21 Done.
+ while (tab)
saroyanm 2016/01/25 14:45:43 What if the dialog where the collection is impleme
Thomas Greiner 2016/01/25 18:04:21 The main challenge with focus management is that y
saroyanm 2016/01/26 18:40:12 Fare enough.
+ {
+ if (tab.classList.contains("tab-content")
+ || tab.classList.contains("dialog-content"))
saroyanm 2016/01/25 14:45:43 I think we need to make it obvious whether it's so
Thomas Greiner 2016/01/25 18:04:21 We could put this into a separate function that's
+ break;
+
+ tab = tab.parentElement;
+ }
+ focusNextElement(tab || document, control);
+ }
+ }
+
element.parentElement.removeChild(element);
if (this.items.length == 0)
this._setEmpty(table, this.details[i].emptyText);
}
};
+ Collection.prototype.updateItem = function(item)
+ {
+ var access = (item.url || item.text).replace(/'/g, "\\'");
+ for (var i = 0; i < this.details.length; i++)
+ {
+ var table = E(this.details[i].id);
+ var element = table.querySelector("[data-access='" + access + "']");
+ if (!element)
+ continue;
+
+ var text = item.title || item.url || item.text;
+ element.querySelector(".display").textContent = text;
+ if (text)
+ element.setAttribute("data-search", text.toLowerCase());
+ var control = element.querySelector(".control[role='checkbox']");
+ if (control)
+ control.setAttribute("aria-checked", item.disabled == false);
+ }
+ };
+
Collection.prototype.clearAll = function()
{
this.items = [];
@@ -119,11 +162,27 @@
}
};
+ function focusNextElement(container, currentElement)
+ {
+ var focusables = container.querySelectorAll("a, button, input, .control");
+ focusables = Array.prototype.slice.call(focusables);
+ var index = focusables.indexOf(currentElement);
+ index += (index == focusables.length - 1) ? -1 : 1;
+
+ var nextElement = focusables[index];
+ if (!nextElement)
+ return false;
+
+ nextElement.focus();
+ return true;
+ }
+
function onToggleSubscriptionClick(e)
{
e.preventDefault();
- var subscriptionUrl = e.target.parentNode.getAttribute("data-access");
- if (!e.target.checked)
+ var checkbox = e.target;
+ var subscriptionUrl = checkbox.parentElement.getAttribute("data-access");
+ if (checkbox.getAttribute("aria-checked") == "true")
{
ext.backgroundPage.sendMessage({
type: "subscriptions.remove",
@@ -220,30 +279,21 @@
{
function onObjectChanged()
{
- var access = (subscriptionUrl || subscription.text).replace(/'/g, "\\'");
- var elements = document.querySelectorAll("[data-access='" + access + "']");
- for (var i = 0; i < elements.length; i++)
+ for (var i in collections)
+ collections[i].updateItem(subscription);
+
+ var recommendation = recommendationsMap[subscriptionUrl];
+ if (recommendation && recommendation.type == "ads")
{
- var element = elements[i];
- var control = element.querySelector(".control");
- if (control.localName == "input")
- control.checked = subscription.disabled == false;
- if (subscriptionUrl in recommendationsMap)
+ if (subscription.disabled == false)
{
- var recommendation = recommendationsMap[subscriptionUrl];
- if (recommendation.type == "ads")
- {
- if (subscription.disabled == false)
- {
- collections.allLangs.removeItem(subscription);
- collections.langs.addItems(subscription);
- }
- else
- {
- collections.allLangs.addItems(subscription);
- collections.langs.removeItem(subscription);
- }
- }
+ collections.allLangs.removeItem(subscription);
+ collections.langs.addItems(subscription);
+ }
+ else
+ {
+ collections.allLangs.addItems(subscription);
+ collections.langs.removeItem(subscription);
}
}
}
« no previous file with comments | « options.html ('k') | skin/options.css » ('j') | skin/options.css » ('J')

Powered by Google App Engine
This is Rietveld