Closed Bug 973045 Opened 6 years ago Closed 6 years ago

Provide a tabbox input type

Categories

(Firefox for Android :: General, defect)

x86_64
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 30

People

(Reporter: wesj, Unassigned)

References

Details

Attachments

(1 file, 4 obsolete files)

For the new quickshare designs, we also need a way to show a tab-box in a prompt
Attached patch Patch (obsolete) — Splinter Review
There's a little bit of spacing cleanup in here as well.
Attachment #8376471 - Flags: review?(bnicholson)
Blocks: 942270
Attached patch Patch (obsolete) — Splinter Review
Grr. Forgot a file.
Attachment #8376471 - Attachment is obsolete: true
Attachment #8376471 - Flags: review?(bnicholson)
Attachment #8376530 - Flags: review?(bnicholson)
Attached patch Patch (obsolete) — Splinter Review
Attachment #8376530 - Attachment is obsolete: true
Attachment #8376530 - Flags: review?(bnicholson)
Attachment #8376542 - Flags: review?(bnicholson)
Attached patch Patch (obsolete) — Splinter Review
Cleanup from the new adapter code.
Attachment #8376542 - Attachment is obsolete: true
Attachment #8376542 - Flags: review?(bnicholson)
Attachment #8378856 - Flags: review?(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
Attachment #8378856 - Flags: review?(bnicholson) → review+
Attached patch PatchSplinter Review
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.
Attachment #8378856 - Attachment is obsolete: true
Attachment #8379986 - Flags: review?(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
Attachment #8379986 - Flags: review?(bnicholson) → review+
https://hg.mozilla.org/mozilla-central/rev/01fe5f5afa0f
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.