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.

Sign up for GitHub

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

Combobox keyboard navigation. Fixes #362. #497

Closed
wants to merge 5 commits into from
Closed

Combobox keyboard navigation. Fixes #362. #497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
356b460
Combobox: keyboard navigation (code, test, doc). Fixes #362
Jan 28, 2015
b99e321
Combobox.js: add workaround for delite #373
Jan 28, 2015
485ba54
Combobox: add missing @returns and remove useless remote.end() - @wke…
Jan 29, 2015
c70321a
Various improvements and fixes, mostly per feedback from wkeese.
Jan 29, 2015
c93c2cd
Further fixes per @wkeese feedback
Jan 30, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
202 changes: 150 additions & 52 deletions Combobox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,38 +375,90 @@ define([

this.dropDown = dropDown; // delite/HasDropDown's property

/* TODO: keyboard navigation support will come later.
this.list.on("keynav-child-navigated", function(evt) {
var input = this._popupInput || this.inputNode;
if (evt.newValue) {
this.list.selectFromEvent(evt, evt.newValue, evt.newValue, true);
input.setAttribute("aria-activedescendant", evt.newValue.id);
// Focus stays on the input element
this.dropDown.focusOnOpen = false;

// (temporary?) Workaround for delite #373
this.dropDown.focus = null;

this._initHandlers();
this._initValue();
},

/**
* Handles both keyboard navigation and mouse interaction.
* @param {Node} The DOM node targeted by the navigation.
* @param {boolean} keyboard true if interaction triggered by key event, false otherwise.
* @private
*/
_clickOrKeyNavHandler: function (target, keyboard) {
var input = this._popupInput || this.inputNode;
var rend = target ? this.list.getEnclosingRenderer(target) : null;
if (this.selectionMode === "single") {
if (rend) {
if (keyboard) { // "keynav-child-navigated" event triggerred by key event
if (!this.list.isSelected(rend.item)) {
this.list.setSelected(rend.item, true);
this._updateScroll(rend.item, true);
}
} else { // mouse interaction ("click" event)
this.defer(function () {
// deferred such that the user can see the selection feedback
// before the dropdown closes.
this.closeDropDown(true/*refocus*/);
}.bind(this), 100); // worth exposing a property for the delay?
}
input.setAttribute("aria-activedescendant", target.id);
} else {
input.removeAttribute("aria-activedescendant");
}
}.bind(this));
*/
} else if (this.selectionMode === "multiple") {
if (rend) {
if (keyboard) {
this._updateScroll(rend.item);
}
input.setAttribute("aria-activedescendant", target.id);
} else {
input.removeAttribute("aria-activedescendant");
}
}
},

