Note: There are a few cases of duplicates in user autocompletion which are being worked on.

[TABLET] Tabs Menu

VERIFIED FIXED in Firefox 15

Status

()

Firefox for Android
General
VERIFIED FIXED
5 years ago
a year ago

People

(Reporter: ibarlow, Assigned: sriram)

Tracking

(Depends on: 2 bugs)

unspecified
Firefox 16
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox15 verified, firefox16 verified, firefox17 verified, blocking-fennec1.0 -, fennec14+)

Details

(Whiteboard: [tablet])

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Created attachment 609478 [details]
Mockup

Let's build on our existing tabs menu and make it even better.

Users should be able to 
* Open / close menu with a swipe gesture
* Create a new tab
* Close tabs
* Undo closed tabs

A mockup is attached, and more detailed specs and assets will follow soon.
-ing to keep triage lists clean
blocking-fennec1.0: --- → -
tracking-fennec: --- → 14+
Slick! Does the darker background denote the current tab or the focus state? There should be a focus state indicator, also for the close button. Same with the phone version.
Just a quick clarification of what I mean by "focus": It is when the user uses a keyboard or directional controller to navigate through on screen controls, like pressing tab on a desktop.

Updated

5 years ago
Depends on: 757198

Updated

5 years ago
Depends on: 758317

Updated

5 years ago
Depends on: 758319
(Assignee)

Comment 4

5 years ago
Created attachment 629690 [details] [diff] [review]
Patch

This patch adds new layout files (gecko_app.xml) for Tablet UI. 
The Tabs-Tray and Remote-Tabs are made as LinearLayout instead of an Activity.
The TabsPanel takes care of displaying the required UI. TabsPanel is a private variable in GeckoApp.
This doesn't change the Tabs-button yet.
Assignee: nobody → sriram
Attachment #629690 - Flags: review?(mark.finkle)
(Assignee)

Comment 5

5 years ago
Created attachment 629692 [details] [diff] [review]
Patch

It's been a long time since the misnomer was rectified.
In the holy name of Fennec, I (would like to) hereby pronounce that TabsTray be called as "LocalTabs". ;)
Attachment #629692 - Flags: review?(mark.finkle)
(Assignee)

Comment 6

5 years ago
At this point, this requires three more patches:
1. Adding an expandable tabs-button, moving the "+" and "synced tabs" to it.
2. Adding the cleanup from margaret's patch
3. Changing the layout for tab thumnails and titles in the list.

In this, #3 is just addition of layout files, and hence won't be a big problem if we have to uplift it.
(Assignee)

Comment 7

5 years ago
Comment on attachment 629692 [details] [diff] [review]
Patch

This requires more changes, in styles, themes, filenames for drawables -- to be consistent. Hence I dont feel like having this change now.
Attachment #629692 - Attachment is obsolete: true
Attachment #629692 - Flags: review?(mark.finkle)
(Assignee)

Comment 8

5 years ago
Created attachment 631209 [details] [diff] [review]
Patch (1/2): Layout change

* This patch changes the layout of the tabs-tray and remote-tabs to be a LinearLayout instead of an Activity.
* Newer files are added for tablets
* The tabs button won't work until gecko is ready.
  - Earlier, we opened the AwesomeBar. Now, we show the list, even if there is just one tab.
* This also kills the ActionBar.
  - I spent time in fixing the regression, to layout the actionbar on rotation. It didn't work. (Filing a followup).

Further followups:
1. The thumbnails are of different sizes in tablets and phones as per specification. This would potentially affect the about:home also.
2. The tabs-button should get rid of black portion. -- Need resources.
3. The sync-button are re-using existing resources. -- Need resources.
4. Orange! All tab and tab-button highlights should use Orange!! -- :sigh:
Attachment #629690 - Attachment is obsolete: true
Attachment #629690 - Flags: review?(mark.finkle)
Attachment #631209 - Flags: review?(mark.finkle)
(Assignee)

