Closed Bug 1064415 Opened 5 years ago Closed 5 years ago

Tabs tray/grid/panel interactions and visual design

Categories

(Firefox for Android :: Theme and Visual Design, defect)

x86
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 36

People

(Reporter: antlam, Assigned: mhaigh)

References

Details

Attachments

(9 files, 4 obsolete files)

81.86 KB, image/png
Details
83.70 KB, image/png
Details
4.51 KB, application/zip
Details
71.18 KB, image/png
Details
89.66 KB, image/png
Details
39.31 KB, application/zip
Details
52.12 KB, image/png
antlam
: feedback+
lucasr
: feedback+
Details
23.92 KB, patch
lucasr
: feedback+
Details | Diff | Splinter Review
64.32 KB, image/png
lucasr
: feedback+
antlam
: feedback+
Details
Opening this bug to track the V1 visual and interaction side of the new grid view for the tabs tray/panel.

Martyn's working APK: https://dl.dropboxusercontent.com/u/7163922/fennec-grid-tabstray.apk
Attached image prev_tabtray_vert_mock1.png (obsolete) —
Posting the first WIP of this view. Focusing on the structure and proportions first of all and just keeping on par with our current functionality. 

Indicator for which tab is open is missing, along with tabs for "private" vs "normal"
Attached image prev_tabtray_vert_mock2.png (obsolete) —
I think this covers all of the bases for V1...
Attachment #8490252 - Attachment is obsolete: true
Attached image prev_tabtray_vert_mock3.png (obsolete) —
After trying it on a tablet, the icons on the top right needed more padding!
Attachment #8491071 - Attachment is obsolete: true
Attached image prev_tabtray_hori_mock2.png (obsolete) —
Attaching what the horizontal view would look like. Lots more room, but I'd recommend trying it out on a N7 to get the full effect. Tried adding another in there but seemed too cramped. I'm just trying to keep on par with our current functionality.

We can and will revisit this after V1 to be more robust and full fledged. But for V1, this is a good start I reckon.

Thoughts?

Also, NI-ing Martyn since he did the first build for this.
Flags: needinfo?(mhaigh)
Nice work Anthony!

I talked with Anthony on the icons this afternoon. We were debating on whether to show "tabs" indicator on the tab panel.

I think without the zooming transitions between viewport and tab panel, having the tab indicator will certainly help. But there is a compelling and efficient transition that is easier to perform than hitting a icon, I see the tab indicator on the panel becomes redundant. 

We decided to try both approaches at this stage and test both out. So Martyn, when you implement the tab panel, it would be great to get one version with tab indicator on the panel, and the other without. Thanks!

I will put transition specifics into the bug soon.
Just gotta finish bug 1066546 before I can deliver those APKs, but wanted to know a few things:

1, do the tab views change dimensions based on screen orientation?  how about on different sized screens?
2, This will probably be at least partially answered by 1, but are we dynamically allocating the number of tabs per row, or will this change depending on screen size?
3, are we getting rid of the chrome (tabs, awesomebar etc)? if so, how do we return to the main browser window?
Flags: needinfo?(mhaigh)
Hey Martyn,

Great questions! For V1:

1) Nope, they're the same in the design at 168 dp x 140 dp
2) I've tried to fix just the left most and right most margins at 24 dp. This, coupled with the fixed screen size should help us be a bit smart with the number of tabs we display.
3) Yuan and I are actually hoping to get a build to clear this interaction up. We're juggling two ideas ATM: A) leaving the tabs icon there to quickly switch back, B) utilizing the hardware back button. Both of these ideas ATM do account for the fact that we are looking to incorporate gestures soon that will also be a way to do this, and also tapping on the "active" tab in the tray view.

Let me know if you have more questions :)
^
Flags: needinfo?(mhaigh)
Blocks: 1072862
Thanks for the builds Martyn! I think there's a bigger issue here: how much of the tabs should we show along the bottom? I.e. when the user has the tabs tray active. But that is a much bigger thing that I think we need to figure out for V2. 

For the time being, let's keep the tab icon out. And we can file a separate bug to refine the tray (more specifically how this view).

But I'd also like to remove the + from the bottom as well. It shows up in two places which is where it starts to look really complicated (top right and bottom right when the tray is up).

Thoughts?
Flags: needinfo?(mhaigh)
I agree - maybe we could fade out the bottom + when the panel opens?

I heard there was talk of making the awesomebar draggable to show the tabs panel - can't find a bug no. for that atm but if that's going to land soon we should keep it in mind. (Is this actually a thing - did I make it up?)
Flags: needinfo?(mhaigh)
Attached image spec_tabtray1.png
Specs! 

