973045 - Provide a tabbox input type

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Wesley Johnston (:wesj)]

For the new quickshare designs, we also need a way to show a tab-box in a prompt

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch

There's a little bit of spacing cleanup in here as well.

Comment 2

[Wesley Johnston (:wesj)]

Attachment: Patch

Grr. Forgot a file.

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch

Comment 4

[Wesley Johnston (:wesj)]

Attachment: Patch

Cleanup from the new adapter code.

Comment 5

[Brian Nicholson (:bnicholson)]

Comment on attachment 8378856 [details] [diff] [review]
Patch

Review of attachment 8378856 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/prompts/TabInput.java
@@ +28,5 @@
> +
> +public class TabInput extends PromptInput implements AdapterView.OnItemClickListener {
> +    public static final String INPUT_TYPE = "tabs";
> +    public static final String LOGTAG = "GeckoTabInput";
> +    private HashMap<String, PromptListItem[]> mTabs;

Nit: final

@@ +44,5 @@
> +                JSONArray items = tab.getJSONArray("items");
> +                mTabs.put(title, PromptListItem.getArray(items));
> +            }
> +        } catch(JSONException ex) {
> +            Log.i(LOGTAG, "Exception", ex);

Nit: Log.e

@@ +56,5 @@
> +        mHost.setup();
> +
> +        for (String title : mTabs.keySet()) {
> +            final TabHost.TabSpec spec = mHost.newTabSpec(title);
> +            Log.i(LOGTAG, "Add tab for " + title);

Drop this

@@ +58,5 @@
> +        for (String title : mTabs.keySet()) {
> +            final TabHost.TabSpec spec = mHost.newTabSpec(title);
> +            Log.i(LOGTAG, "Add tab for " + title);
> +            spec.setContent(new TabHost.TabContentFactory() {
> +                public View createTabContent(final String tag) {

Nit: @Override

@@ +59,5 @@
> +            final TabHost.TabSpec spec = mHost.newTabSpec(title);
> +            Log.i(LOGTAG, "Add tab for " + title);
> +            spec.setContent(new TabHost.TabContentFactory() {
> +                public View createTabContent(final String tag) {
> +                    Log.i(LOGTAG, "Creating list view " + tag);

and this

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Patch

There isn't much different here, but I did realize that I have to use a LinkedHashMap so that the HashMap and the JSON will remain in sync (so that if we return "tab 1 was selected" that will match what JS sent us). Alternatively, we could send up String keys for tabs from JS, but this approach was a bit simpler for now.

Comment 7

[Brian Nicholson (:bnicholson)]

Comment on attachment 8379986 [details] [diff] [review]
Patch

Review of attachment 8379986 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/prompts/TabInput.java
@@ +96,5 @@
> +    }
> +
> +    @Override
> +    public boolean canApplyInputStyle() {
> +	   return false;

Indentation

::: mobile/android/base/resources/layout/tab_prompt_input.xml
@@ +25,5 @@
> +            android:layout_width="match_parent"
> +            android:layout_height="match_parent" >
> +        </FrameLayout>
> +    </LinearLayout>
> +</TabHost>

Nit: newlines between each of these 3 closing tags

Comment 8

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/fx-team/rev/01fe5f5afa0f

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/01fe5f5afa0f

Comment 10

[Teodora Vermesan (:TeoVermesan)]

When choosing to share a link/image via context menu a tab-box appears, so:
Verified fixed on:
Device: LG Nexus 4 (Android 4.4.2)
Builds: Firefox for Android 32.0a1 Nightly 05/26, 31.0a2 Aurora 05/26 and 30 Beta 6