Closed Bug 1041738 Opened 5 years ago Closed 5 years ago

UX/UI polish for the search activity actionbar and search suggestions

Categories

(Firefox for Android Graveyard :: Search Activity, defect)

x86
Android
defect
Not set

Tracking

(firefox34 verified)

VERIFIED FIXED
Firefox 34
Tracking Status
firefox34 --- verified

People

(Reporter: antlam, Assigned: Margaret)

References

Details

Attachments

(4 files)

Attached image prev_currentstate.png
Splitting off from bug 1022102 as part of a polishing effort for the action bar specifically.

Also attaching a screen shot of what we currently have to easily compare and tweak :)
I'd like to raise the issue that the inactive state of the search bar in this mock-up really doesn't look like a click target.

The main things I see that we need to work on here are:

* Tweak the background color for the activity (color value?)
* Tweak the inactive state text color (color value?)
* Use a black magnifying glass icon in the active state (right now I only have a gray icon)
* Use an orange underline for the active state

Do we also want to use orange text selection handles to match Fennec? I can copy over the assets for now, and we can remove dupes when we deprecate the github repo build.

I've been trying to dig into where the current background gray color for the activity comes from, but I'm having trouble finding it. It looks like all the fragments just have transparent backgrounds, so the background that's showing is the app's background, but I can't figure out where that's being specified.
Assignee: nobody → margaret.leibovic
Blocks: search
Flags: needinfo?(eedens)
(In reply to :Margaret Leibovic from comment #1)
> I'd like to raise the issue that the inactive state of the search bar in
> this mock-up really doesn't look like a click target.

Fair point, I'll experiment with some options here. My hope was to really draw focus to the search terms/ history items. Also, since there's not much to do on the page in terms of CTA's that demand the users attention, that keeping it clean and simple would not only lend itself to feeling light and quick but also be just enough.

That being said, I will give this some more thought still. stay tuned! :)

> The main things I see that we need to work on here are:
> 
> * Tweak the background color for the activity (color value?)

Here it is! #EBEBF0

> * Tweak the inactive state text color (color value?)

Roboto light, 15 dp, #777777
> I've been trying to dig into where the current background gray color for the
> activity comes from

The style is specified in AndroidManifest as:

            android:theme="@style/AppTheme"

AppTheme is defined in values/search_styles.xml (older devices) and values-v13/search_styles.xml (newer devices).

Both XML files will need to be updated; something like this would work:

    <style name="AppTheme" parent="@android:style/<theme for sdk level>">
        <item name="android:windowBackground">@android:color/global_background_color</item>
        <item name="android:colorBackground">@android:color/global_background_color</item>
    </style>
Flags: needinfo?(eedens)
Morphing this to also be about styling the search suggestions, since I decided to just make one big polish pass.

WIP here: https://github.com/leibovic/FirefoxSearch/commit/431ef13d761878d7c1766022610d3a83e9e5508e

Build here: http://people.mozilla.org/~mleibovic/search.apk
Summary: UX/UI polish for the search activity actionbar → UX/UI polish for the search activity actionbar and search suggestions
Attached file icon_search.zip
Attaching magnifying glass..
Attached file icon_x.zip
...and the x
In case the border is hard to catch, I thought I'd try to be helpful and provide the color code: #BFBFBF

Also, if it helps - the margins in the mock up are as follows:)
 - on either side are 15dp
 - in between each "card" are 7 dp
 - The first card is 68 dp away from the bottom of the top menu bar.
Here's a cleaned up version of my patch that addresses some spacing and color issues. Outstanding issues from feedback that I will address in follow-up bugs (issues that haven't already been filed as other bugs):

* Clear button shouldn't show unless there is text in the input field
* Active color for cards when they are pressed
Attachment #8461189 - Flags: review?(eedens)
Comment on attachment 8461189 [details] [diff] [review]
Style search bar and suggestions

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

Just two small things; looks good!

::: mobile/android/search/java/org/mozilla/search/autocomplete/ClearableEditText.java
@@ +100,5 @@
>          editText.setFocusable(active);
>          editText.setFocusableInTouchMode(active);
>  
> +        final int left = active ? R.drawable.search_icon_active : R.drawable.search_icon_inactive;
> +        editText.setCompoundDrawablesWithIntrinsicBounds(left, 0, 0, 0);

Nit: var name `left` is a bit ambiguous -- initially I thought it was a distance value. Maybe rename (leftDrawable) or skip the var and inline the ternary.

::: mobile/android/search/res/layout/search_auto_complete_row.xml
@@ +20,4 @@
>          android:layout_height="wrap_content"
> +        android:paddingTop="@dimen/card_padding_y"
> +        android:paddingBottom="@dimen/card_padding_y"
> +        android:textSize="16sp"/>

This TextView is the same as search_card_history, right? If so, could we do an <include> here?
Attachment #8461189 - Flags: review?(eedens) → review+
(In reply to Eric Edens [:eedens] from comment #9)
> Comment on attachment 8461189 [details] [diff] [review]
> Style search bar and suggestions
> 
> Review of attachment 8461189 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Just two small things; looks good!
> 
> :::
> mobile/android/search/java/org/mozilla/search/autocomplete/ClearableEditText.
> java
> @@ +100,5 @@
> >          editText.setFocusable(active);
> >          editText.setFocusableInTouchMode(active);
> >  
> > +        final int left = active ? R.drawable.search_icon_active : R.drawable.search_icon_inactive;
> > +        editText.setCompoundDrawablesWithIntrinsicBounds(left, 0, 0, 0);
> 
> Nit: var name `left` is a bit ambiguous -- initially I thought it was a
> distance value. Maybe rename (leftDrawable) or skip the var and inline the
> ternary.

Will do.

> ::: mobile/android/search/res/layout/search_auto_complete_row.xml
> @@ +20,4 @@
> >          android:layout_height="wrap_content"
> > +        android:paddingTop="@dimen/card_padding_y"
> > +        android:paddingBottom="@dimen/card_padding_y"
> > +        android:textSize="16sp"/>
> 
> This TextView is the same as search_card_history, right? If so, could we do
> an <include> here?

Unfortunately, no, they're not the same. For the search_auto_complete_row, the background is applied to the FrameLayout that hold the TextView and the Button, but for search_card_history, the background is applied to the TextView itself. Also unfortunately, I couldn't apply the same padding to the search_auto_complete_row FrameLayout and the search_card_history TextView, since the jump button takes up a decent amount of vertical space and would make the search suggestion rows too high if there was also padding around it.
https://hg.mozilla.org/mozilla-central/rev/d849aa6002c3
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
(In reply to Richard Newman [:rnewman] from comment #13)
> For the record, this change added 100KB to our APK size.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/3f9060993657	armv6
> 28935087
> https://hg.mozilla.org/integration/mozilla-inbound/rev/5d2176af4ed1	armv6
> 28934535
> https://hg.mozilla.org/integration/mozilla-inbound/rev/b83bb45e9010	armv6
> 29040131
> https://hg.mozilla.org/integration/mozilla-inbound/rev/707fe2b64a85	armv6
> 29038759
> https://hg.mozilla.org/integration/mozilla-inbound/rev/69e731a9f604	armv6
> 29040811

Well, resources will do that. I'm not sure of a way around this if we want things to look a certain way.

In fact, I bet the whole search activity adds a lot to our APK size.
Very likely!

We should make sure that we're shipping crunched PNGs. Eventually the split APK work will help to reduce the impact.
Verified as fixed in build 34.0a1 (2014-08-04);
Devices: Samsung Galaxy Nexus (Android 4.2.1) and Asus Transformer Tab (Android 4.2.1).
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.