I hope this helps Martyn. Let me know if you have questions.

I basically just spec'd out the sides for margins since it really depends on the device. The 'x' is also just vertically center aligned to the text and right aligned to the edge of the tab's preview.

Lastly, on active, I have the Page title color as #F5F5F5. 

I'll also attached the x after in case we need it.
Flags: needinfo?(mhaigh)
Attached file icon_x.zip
icons!

Also, FYI, I used the same darker ones already in the tabs strip along the top (not sure if they got implemented yet).
NI self to try icons in comment 14 for tab strip.
Flags: needinfo?(michael.l.comella)
Sorry for the late opinion, but I'm glad to see this being worked on.  I just wanted to state that in the current design many users on Input have had issues seeing (and perhaps clicking) on the "x"'s to close the tab.  Glad to see that the new mocks have a more accentuated contrast there and I hope that we're able to keep/integrate it.

Thanks
Thanks Rob!

That's actually pretty useful
Filed bug 1087734 for comment 15.
Flags: needinfo?(michael.l.comella)
Updated with the globe, fixed margins on left to be consistent to the "non-tray" view.
Attachment #8492335 - Attachment is obsolete: true
^
Attachment #8491819 - Attachment is obsolete: true
Here are the "just globes"
Attached image Layout screenshot
Current layout - ready for review but wanted to get UX opinion first
Flags: needinfo?(mhaigh)
Attachment #8513450 - Flags: feedback?(alam)
Attachment #8513450 - Flags: feedback?(lucasr.at.mozilla)
Spacing, icons, and colours adjusted.  Feedback request for quick turn around depending on UX feedback
Attachment #8513464 - Flags: feedback?(lucasr.at.mozilla)
Comment on attachment 8513464 [details] [diff] [review]
Tabs tray/grid/panel interactions and visual

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

Looking pretty good. Just need some tweaks.

You will have to stub all new images in the drawable directory. Otherwise this will break SDK = 9 builds. You can just copy drawable/new_tablet_tabs_count.xml with each new drawable new introduced here.

::: mobile/android/base/newtablet/res/color-large-v11/new_tablet_tab_item_title.xml
@@ +3,5 @@
> +
> +    <item android:state_checked="true"
> +          android:color="@color/text_color_primary_inverse"/>
> +
> +    <item android:color="@color/new_tablet_tab_grid_text_unselected"/>

No way to reuse an existing color here?

::: mobile/android/base/newtablet/res/drawable-large-v11/new_tablet_tab_panel_close_button.xml
@@ +1,1 @@
> +<?xml version="1.0" encoding="utf-8"?>

Rename to new_tablet_tab_item_close_button for consistency with new_tablet_tab_item_title

::: mobile/android/base/resources/layout/new_tablet_tabs_item_cell.xml
@@ +28,2 @@
>                    style="@style/TabLayoutItemTextAppearance"
> +                  android:textSize="14sp"

This should probably be defined in the style using a predefined textAppearance instead. File a follow-up.

@@ +38,5 @@
>                       android:background="@drawable/action_bar_button_inverse"
>                       android:scaleType="center"
>                       android:contentDescription="@string/close_tab"
> +                     android:src="@drawable/new_tablet_tab_panel_close_button"
> +                     android:duplicateParentState="true"/>

Why does it duplicate the parent's state?

::: mobile/android/base/resources/values/dimens.xml
@@ +124,5 @@
>  
>  
> +    <dimen name="new_tablet_tab_thumbnail_width">168dp</dimen>
> +    <dimen name="new_tablet_tab_thumbnail_height">140dp</dimen>
> +    <dimen name="new_tablet_tab_grid_width">178dp</dimen>

new_tablet_tab_panel_column_width for clarity

