882715 - Implement history sub-fragment in the new about:home

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect, P1)

Description

[Lucas Rocha (:lucasr)]

Tapping on the "History" button should open a new fragment inside homepager.

Comment 1

[Brian Nicholson (:bnicholson)]

Didn't we come to the conclusion that nested fragments are too broken when we tried making HomePager a fragment? I think the problems mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=868553#c2 and https://bugzilla.mozilla.org/show_bug.cgi?id=868553#c5 would still apply.

Comment 2

[Lucas Rocha (:lucasr)]

(In reply to Brian Nicholson (:bnicholson) from comment #1)
> Didn't we come to the conclusion that nested fragments are too broken when
> we tried making HomePager a fragment? I think the problems mentioned in
> https://bugzilla.mozilla.org/show_bug.cgi?id=868553#c2 and
> https://bugzilla.mozilla.org/show_bug.cgi?id=868553#c5 would still apply.

Sorry, my use of "sub-fragment" was a bit misleading. My rough plan is to turn homepager into a viewgroup that contains the current viewpager and add these "sub-fragments" on top of the viewpager. So, this is not really about nesting fragments.

Comment 3

[Lucas Rocha (:lucasr)]

Attachment: Define a background_light pre-defined color

Comment 4

[Lucas Rocha (:lucasr)]

Pushed: http://hg.mozilla.org/projects/fig/rev/4651e3ef81a9

Remaining patches coming soon.

Comment 5

[Lucas Rocha (:lucasr)]

Attachment: Implement history page in new about:home

Comment 6

[Lucas Rocha (:lucasr)]

FYI: this patch only contains the implementation of the history page fragment which is fairly self-contained. The patch to actually show it in the UI is coming soon.

Comment 7

[Lucas Rocha (:lucasr)]

Attachment: Here is how it looks

Comment 8

[Sriram Ramasubramanian [:sriram]]

Drive-by: Please use CursorAdapter instead of SimpleCursorAdapter. SimpleCursorAdapter is a simple implementation of CusorAdapter, where it lets us map a cursor column with a text resource. We have complex rows where we need to bind more values (like favicon, url, bookmark icon). The SimpleCursorAdapter's work is unused in this case. It's better to use CursorAdapter (it has all the convertView optimizations in its getView()).

Comment 9

[Lucas Rocha (:lucasr)]

Attachment: (1/3) Allow fragments to handle back button

Comment 10

[Lucas Rocha (:lucasr)]

Attachment: (2/3) Pack home pager in a frame layout

Comment 11

[Lucas Rocha (:lucasr)]

Attachment: (3/3) Implement history/last tabs buttons in VisitedPage

Comment 12

[Lucas Rocha (:lucasr)]

Comment on attachment 767884 [details] [diff] [review]
(3/3) Implement history/last tabs buttons in VisitedPage

This is what glues the history and last tabs fragments into the new UI. Requesting sriram's feedback for the UI/style bits.

Comment 13

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 767884 [details] [diff] [review]
(3/3) Implement history/last tabs buttons in VisitedPage

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

f- for the style changes.

::: mobile/android/base/resources/values/styles.xml
@@ +493,5 @@
> +        <item name="android:background">@drawable/action_bar_button</item>
> +        <item name="android:focusable">true</item>
> +        <item name="android:gravity">center|left</item>
> +        <item name="android:paddingLeft">10dip</item>
> +        <item name="android:paddingRight">10dip</item>

I somehow like splitting the TextAppearance from the layout stuff. Let the TextAppearances go under "TextAppearance.AboutHome.PageButton" or something. And that will be referred here. That's how android does and its a bit clean. (Same for EmptyMessage too).

Comment 14

[Brian Nicholson (:bnicholson)]

Comment on attachment 767723 [details] [diff] [review]
Implement history page in new about:home

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

Nice, looks good.

::: mobile/android/base/home/HistoryPage.java
@@ +17,5 @@
> +import android.content.res.Resources;
> +import android.database.Cursor;
> +import android.os.Bundle;
> +import android.support.v4.app.Fragment;
> +import android.support.v4.app.LoaderManager;

Last two imports are unused

Comment 15

[Brian Nicholson (:bnicholson)]

Comment on attachment 767884 [details] [diff] [review]
(3/3) Implement history/last tabs buttons in VisitedPage

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

::: mobile/android/base/resources/layout/home_visited_page.xml
@@ +21,5 @@
> +            android:layout_height="fill_parent"
> +            android:layout_weight="1"/>
> +
> +    <LinearLayout android:id="@+id/buttons_container"
> +    	            android:layout_width="fill_parent"

Tab alert

Comment 16

[Lucas Rocha (:lucasr)]

(In reply to Brian Nicholson (:bnicholson) from comment #14)
> Comment on attachment 767723 [details] [diff] [review]
> Implement history page in new about:home
> 
> Review of attachment 767723 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nice, looks good.
> 
> ::: mobile/android/base/home/HistoryPage.java
> @@ +17,5 @@
> > +import android.content.res.Resources;
> > +import android.database.Cursor;
> > +import android.os.Bundle;
> > +import android.support.v4.app.Fragment;
> > +import android.support.v4.app.LoaderManager;
> 
> Last two imports are unused

Fixed.

Comment 17

[Lucas Rocha (:lucasr)]

(In reply to Brian Nicholson (:bnicholson) from comment #15)
> Comment on attachment 767884 [details] [diff] [review]
> (3/3) Implement history/last tabs buttons in VisitedPage
> 
> Review of attachment 767884 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/resources/layout/home_visited_page.xml
> @@ +21,5 @@
> > +            android:layout_height="fill_parent"
> > +            android:layout_weight="1"/>
> > +
> > +    <LinearLayout android:id="@+id/buttons_container"
> > +    	            android:layout_width="fill_parent"
> 
> Tab alert

Fixed :-)

Comment 18

[Lucas Rocha (:lucasr)]

Pushed:
http://hg.mozilla.org/projects/fig/rev/af3a72f7b78e
http://hg.mozilla.org/projects/fig/rev/81c3af05d10a
http://hg.mozilla.org/projects/fig/rev/6c702f748e20
http://hg.mozilla.org/projects/fig/rev/d239a58564cb

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4651e3ef81a9
https://hg.mozilla.org/mozilla-central/rev/af3a72f7b78e
https://hg.mozilla.org/mozilla-central/rev/81c3af05d10a
https://hg.mozilla.org/mozilla-central/rev/6c702f748e20
https://hg.mozilla.org/mozilla-central/rev/d239a58564cb