/**
* Initializes event handlers for item navigation.
* @private
*/
_initHandlers: function () {
if (this._initHandlersDone) {
return; // set handlers only once
}
this._initHandlersDone = true;

this._initValue();
// Keyboard navigation
this.list.on("keynav-child-navigated", function (evt) {
if (!(evt.triggerEvent &&
(evt.triggerEvent.type === "keydown" || evt.triggerEvent.type === "keypress"))) {
return; // navigation not triggered by keyboard events
}
this._clickOrKeyNavHandler(evt.newValue, true);
}.bind(this));

// Mouse navigation
this.list.on("click", function (evt) {
this._clickOrKeyNavHandler(evt.target, false);
}.bind(this));

// React to programmatic changes of selected items
this.list.observe(function (oldValues) {
if (this.selectionMode === "single" && "selectedItem" in oldValues) {
var selectedItem = this.list.selectedItem;
// selectedItem non-null because List in radio selection mode, but
// the List can be empty, so:
this.inputNode.value = selectedItem ? this._getItemLabel(selectedItem) : "";
this.value = selectedItem ? this._getItemValue(selectedItem) : "";
this.handleOnInput(this.value); // emit "input" event
this.defer(function () {
// deferred such that the user can see the selection feedback
// before the dropdown closes.
this.closeDropDown(true/*refocus*/);
}.bind(this), 100); // worth exposing a property for the delay?
} else if (this.selectionMode === "multiple" && "selectedItems" in oldValues) {
// if _useCenteredDropDown() is true, let the dropdown's OK/Cancel
// buttons do the job
if (!this._useCenteredDropDown()) {
if ("selectedItems" in oldValues) {
if (this.selectionMode === "single") {
var selectedItem = this.list.selectedItem;
// selectedItem non-null because List in radio selection mode, but
// the List can be empty, so:
this.inputNode.value = selectedItem ? this._getItemLabel(selectedItem) : "";
this.value = selectedItem ? this._getItemValue(selectedItem) : "";
this.handleOnInput(this.value); // emit "input" event
} else if (this.selectionMode === "multiple") {
this._validateMultiple(this._popupInput || this.inputNode);
}
}
Expand Down Expand Up @@ -563,11 +615,39 @@ define([
evt.preventDefault();
}.bind(this), inputElement);
this.on("keydown", function (evt) {
/* jshint maxcomplexity: 15 */
// deliteful issue #382: prevent the browser from navigating to
// the previous page when typing backspace in a readonly input
if (inputElement.readOnly && evt.keyCode === keys.BACKSPACE) {
evt.stopPropagation();
evt.preventDefault();
} else if (evt.keyCode === keys.ENTER) {
evt.stopPropagation();
evt.preventDefault();
if (this.opened) {
this.closeDropDown(true/*refocus*/);
}
} else if (evt.keyCode === keys.SPACE) {
// Simply forwarding the key event to List doesn't allow toggling
// the selection, because List's mechanism is based on the event target
// which here is the input element outside the List. TODO: see deliteful #500.
if (this.selectionMode === "multiple") {
var rend = this.list.getEnclosingRenderer(this.list.navigatedDescendant);
var item = rend.item;
this.list.setSelected(item, !this.list.isSelected(item));
}
if (this.selectionMode === "multiple" || !this.autoFilter) {
evt.stopPropagation();
evt.preventDefault();
}
} else if (evt.keyCode === keys.DOWN_ARROW || evt.keyCode === keys.UP_ARROW ||
evt.keyCode === keys.PAGE_DOWN || evt.keyCode === keys.PAGE_UP ||
evt.keyCode === keys.HOME || evt.keyCode === keys.END) {
if (this._useCenteredDropDown()) {
this.list.emit("keydown", evt);
}
evt.stopPropagation();
evt.preventDefault();
}
}.bind(this), inputElement);
},
Expand Down Expand Up @@ -619,36 +699,23 @@ define([

openDropDown: dcl.superCall(function (sup) {
return function () {
var selectedItems = this.list.selectedItems;
// Store the value, to be able to restore on cancel. (Could spare
// it in situations when there is no cancel button, though.)
this._selectedItems = selectedItems;

// Temporary workaround for issue with bad pairing in List of the
// busy on/off state. The issue appears to go away if List.attachedCallback
// wouldn't break the automatic chaining (hence the workaround wouldn't
// be necessary if List gets this change), but this requires further
// investigation (TODO).
this.defer(function () {
this.list._hideLoadingPanel();
}.bind(this), 300);
if (!this.opened) {
// Temporary workaround for issue with bad pairing in List of the
// busy on/off state. The issue appears to go away if List.attachedCallback
// wouldn't break the automatic chaining (hence the workaround wouldn't
// be necessary if List gets this change), but this requires further
// investigation (TODO).
this.defer(function () {
this.list._hideLoadingPanel();
// Avoid loosing focus when clicking the arrow (instead of the input element):
this.focusNode.focus();
}.bind(this), 300);
}

var promise = sup.apply(this, arguments);

return promise.then(function () {
var firstSelectedItem = selectedItems && selectedItems.length > 0 ?
selectedItems[0] : null;
if (firstSelectedItem) {
// Make the first selected item (if any) visible.
// Must be done after sup.apply, because List.getBottomDistance
// relies on dimensions which are not available if the DOM nodes
// are not (yet) visible, hence the popup needs to be shown before.
var id = this.list.getIdentity(firstSelectedItem);
var renderer = this.list.getRendererByItemId(id);
if (renderer) {
this.list.scrollBy({y: this.list.getBottomDistance(renderer)});
} // null if the list is empty because no item matches the auto-filtering
}
this._updateScroll(undefined, true);
}.bind(this));
};
}),
Expand Down Expand Up @@ -683,6 +750,37 @@ define([

sup.apply(this, arguments);
};
})
}),

/**
* Scrolls the list inside the popup such that the specified item, or
* the first selected item if no item is specified, is visible.
* @private
*/
_updateScroll: function (item, navigate) {
// Since List is in focus-less mode, it does not give focus to
// navigated items, thus the browser does not autoscroll.
// TODO: see deliteful #498

if (!item) {
var selectedItems = this.list.selectedItems;
item = selectedItems && selectedItems.length > 0 ?
selectedItems[0] : null;
}
if (item) {
// Make the first selected item (if any) visible.
// Must be done after sup.apply, because List.getBottomDistance
// relies on dimensions which are not available if the DOM nodes
// are not (yet) visible, hence the popup needs to be shown before.
var id = this.list.getIdentity(item);
var renderer = this.list.getRendererByItemId(id);
if (renderer) {
this.list.scrollBy({y: this.list.getBottomDistance(renderer)});
if (navigate) {
this.list.navigatedDescendant = renderer.childNodes[0];
}
} // null if the list is empty because no item matches the auto-filtering
}
}
});
});
16 changes: 15 additions & 1 deletion docs/Combobox.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,21 @@ The following table lists the CSS classes that can be used to style the Combobox

