Implement history sub-fragment in the new about:home

RESOLVED FIXED in Firefox 26

Status

()

defect
P1
normal
RESOLVED FIXED
6 years ago
6 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

6 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

6 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

6 years ago
Attachment #766755 - Flags: review?(sriram)
Attachment #766755 - Flags: review?(sriram) → review+
(Assignee)

Comment 4

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

Remaining patches coming soon.
(Assignee)

Comment 5

6 years ago
Attachment #767723 - Flags: review?(bnicholson)
(Assignee)

Comment 6

6 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

6 years ago
Posted image 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

6 years ago
Attachment #767882 - Flags: review?(bnicholson)
(Assignee)

Comment 10

6 years ago
Attachment #767883 - Flags: review?(bnicholson)
(Assignee)

Updated

6 years ago
Blocks: 882716
(Assignee)

Comment 12

6 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+
Priority: -- → P1
(Assignee)

Comment 16

6 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

6 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 :-)

Updated

6 years ago
Depends on: 891485
(Assignee)

Updated

6 years ago
Blocks: 891883
You need to log in before you can comment on or make changes to this bug.