Implement history sub-fragment in the new about:home

RESOLVED FIXED in Firefox 26

Status

()

Firefox for Android
Awesomescreen
P1
normal
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: lucasr, Assigned: lucasr)

Tracking

unspecified
Firefox 26
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-fig)

Attachments

(6 attachments)

(Assignee)

Description

5 years ago
Tapping on the "History" button should open a new fragment inside homepager.
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.
(Assignee)

Comment 2

5 years ago
(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.
(Assignee)

Comment 3

5 years ago
Created attachment 766755 [details] [diff] [review]
Define a background_light pre-defined color
Attachment #766755 - Flags: review?(sriram)
Attachment #766755 - Flags: review?(sriram) → review+
(Assignee)

Comment 4

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

Remaining patches coming soon.
(Assignee)

Comment 5

5 years ago
Created attachment 767723 [details] [diff] [review]
Implement history page in new about:home
Attachment #767723 - Flags: review?(bnicholson)
(Assignee)

Comment 6

5 years ago
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.
(Assignee)

Comment 7

5 years ago
Created attachment 767726 [details]
Here is how it looks
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()).
(Assignee)

Comment 9

5 years ago
Created attachment 767882 [details] [diff] [review]
(1/3) Allow fragments to handle back button
Attachment #767882 - Flags: review?(bnicholson)
(Assignee)

Comment 10

5 years ago
Created attachment 767883 [details] [diff] [review]
(2/3) Pack home pager in a frame layout
Attachment #767883 - Flags: review?(bnicholson)
(Assignee)

Comment 11

5 years ago
Created attachment 767884 [details] [diff] [review]
(3/3) Implement history/last tabs buttons in VisitedPage
Attachment #767884 - Flags: review?(bnicholson)
(Assignee)

Updated

5 years ago
Blocks: 882716
(Assignee)

Comment 12

5 years ago
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.
Attachment #767884 - Flags: feedback?(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).
Attachment #767884 - Flags: feedback?(sriram) → feedback-
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
Attachment #767723 - Flags: review?(bnicholson) → review+
Attachment #767882 - Flags: review?(bnicholson) → review+
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
Attachment #767884 - Flags: review?(bnicholson) → review+
Attachment #767883 - Flags: review?(bnicholson) → review+

Updated

5 years ago
Priority: -- → P1
(Assignee)

Comment 16

5 years ago
(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.
(Assignee)

Comment 17

5 years ago
(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 :-)
(Assignee)

Comment 18

5 years ago
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
Whiteboard: fixed-fig

Updated

5 years ago
Depends on: 891485
(Assignee)

Updated

5 years ago
Blocks: 891883
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
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
You need to log in before you can comment on or make changes to this bug.