### Accessibility

Keyboard and screen reader accessibility will be supported in the next release.
|type|status|comment|
|----|------|-------|
|Keyboard|ok|For details, see below this table.|
|Visual Formatting|ok|Tested for high constrast and browser zoom (200%), in IE and Firefox.|
|Screen Reader|ok|Tested on JAWS 15 and iOS VoiceOver.|

Keyboard navigation details:
* DOWN arrow opens the focused combobox.
* In single selection mode:
* UP and DOWN arrows select the next, respectively the previous option.
* RETURN and ESCAPE validate the change.
* In multiple selection mode:
* UP and DOWN arrows navigate to the next, respectively the previous option.
* SPACE toggles the selected state of the currently navigated option.
* RETURN and ESCAPE validate the change.

### Globalization

Expand Down
14 changes: 12 additions & 2 deletions list/List.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,8 @@ define([
/**
* Event handler that performs items (de)selection.
* @param {Event} event The event the handler was called for.
* @returns {boolean} `true` if the event has been handled, that is if the
* event target has an enclosing item renderer. Returns `false` otherwise.
* @protected
*/
handleSelection: function (/*Event*/event) {
Expand All @@ -467,7 +469,9 @@ define([
if (!this.isCategoryRenderer(eventRenderer)) {
this.selectFromEvent(event, eventRenderer.item, eventRenderer, true);
}
return true;
}
return false;
},

//////////// Private methods ///////////////////////////////////////
Expand Down Expand Up @@ -1104,8 +1108,14 @@ define([
*/
_spaceKeydownHandler: function (evt) {
if (this.selectionMode !== "none") {
evt.preventDefault();
this.handleSelection(evt);
if (this.handleSelection(evt)) {
evt.preventDefault();
} // else do not prevent-default, for the sake of use-cases
// such as Combobox where the target of the key event is an
// input element outside the List. In this use-case, delite/HasDropDown
// forwards "keydown" events to the List instance and prevent-defaults
// the event if any key handler prevent-defaults the event, which would
// forbid the user from entering space characters in the input element.
}
},

Expand Down
Loading