Comment 9

5 years ago
Created attachment 631210 [details] [diff] [review]
Patch (2/2): Animation

This patch adds a nice little sliding animation to the tabs.
It feels a bit rough because Gecko is unable to resize the viewport at 10ms.
Attachment #631210 - Flags: review?(mark.finkle)
(Assignee)

Updated

5 years ago
Depends on: 762722
(Assignee)

Updated

5 years ago
Depends on: 762723
(Assignee)

Updated

5 years ago
Depends on: 762724
(Assignee)

Updated

5 years ago
Depends on: 762725
(Assignee)

Updated

5 years ago
Depends on: 762727
Comment on attachment 631209 [details] [diff] [review]
Patch (1/2): Layout change

This is a huge patch, but, thankfully, it's not adding a lot of new functionality. The patch does add some behavior that might be tweaked or changed later. We really need to try it out first to see how it feels. The core changes seem sane.

Before landing this, I want you to make sure that basic browser functionality does not lead to crashes or severely broken functionality. I am OK with some rough edges, but not a busted browser. Test on phones and tablets.

>diff --git a/mobile/android/base/resources/values/styles.xml b/mobile/android/base/resources/values/styles.xml

>     <style name="FindBar">
>-        <item name="android:layout_width">fill_parent</item>
>-        <item name="android:layout_height">wrap_content</item>

Why drop these. I see you have fill_parent and wrap_content in the layout xml files anyway. Wouldn't it be cleaner to leave this in place?

>-        <item name="android:orientation">horizontal</item>

I assume this is the default, so it's redundant

r+, and make sure followup bugs for animation problems and new resources are filed and link back to this bug so we can track the followups.
Attachment #631209 - Flags: review?(mark.finkle) → review+
This probably needs a clobber so use the clobberer webpage.
Comment on attachment 631210 [details] [diff] [review]
Patch (2/2): Animation

The TabsLayoutChangeListener is a nice way to keep the code decoupled. I suppose I can let it go for now that the TabsPanel can only have a single TabsLayoutChangeListener. We can add support when we need it.

I have a contempt for all types of animations and this is no different. Slow phones and tablets will be really rough. We definitely need support for suspending the Gecko rendering and maybe a non-linear property interpolator, so we have less animation steps overall. Filed bugs for these issues.

I think we need this landed so we can move it forward. It definitely can't be uplifted until we fix some of the follow up bugs first.
Attachment #631210 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 13

