Skip to content

Commit

Permalink
Bug 1266456 - part9: use HTMLTooltip for autocomplete-popup;r=bgrins
Browse files Browse the repository at this point in the history
Modify the devtools autocomplete-popup to rely on a HTMLTooltip instance
instead of a XUL panel.

Other than the straightforward migration to HTML, the main difference with
the new implementation is that the richlistbox has now been replace with a
simple HTML list element. The former XUL widget used to be able to take the
focus from the input it was linked to.

This is no longer the case. Most autocomplete users were always keeping the
focus in the input, except for the inspector-search, which was moving the
focus back and forth between the input and the autocomplete's richlistbox.
Now the focus is always in the input. A practical example to illustrate how
this changes the UX: before when the user had the focus on the first element
of the list, pressing "DOWN" would keep the element selected but visually move
the focus in the input. Now the selection simply cycles to the next item.

Even though this introduces a difference in behaviour compared to the previous
implementation, it makes the inspector search UX consistent with the other
autocomplete widgets used in devtools.

Another difference is about the display for the inspector-search. The position
of the autocomplete popup used to be above the input. This is now impossible to
achieve because the search input is at the top of the toolbox and the HTML tooltip
can not exceed the limits of the toolbox.

For this #2 issue, either we manage to use XUL panel wrappers, in which case, the
autocomplete will be displayed as it used to. Or we can invert the order in which
items are inserted and explicitly ask for the autocomplete to be displayed below the
input. I prefered not to change this here in order to make the code change easier to
understand, but it should be addressed in a follow-up.

MozReview-Commit-ID: jH9aXm9Jvz

--HG--
extra : rebase_source : 57267be0d214ed807f3152838c4123400ab7b7e3
  • Loading branch information
