Closed Bug 1070087 Opened 10 years ago Closed 10 years ago

Implement new tablet menu bar pressed/focused button size

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 35

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

Attachments

(5 files, 7 obsolete files)

31.16 KB, image/png
Details
569.39 KB, image/png
Details
488.58 KB, image/png
Details
488.56 KB, image/png
antlam
: feedback+
Details
7.64 KB, patch
mcomella
: review+
Details | Diff | Splinter Review
Quite mixed about these rounded corners btw. They're kinda alien in Android's design language both in Holo and the upcoming Material design.
Anthony, thoughts on comment 1?
Flags: needinfo?(alam)
(In reply to Lucas Rocha (:lucasr) from comment #1)
> Quite mixed about these rounded corners btw. They're kinda alien in
> Android's design language both in Holo and the upcoming Material design.

Fair point, Lucas.

I definitely agree it's something new here but I'd like to see how it feels. I've always found the hard corners that most apps use to be a bit... harsh. They also just don't feel that great. Maybe it's because the hit area is usually just the visible boundaries, whereas here I'm trying to contain that a bit more (just visually, the touch area can be the same so it's still easy to tap).

I'm also not sold on the "water's surface" effect that Materials is pushing... Microsoft did that for their Surface's stylus interactions a while back as well.
Flags: needinfo?(alam)
Updating the title to discuss the issues raised in comment 1 and comment 3.
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
Summary: Round edges on pressed state of toolbar menu items → Discuss new tablet menu bar button size
Attached image spec_taparea_feedbackarea.png (obsolete) —
Hopefully this clears things up a bit. Let's see how this feels!

The color on tap here is #D7D7DC.
Flags: needinfo?(michael.l.comella)
I assume you actually meant 56dp instead of 48dp here?
Flags: needinfo?(alam)
Nice catch! These should be the right, updated ones.
Attachment #8496968 - Attachment is obsolete: true
Flags: needinfo?(alam)
I want to note that the pressed state can look a little strange when the visual tap area is the full toolbar height.
(In reply to Michael Comella (:mcomella) from comment #8)
> Created attachment 8497885 [details]
> Pressed state, toolbar height
> 
> I want to note that the pressed state can look a little strange when the
> visual tap area is the full toolbar height.

Yeah, I think I mentioned that before. This is the issue that antlam's design fixes. The pressed state should not touch the toolbar edges.
Attached image Tapped button, smaller visual (obsolete) —
Anthony, is this what you expected?
Attachment #8498549 - Flags: feedback?(alam)
Flags: needinfo?(michael.l.comella)
Anthony, the color for the pressed state of the forward/back buttons was not changed and is darker than the other action bar items - should the color for back/forward be changed as well? Is there any more unifying that should be done here, like bug 1055536?
Flags: needinfo?(alam)
Anthony, is this what we want for private browsing? If not, would it be possible to define the pressed color in terms of an alpha value for both standard and private states, as before (was #33000000)? It'd simplify the code a bit.
Attachment #8498562 - Flags: feedback?(alam)
Comment on attachment 8498545 [details] [diff] [review]
Change tapped action bar buttons' visual to be smaller than the hit area

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

Looks good (with questions answered/dealt with). Please wait for bug 1065494 to land before pushing this.

::: mobile/android/base/resources/drawable-large-v11/new_tablet_action_bar_button.xml
@@ +10,5 @@
> +        <inset android:drawable="@drawable/new_tablet_action_bar_button_pressed"
> +               android:insetTop="10dp"
> +               android:insetBottom="10dp"
> +               android:insetLeft="6dp"
> +               android:insetRight="6dp"/>

It seems the inset did the trick then? ;-)

@@ +16,5 @@
> +
> +    <item android:state_focused="true"
> +          android:state_pressed="false">
> +        <shape>
> +            <solid android:color="@color/highlight_focused"/>

Why don't these have insets too?
Attachment #8498545 - Flags: review?(lucasr.at.mozilla) → review+
Design drive-by: We should probably use a more subtle color for the pressed state in private mode.
Comment on attachment 8498549 [details]
Tapped button, smaller visual

yes! but it looks a bit small, what size is the visual feedback area?
Attachment #8498549 - Flags: feedback?(alam) → feedback-
Flags: needinfo?(alam)
Comment on attachment 8498562 [details]
Private browsing, tapped button, smaller visual

Yeah, I didn't realize I hadn't spec'd this out for you guys but let's try #222222 for the PB mode hit state
Attachment #8498562 - Flags: feedback?(alam) → feedback-
I added the inset to the focused state but it's unnecessary for the default state. I also moved the pressed state drawable directly into the layout (rather than a separate file). I added some dimens to make accessing all of these values easier for both insets, and removed the color attr for the pressed state because it seemed unnecessary.
Attached image Focused button
Anthony gave me this color over IRC, likely temporary.
Is this what you were looking for, Anthony? Note the focused screenshot I added above this comment.
Attachment #8498549 - Attachment is obsolete: true
Attachment #8499240 - Flags: feedback?(alam)
re attachment in comment 18: I also fixed the size of the tapped region, as per comment 16 (we spoke on IRC).
Anthony, do you have final colors for the focused state in regular and private browsing modes (or a temporary one for private browsing), or should I file a followup?
Flags: needinfo?(alam)
Comment on attachment 8499240 [details]
Tapped button, smaller visual

nice!

Focused looks a bit dark (screenshot above) but we can leave that for V1 as I don't imagine that to be a primary design consideration.
Attachment #8499240 - Flags: feedback?(alam) → feedback+
(In reply to Michael Comella (:mcomella) from comment #22)
> Anthony, do you have final colors for the focused state in regular and
> private browsing modes (or a temporary one for private browsing), or should
> I file a followup?

Let's break it off into a follow up bug. 

Filed under bug 1077195
Flags: needinfo?(alam)
Looking great!

Not directly related to this bug but I noticed that there's an extra right margin on the menu button that makes it look slightly off. Is that intended?
(In reply to Lucas Rocha (:lucasr) from comment #25)
> Looking great!
> 
> Not directly related to this bug but I noticed that there's an extra right
> margin on the menu button that makes it look slightly off. Is that intended?

Yeah, I've got clearance on both sides of the UI that makes those visuals not touch the edge of the device. I don't think we did this before, but I thought I'd give it a try. So far, I've found that while it might be a bit of a "weird" idea at first, it seems to play better when actually on a device, in hand. Something about the Android device form factors that make that clearance feel justified and organized.

You'll notice the same thing WRT the "normal tabs" vs "private tabs" indicator (underline) in the new tabs tray/panel (top left).
Going to land this without f+, but we can file a followup if this is undesirable.
Attachment #8498562 - Attachment is obsolete: true
Attachment #8499908 - Flags: feedback?(alam)
Comment on attachment 8499908 [details]
Private browsing, tapped button, smaller visual

Can't find where I left this value but if it's not too much trouble could we do #222222 for V1 visual feedback on-press? For focused (with non-touch devices) we could use #363B40 for now (same as the background).
Attachment #8499908 - Flags: feedback?(alam) → feedback-
Summary: Discuss new tablet menu bar button size → Implement new tablet menu bar pressed/focused button size
Comment on attachment 8499907 [details] [diff] [review]
Part 2: Set private browsing new tablet menu item pressed/focused color

Followup in bug 1077755.
Attachment #8499907 - Attachment is obsolete: true
Attachment #8499907 - Flags: review?(lucasr.at.mozilla)
https://hg.mozilla.org/mozilla-central/rev/b1718e507da7
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.