5 years ago
(In reply to Mark Finkle (:mfinkle) from comment #10)
> Comment on attachment 631209 [details] [diff] [review]
> Patch (1/2): Layout change
> 
> This is a huge patch, but, thankfully, it's not adding a lot of new
> functionality. The patch does add some behavior that might be tweaked or
> changed later. We really need to try it out first to see how it feels. The
> core changes seem sane.
> 
> Before landing this, I want you to make sure that basic browser
> functionality does not lead to crashes or severely broken functionality. I
> am OK with some rough edges, but not a busted browser. Test on phones and
> tablets.
> 
> >diff --git a/mobile/android/base/resources/values/styles.xml b/mobile/android/base/resources/values/styles.xml
> 
> >     <style name="FindBar">
> >-        <item name="android:layout_width">fill_parent</item>
> >-        <item name="android:layout_height">wrap_content</item>
> 
> Why drop these. I see you have fill_parent and wrap_content in the layout
> xml files anyway. Wouldn't it be cleaner to leave this in place?
> 
> >-        <item name="android:orientation">horizontal</item>
> 
> I assume this is the default, so it's redundant
> 
> r+, and make sure followup bugs for animation problems and new resources are
> filed and link back to this bug so we can track the followups.

I have moved the Find-In-Page to be inside the RelativeLayout. While moving, I found every other layout there have the layout-params specified there. This alone was referring to a style, which was more of an eye-sore to me. Hence moved it there, to make everything feel like in place.
(Assignee)

Comment 14

5 years ago
(In reply to Mark Finkle (:mfinkle) from comment #12)
> Comment on attachment 631210 [details] [diff] [review]
> Patch (2/2): Animation
> 
> The TabsLayoutChangeListener is a nice way to keep the code decoupled. I
> suppose I can let it go for now that the TabsPanel can only have a single
> TabsLayoutChangeListener. We can add support when we need it.
> 
> I have a contempt for all types of animations and this is no different. Slow
> phones and tablets will be really rough. We definitely need support for
> suspending the Gecko rendering and maybe a non-linear property interpolator,
> so we have less animation steps overall. Filed bugs for these issues.
> 
> I think we need this landed so we can move it forward. It definitely can't
> be uplifted until we fix some of the follow up bugs first.

I have a added a Decelerating interpolator, that comes with Android. We can change this to anything, based on phones/tablet. We can increase the factor for the interpolator, say 2.0 or something, so that the number of steps reduce.

(Actually I wanted a Bouncing interpolator for phones to make it feel nice)

I've already filed bugs for the followups and tagged them here.
Both the patches have been tested on 1 gingerbread phone, 2 ICS phones, 1 honeycomb tablet. All possible regressions have been fixed in the patches.
(Assignee)

Comment 15

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/6c5327e377a1
http://hg.mozilla.org/integration/mozilla-inbound/rev/6d23c3d5e804
https://hg.mozilla.org/mozilla-central/rev/6c5327e377a1
https://hg.mozilla.org/mozilla-central/rev/6d23c3d5e804

(Merged by Ed Morley)
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16

Updated

5 years ago
Depends on: 762955
(Assignee)

Updated

5 years ago
Duplicate of this bug: 758319
Depends on: 763202

Updated

5 years ago
Depends on: 763448

Updated

5 years ago
Depends on: 763338

Updated

5 years ago
Depends on: 763446

Updated

5 years ago
Depends on: 763406

Updated

5 years ago
Depends on: 763396
Depends on: 763485
Depends on: 763726
Depends on: 764729
Depends on: 765580
Depends on: 765805
(Assignee)

Comment 18

5 years ago
Comment on attachment 631209 [details] [diff] [review]
Patch (1/2): Layout change

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New feature - Tabs UI
User impact if declined: Tablet wouldnt have an optimized Tabs UI
Testing completed (on m-c, etc.): 06/08
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: -
Attachment #631209 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 19

5 years ago
Comment on attachment 631210 [details] [diff] [review]
Patch (2/2): Animation

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New feature - Tabs UI
User impact if declined: Tablet wouldnt have an optimized Tabs UI
Testing completed (on m-c, etc.): 06/08
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: -
Attachment #631210 - Flags: approval-mozilla-aurora?
Depends on: 765941
(Assignee)

Updated

5 years ago
Depends on: 760970
No longer depends on: 765941
(Assignee)

Updated

5 years ago
Depends on: 765941
Comment on attachment 631209 [details] [diff] [review]
Patch (1/2): Layout change

[Triage Comment]
Tablet UI bugs are approved for Aurora 15, as they carry little risk to mobile and none to desktop.
Attachment #631209 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

5 years ago
Attachment #631210 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 21

5 years ago
Pushed to aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/65c97b27926c
https://hg.mozilla.org/releases/mozilla-aurora/rev/6824811f5808

Updated

5 years ago
status-firefox15: --- → fixed

Updated

5 years ago
Depends on: 769388

Updated

5 years ago
Depends on: 768835

Updated

5 years ago
Whiteboard: [tablet]
Depends on: 770270

Updated

5 years ago
Status: RESOLVED → VERIFIED
status-firefox15: fixed → verified
status-firefox16: --- → verified
status-firefox17: --- → verified
You need to log in before you can comment on or make changes to this bug.