::: mobile/android/base/tabs/TabsLayoutItemView.java
@@ +99,2 @@
>          } else {
> +            mThumbnail.setImageResource(R.drawable.new_tablet_tab_thumbnail_default);

This will use the new image in all UIs, including phones and old tablet. I assume this is not intentional. You should probably add a NewTabletUI.isEnabled() check here and use new_tablet_tab_thumbnail_default only on the new tablet UI.
Attachment #8513464 - Flags: feedback?(lucasr.at.mozilla) → feedback+
Comment on attachment 8513450 [details]
Layout screenshot

Thumbs up from me. Please file a follow-up for the default image issue.

How does it look in landscape mode?
Attachment #8513450 - Flags: feedback?(lucasr.at.mozilla) → feedback+
Comment on attachment 8513450 [details]
Layout screenshot

that default image has some weird sizing going on there it's also not centered but I think that's a follow up?

This looks good on vert but how about horizontal orientation?
Attachment #8513450 - Flags: feedback?(alam) → feedback+
Attached image Landscape visuals
Here's the landscape view
Attachment #8513611 - Flags: feedback?(lucasr.at.mozilla)
Attachment #8513611 - Flags: feedback?(alam)
(In reply to Martyn Haigh (:mhaigh) from comment #27)
> Created attachment 8513611 [details]
> Landscape visuals
> 
> Here's the landscape view

The title and close button looks slightly misaligned with the thumbnail in landscape. It's ok to fix this in a follow-up if you don't find a a quick way to fix it.
Comment on attachment 8513611 [details]
Landscape visuals

This is ok to land, but we must fix the title & close misalignments in a follow-up.
Attachment #8513611 - Flags: feedback?(lucasr.at.mozilla) → feedback+
Also, we should probably show less columns and bigger spacing in landscape. Maybe this is just a matter of forcing a larger horizontal spacing in landscape?
Comment on attachment 8513611 [details]
Landscape visuals

^ basically what Lucas said. :)

In the design, I tried to fix just the side gutters on either side. After giving more spacing in between each tab, it added up to about 4 tabs per row for the N7 horizontal.

I'll show you what I mean with a screen shot
Attachment #8513611 - Flags: feedback?(alam) → feedback+
^ comment 4, hadn't realized I posted this somewhere, my bad.
(In reply to Lucas Rocha (:lucasr) from comment #24)
> Comment on attachment 8513464 [details] [diff] [review]
> Tabs tray/grid/panel interactions and visual
> 
> Review of attachment 8513464 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looking pretty good. Just need some tweaks.
> 
> You will have to stub all new images in the drawable directory. Otherwise
> this will break SDK = 9 builds. You can just copy
> drawable/new_tablet_tabs_count.xml with each new drawable new introduced
> here.

Done.

> :::
> mobile/android/base/newtablet/res/color-large-v11/new_tablet_tab_item_title.
> xml
> @@ +3,5 @@
> > +
> > +    <item android:state_checked="true"
> > +          android:color="@color/text_color_primary_inverse"/>
> > +
> > +    <item android:color="@color/new_tablet_tab_grid_text_unselected"/>
> 
> No way to reuse an existing color here?
> 
> :::
> mobile/android/base/newtablet/res/drawable-large-v11/
> new_tablet_tab_panel_close_button.xml
> @@ +1,1 @@
> > +<?xml version="1.0" encoding="utf-8"?>
> 
> Rename to new_tablet_tab_item_close_button for consistency with
> new_tablet_tab_item_title

Done.

> ::: mobile/android/base/resources/layout/new_tablet_tabs_item_cell.xml
> @@ +28,2 @@
> >                    style="@style/TabLayoutItemTextAppearance"
> > +                  android:textSize="14sp"
> 
> This should probably be defined in the style using a predefined
> textAppearance instead. File a follow-up.

Filed bug 1091558.

> @@ +38,5 @@
> >                       android:background="@drawable/action_bar_button_inverse"
> >                       android:scaleType="center"
> >                       android:contentDescription="@string/close_tab"
> > +                     android:src="@drawable/new_tablet_tab_panel_close_button"
> > +                     android:duplicateParentState="true"/>
> 
> Why does it duplicate the parent's state?

Got it, it's for the checked/pressed state.

> ::: mobile/android/base/resources/values/dimens.xml
> @@ +124,5 @@
> >  
> >  
> > +    <dimen name="new_tablet_tab_thumbnail_width">168dp</dimen>
> > +    <dimen name="new_tablet_tab_thumbnail_height">140dp</dimen>
> > +    <dimen name="new_tablet_tab_grid_width">178dp</dimen>
> 
> new_tablet_tab_panel_column_width for clarity

Done.

> ::: mobile/android/base/tabs/TabsLayoutItemView.java
> @@ +99,2 @@
> >          } else {
> > +            mThumbnail.setImageResource(R.drawable.new_tablet_tab_thumbnail_default);
> 
> This will use the new image in all UIs, including phones and old tablet. I
> assume this is not intentional. You should probably add a
> NewTabletUI.isEnabled() check here and use new_tablet_tab_thumbnail_default
> only on the new tablet UI.

Will fix this by implementing same behaviour across all UIs. Patch coming.
https://hg.mozilla.org/integration/fx-team/rev/5ca2031b7c33

Default thumbnail and aspect ratio will be fixed in bug 1083263.
Assignee: nobody → mhaigh
https://hg.mozilla.org/mozilla-central/rev/5ca2031b7c33
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
You need to log in before you can comment on or make changes to this bug.