Closed Bug 887269 Opened 8 years ago Closed 8 years ago

Restore "Open all previous tabs" feature in new about:home

Categories

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

All
Android
defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: lucasr, Assigned: lucasr)

References

Details

(Whiteboard: fixed-fig, abouthome-hackathon)

Attachments

(2 files)

The initial plan is to add a button at the bottom of the "tabs from last time" page using the same style than the ones in the "visited" page (history, tabs from last time buttons).
Priority: -- → P1
Attachment #774050 - Flags: review?(sriram)
Comment on attachment 774048 [details] [diff] [review]
(1/2) Split last tabs/history layouts into separate files

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

Confused over overdraw parts. Holding review until doubts are clarified.

::: mobile/android/base/resources/layout/home_history_page.xml
@@ +6,5 @@
> +<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
> +              android:layout_width="fill_parent"
> +              android:layout_height="fill_parent"
> +              android:orientation="vertical"
> +              android:background="@android:color/white">

HomePager has a white background. This feels like an overdraw.

::: mobile/android/base/resources/layout/home_last_tabs_page.xml
@@ +6,5 @@
> +<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
> +              android:layout_width="fill_parent"
> +              android:layout_height="fill_parent"
> +              android:orientation="vertical"
> +              android:background="@android:color/white">

Overdraw?

::: mobile/android/base/resources/layout/home_list_with_title.xml
@@ +10,5 @@
>  
>      <org.mozilla.gecko.home.HomeListView
>              android:id="@+id/list"
>              android:layout_width="fill_parent"
> +            android:layout_height="fill_parent"

android:layout_height should be 0dp, when a weight is specified. That's the android's recommended approach.
Comment on attachment 774050 [details] [diff] [review]
(2/2) Add "Open all previous tabs" button

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

Everything looks good. Holding the review to see a mockup to determine the need for an extra layout.

::: mobile/android/base/resources/layout/home_last_tabs_page.xml
@@ +20,5 @@
> +                android:text="@string/abouthome_last_tabs_open"
> +                android:gravity="center"/>
> +
> +    </LinearLayout>
> +

This feels like an extra layout. Is there a mockup I can see?
Screenshot is here: https://dl.dropboxusercontent.com/u/1187037/Screenshot_2013-07-11-17-41-09.png

The reason for the extra layout is to allow us to reuse @drawable/action_bar_button (which is transparent on normal state) as the background of the button without any changes. There are also many other possible variations, of course.
Whiteboard: good-first-bug-fig → abouthome-hackathon
Ping?
Assignee: nobody → lucasr.at.mozilla
Comment on attachment 774050 [details] [diff] [review]
(2/2) Add "Open all previous tabs" button

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

Everything looks good.

::: mobile/android/base/resources/layout/home_last_tabs_page.xml
@@ +20,5 @@
> +                android:text="@string/abouthome_last_tabs_open"
> +                android:gravity="center"/>
> +
> +    </LinearLayout>
> +

This feels like an extra layout. Is there a mockup I can see?
Attachment #774050 - Flags: review?(sriram) → review+
Comment on attachment 774048 [details] [diff] [review]
(1/2) Split last tabs/history layouts into separate files

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

I guess the overdraw is unavoidable in this case, as the ViewPager is not detached, but the HistoryPage is added to it (and shown on top of it).
I would recommended coming up with a different approach for the history fragment. r+ for now.
And please change the layout_height to 0dp as recommended by Android. r+ with that.

::: mobile/android/base/resources/layout/home_history_page.xml
@@ +6,5 @@
> +<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
> +              android:layout_width="fill_parent"
> +              android:layout_height="fill_parent"
> +              android:orientation="vertical"
> +              android:background="@android:color/white">

HomePager has a white background. This feels like an overdraw.

::: mobile/android/base/resources/layout/home_last_tabs_page.xml
@@ +6,5 @@
> +<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
> +              android:layout_width="fill_parent"
> +              android:layout_height="fill_parent"
> +              android:orientation="vertical"
> +              android:background="@android:color/white">

Overdraw?

::: mobile/android/base/resources/layout/home_list_with_title.xml
@@ +10,5 @@
>  
>      <org.mozilla.gecko.home.HomeListView
>              android:id="@+id/list"
>              android:layout_width="fill_parent"
> +            android:layout_height="fill_parent"

android:layout_height should be 0dp, when a weight is specified. That's the android's recommended approach.
Attachment #774048 - Flags: review?(sriram) → review+
(In reply to Sriram Ramasubramanian [:sriram] from comment #8)
> ::: mobile/android/base/resources/layout/home_list_with_title.xml
> @@ +10,5 @@
> >  
> >      <org.mozilla.gecko.home.HomeListView
> >              android:id="@+id/list"
> >              android:layout_width="fill_parent"
> > +            android:layout_height="fill_parent"
> 
> android:layout_height should be 0dp, when a weight is specified. That's the
> android's recommended approach.

Done.
Pushed:
http://hg.mozilla.org/projects/fig/rev/b46dc80ff462
http://hg.mozilla.org/projects/fig/rev/ff3604761a5f
Whiteboard: abouthome-hackathon → fixed-fig, abouthome-hackathon
https://hg.mozilla.org/mozilla-central/rev/b46dc80ff462
https://hg.mozilla.org/mozilla-central/rev/ff3604761a5f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.