juliandescottes committed Jul 7, 2016
1 parent 74d5a93 commit d03c301
Show file tree
Hide file tree
Showing 11 changed files with 338 additions and 405 deletions.
1 change: 0 additions & 1 deletion devtools/client/debugger/content/views/sources-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,6 @@ SourcesView.prototype = Heritage.extend(WidgetMethods, {
// previously set, revert to using an empty string by default.
this._cbTextbox.value = expr;


function openPopup() {
// Show the conditional expression panel. The popup arrow should be pointing
// at the line number node in the breakpoint item view.
Expand Down
158 changes: 63 additions & 95 deletions devtools/client/inspector/inspector-search.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ InspectorSearch.prototype = {
let res = yield this.walker.search(query, { reverse });

// Value has changed since we started this request, we're done.
if (query != this.searchBox.value) {
if (query !== this.searchBox.value) {
return;
}

Expand Down Expand Up @@ -142,21 +142,19 @@ function SelectorAutocompleter(inspector, inputNode) {

this.showSuggestions = this.showSuggestions.bind(this);
this._onSearchKeypress = this._onSearchKeypress.bind(this);
this._onListBoxKeypress = this._onListBoxKeypress.bind(this);
this._onSearchPopupClick = this._onSearchPopupClick.bind(this);
this._onMarkupMutation = this._onMarkupMutation.bind(this);

// Options for the AutocompletePopup.
let options = {
panelId: "inspector-searchbox-panel",
listBoxId: "searchbox-panel-listbox",
listId: "searchbox-panel-listbox",
autoSelect: true,
position: "before_start",
direction: "ltr",
position: "top",
theme: "auto",
onClick: this._onListBoxKeypress,
onKeypress: this._onListBoxKeypress
onClick: this._onSearchPopupClick,
};
this.searchPopup = new AutocompletePopup(this.panelDoc, options);

this.searchPopup = new AutocompletePopup(inspector._toolbox, options);

this.searchBox.addEventListener("input", this.showSuggestions, true);
this.searchBox.addEventListener("keypress", this._onSearchKeypress, true);
Expand Down Expand Up @@ -231,11 +229,11 @@ SelectorAutocompleter.prototype = {
lastChar = secondLastChar;

case this.States.TAG: // eslint-disable-line
if (lastChar == ".") {
if (lastChar === ".") {
this._state = this.States.CLASS;
} else if (lastChar == "#") {
} else if (lastChar === "#") {
this._state = this.States.ID;
} else if (lastChar == "[") {
} else if (lastChar === "[") {
this._state = this.States.ATTRIBUTE;
} else {
this._state = this.States.TAG;
Expand All @@ -246,11 +244,11 @@ SelectorAutocompleter.prototype = {
if (subQuery.match(/[\.]+[^\.]*$/)[0].length > 2) {
// Checks whether the subQuery has atleast one [a-zA-Z] after the
// '.'.
if (lastChar == " " || lastChar == ">") {
if (lastChar === " " || lastChar === ">") {
this._state = this.States.TAG;
} else if (lastChar == "#") {
} else if (lastChar === "#") {
this._state = this.States.ID;
} else if (lastChar == "[") {
} else if (lastChar === "[") {
this._state = this.States.ATTRIBUTE;
} else {
this._state = this.States.CLASS;
Expand All @@ -262,11 +260,11 @@ SelectorAutocompleter.prototype = {
if (subQuery.match(/[#]+[^#]*$/)[0].length > 2) {
// Checks whether the subQuery has atleast one [a-zA-Z] after the
// '#'.
if (lastChar == " " || lastChar == ">") {
if (lastChar === " " || lastChar === ">") {
this._state = this.States.TAG;
} else if (lastChar == ".") {
} else if (lastChar === ".") {
this._state = this.States.CLASS;
} else if (lastChar == "[") {
} else if (lastChar === "[") {
this._state = this.States.ATTRIBUTE;
} else {
this._state = this.States.ID;
Expand All @@ -275,13 +273,13 @@ SelectorAutocompleter.prototype = {
break;

case this.States.ATTRIBUTE:
if (subQuery.match(/[\[][^\]]+[\]]/) != null) {
if (subQuery.match(/[\[][^\]]+[\]]/) !== null) {
// Checks whether the subQuery has at least one ']' after the '['.
if (lastChar == " " || lastChar == ">") {
if (lastChar === " " || lastChar === ">") {
this._state = this.States.TAG;
} else if (lastChar == ".") {
} else if (lastChar === ".") {
this._state = this.States.CLASS;
} else if (lastChar == "#") {
} else if (lastChar === "#") {
this._state = this.States.ID;
} else {
this._state = this.States.ATTRIBUTE;
Expand Down Expand Up @@ -311,20 +309,17 @@ SelectorAutocompleter.prototype = {
* Handles keypresses inside the input box.
*/
_onSearchKeypress: function (event) {
let query = this.searchBox.value;
let popup = this.searchPopup;

switch (event.keyCode) {
case event.DOM_VK_RETURN:
case event.DOM_VK_TAB:
if (popup.isOpen &&
popup.getItemAtIndex(popup.itemCount - 1)
.preLabel == query) {
popup.selectedIndex = popup.itemCount - 1;
this.searchBox.value = popup.selectedItem.label;
if (popup.isOpen) {
if (popup.selectedItem) {
this.searchBox.value = popup.selectedItem.label;
}
this.hidePopup();
} else if (!popup.isOpen &&
event.keyCode === event.DOM_VK_TAB) {
} else if (!popup.isOpen) {
// When tab is pressed with focus on searchbox and closed popup,
// do not prevent the default to avoid a keyboard trap and move focus
// to next/previous element.
Expand All @@ -335,25 +330,32 @@ SelectorAutocompleter.prototype = {

case event.DOM_VK_UP:
if (popup.isOpen && popup.itemCount > 0) {
popup.focus();
if (popup.selectedIndex == popup.itemCount - 1) {
popup.selectedIndex =
Math.max(0, popup.itemCount - 2);
} else {
if (popup.selectedIndex === 0) {
popup.selectedIndex = popup.itemCount - 1;
} else {
popup.selectedIndex--;
}
this.searchBox.value = popup.selectedItem.label;
}
break;

case event.DOM_VK_DOWN:
if (popup.isOpen && popup.itemCount > 0) {
popup.focus();
popup.selectedIndex = 0;
if (popup.selectedIndex === popup.itemCount - 1) {
popup.selectedIndex = 0;
} else {
popup.selectedIndex++;
}
this.searchBox.value = popup.selectedItem.label;
}
break;

case event.DOM_VK_ESCAPE:
if (popup.isOpen) {
this.hidePopup();
}
break;

default:
return;
}
Expand All @@ -364,59 +366,17 @@ SelectorAutocompleter.prototype = {
},

/**
* Handles keypress and mouse click on the suggestions richlistbox.
* Handles click events from the autocomplete popup.
*/
_onListBoxKeypress: function (event) {
let popup = this.searchPopup;

switch (event.keyCode || event.button) {
case event.DOM_VK_RETURN:
case event.DOM_VK_TAB:
case 0:
// left mouse button
event.stopPropagation();
event.preventDefault();
this.searchBox.value = popup.selectedItem.label;
this.searchBox.focus();
this.hidePopup();
break;

case event.DOM_VK_UP:
if (popup.selectedIndex == 0) {
popup.selectedIndex = -1;
event.stopPropagation();
event.preventDefault();
this.searchBox.focus();
} else {
let index = popup.selectedIndex;
this.searchBox.value = popup.getItemAtIndex(index - 1).label;
}
break;

case event.DOM_VK_DOWN:
if (popup.selectedIndex == popup.itemCount - 1) {
popup.selectedIndex = -1;
event.stopPropagation();
event.preventDefault();
this.searchBox.focus();
} else {
let index = popup.selectedIndex;
this.searchBox.value = popup.getItemAtIndex(index + 1).label;
}
break;

case event.DOM_VK_BACK_SPACE:
event.stopPropagation();
event.preventDefault();
this.searchBox.focus();
if (this.searchBox.selectionStart > 0) {
this.searchBox.value = this.searchBox.value.substring(0,
this.searchBox.selectionStart - 1);
}
this.hidePopup();
break;
_onSearchPopupClick: function (event) {
let selectedItem = this.searchPopup.selectedItem;
if (selectedItem) {
this.searchBox.value = selectedItem.label;
}
this.emit("processing-done");
this.hidePopup();

event.preventDefault();
event.stopPropagation();
},

/**
Expand All @@ -430,6 +390,9 @@ SelectorAutocompleter.prototype = {

/**
* Populates the suggestions list and show the suggestion popup.
*
* @return {Promise} promise that will resolve when the autocomplete popup is fully
* displayed or hidden.
*/
_showPopup: function (list, firstPart, popupState) {
let total = 0;
Expand Down Expand Up @@ -473,21 +436,24 @@ SelectorAutocompleter.prototype = {
break;
}
}

if (total > 0) {
let onPopupOpened = this.searchPopup.once("popup-opened");
this.searchPopup.setItems(items);
this.searchPopup.openPopup(this.searchBox);
} else {
this.hidePopup();
return onPopupOpened;
}

return this.hidePopup();
},

/**
* Hide the suggestion popup if necessary.
*/
hidePopup: function () {
if (this.searchPopup.isOpen) {
this.searchPopup.hidePopup();
}
let onPopupClosed = this.searchPopup.once("popup-closed");
this.searchPopup.hidePopup();
return onPopupClosed;
},

/**
Expand Down Expand Up @@ -534,7 +500,7 @@ SelectorAutocompleter.prototype = {
if (result.query !== query) {
// This means that this response is for a previous request and the user
// as since typed something extra leading to a new request.
return;
return promise.resolve(null);
}

if (state === this.States.CLASS) {
Expand All @@ -550,7 +516,9 @@ SelectorAutocompleter.prototype = {
result.suggestions = [];
}

this._showPopup(result.suggestions, firstPart, state);
// Wait for the autocomplete-popup to fire its popup-opened event, to make sure
// the autoSelect item has been selected.
return this._showPopup(result.suggestions, firstPart, state);
});

return;
Expand Down
8 changes: 2 additions & 6 deletions devtools/client/inspector/markup/markup.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const DRAG_DROP_MIN_INITIAL_DISTANCE = 10;
const DRAG_DROP_HEIGHT_TO_SPEED = 500;
const DRAG_DROP_HEIGHT_TO_SPEED_MIN = 0.5;
const DRAG_DROP_HEIGHT_TO_SPEED_MAX = 1;
const AUTOCOMPLETE_POPUP_PANEL_ID = "markupview_autoCompletePopup";
const ATTR_COLLAPSE_ENABLED_PREF = "devtools.markup.collapseAttributes";
const ATTR_COLLAPSE_LENGTH_PREF = "devtools.markup.collapseAttributeLength";
const PREVIEW_MAX_DIM_PREF = "devtools.inspector.imagePreviewTooltipSize";
Expand Down Expand Up @@ -109,12 +108,9 @@ function MarkupView(inspector, frame, controllerWindow) {
let options = {
autoSelect: true,
theme: "auto",
// panelId option prevents the markupView autocomplete popup from
// sharing XUL elements with other views, such as ruleView (see Bug 1191093)
panelId: AUTOCOMPLETE_POPUP_PANEL_ID
};
this.popup = new AutocompletePopup(this.doc.defaultView.parent.document,
options);

this.popup = new AutocompletePopup(inspector._toolbox, options);

this.undo = new UndoStack();
this.undo.installController(controllerWindow);
Expand Down
2 changes: 1 addition & 1 deletion devtools/client/inspector/rules/rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ function CssRuleView(inspector, document, store, pageStyle) {
autoSelect: true,
theme: "auto"
};
this.popup = new AutocompletePopup(this.styleDocument, options);
this.popup = new AutocompletePopup(inspector._toolbox, options);

this._showEmpty();

Expand Down
Loading

0 comments on commit d03c301

Please sign in to comment.