592904 - Hide the Form Assistant select UI filter box for small select lists

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

If a select only has a few items, or all the items are visible on screen, we do not need the filter UI. It is just confusing for small lists.

For large lists, it is still confusing for users who don't know what it does. We should add a simple placeholder hint: "filter list" or "search list"

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Another idea is to collapse the Select UI when the user picks an item from a single selection combobox. The FormHelper bar should still be present so the forward/back buttons can be used.

Comment 2

[Wesley Johnston (:wesj)]

Attachment: Patch - Tweak filterbar

This addresses things in the first comment. Only shows the filter bar if we overflow (note that doesn't necessarily mean the list is long, it one element slightly overflow it will be shown), and adds some placeholder text reading "Filter List".

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 487031 [details] [diff] [review]
Patch - Tweak filterbar

>diff --git a/chrome/content/browser-ui.js b/chrome/content/browser-ui.js

> var SelectHelperUI = {
>   _list: null,
>   _selectedIndexes: null,
>-
>+  _initialized : false,
>+  

Drop the _initialized and just call SelectHelperUI.init() here:
http://mxr.mozilla.org/mobile-browser/source/chrome/content/browser-ui.js#504

>+  init: function() {
>+    if (this._initialized)
>+      return;

Remove the if check too

>+
>+    this._initialized = true;
>+    this._panel.addEventListener("overflow", this, true);

Do we need this attached all the time? I think we add / remove event listeners in show() / hide(). Can you do the same?

>-    this._panel.height = this._panel.getBoundingClientRect().height;
>+    this.showFilter = this.showFilter;

Kinda weird to see this. Add a comment about why we are cranking the setter here. Or reorganize the code to make this easier to understand.

>+    let height = this._panel.getBoundingClientRect().height;
>+    this._panel.height = height;

Why make the local here?

>+          <textbox id="select-helper-textbox" class="search-bar content-navigator-item" oncommand="SelectHelperUI.filter(this.value)"
>+                   placeholder="&select-helper.emptytext;" type="search" flex="1" hidden="true"/>

I don't see select-helper.emptytext in a DTD. And can we name it selectHelper.emptytext

r- for some small fixes

Comment 4

[Wesley Johnston (:wesj)]

Attachment: Patch v2

Nits addressed

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mobile-browser/rev/8b3cd5a5cfcc

The filter box is hidden if the contents of the list don't overflow the list (need to be panned)