1022102 - Clean up the actionbar layout and styling

Status: RESOLVED FIXED

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

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

The actionbar holds the editbox for typing searches. It also has "go" and "clear" buttons that should display at the right times.

Can we get a UX mockup (with styling details) of the actionbar and then get some image assets for the buttons.

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_search_toolbar_mock1.png

Here's the design I've got right now for this.

It essentially functions like our current Fennec toolbar with some visual updates to align more with our efforts via bug 1014848

You'll notice that there is no 'go' button though (as on Fennec) as we can leverage the "go" on the keyboard to clean it up even more.

Comment 2

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: icon_x.zip

Here's the x icon

Comment 3

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: icon_search.zip

.. and the search icon!

Comment 4

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_search_toolbar_mock2.png

Got some specs here for you, Eric :)

This toolbar should be similar to the one in Bug 1010740 too if you're looking for assets to reuse or what not.

Comment 5

[Eric Edens [:eedens]]

Attachment: 9sp-font.png

Anthony, before I get too deep into this, I wanted to check some of the sizes.

For the font, 9sp is looking pretty small. The smallest pre-defined font size in Android is 12sp (micro), and the next larger is 14sp (small). [1]

The icons are a bit smaller than Android's icons as well. For example, for XHDPI, the attached "x" icon is 28px, while Android's is 36px. [2]

[1] https://developer.android.com/design/style/typography.html
[2] https://developer.android.com/design/downloads/index.html

Comment 6

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Hey Eric,

Let me play with this for a bit more (since I'll have to figure this out anyways as a part of the other bug I mentioned) and let's try 10sp for now.

Comment 7

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Comment on attachment 8446678 [details]
prev_search_toolbar_mock2.png

I converted these incorrectly (thought it was XX but it was just XHDPI) - will update.

My bad, Eric! (that type should've been 14 sp)

Comment 8

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_toolbar_states3_specs.png

Eric, I quickly made some specs (hopefully correctly this time). Let me know if this helps.

Comment 9

[Eric Edens [:eedens]]

Hi Anthony,

Here's a quick snapshot: https://people.mozilla.org/~eedens/search/bin/FirefoxSearch_20140701.apk

Some notes / questions:

- This build only has UI/UX For this upload, I have been working on the search bar exclusively. Much of the rest of the app doesn't match your design yet :)

- I feel like the clickable area of the "x" could be increased. What do you think?

- The orange border -- how should that look when the search box doesn't have focus? 

- The stroke of the "x" button is pretty thick. Also, the magnifying glass is a different color in the icon pack than in the spec screenshot. For consistency, how would you feel about using the stock icons (https://drive.google.com/a/mozilla.com/folderview?id=0B370567wSAPBZWZUNFFmbHdsYVk&usp=drive_web) ? 

- Regarding fonts, Roboto ships with Android 4.0 and greater. Older devices will see their default sans serif.

Comment 10

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Eric Edens [:eedens] from comment #9)
> - This build only has UI/UX For this upload, I have been working on the
> search bar exclusively. Much of the rest of the app doesn't match your
> design yet :)

Awesome!

> - I feel like the clickable area of the "x" could be increased. What do you
> think?

yep! the hit area should be the entire bar's height and extend to the right edge of the screen and the left edge of the input box. 

> - The orange border -- how should that look when the search box doesn't have
> focus? 

It should revert back to the normal state.
 
> - The stroke of the "x" button is pretty thick. Also, the magnifying glass
> is a different color in the icon pack than in the spec screenshot. For
> consistency, how would you feel about using the stock icons
> (https://drive.google.com/a/mozilla.com/
> folderview?id=0B370567wSAPBZWZUNFFmbHdsYVk&usp=drive_web) ? 

I'm actually in the process of refining these controls as a part of another bug for the tool bar in general so let's try to use these new ones. I think the 'x' you've used is the incorrect one though it looks too dark...

> - Regarding fonts, Roboto ships with Android 4.0 and greater. Older devices
> will see their default sans serif.

Works!

Comment 11

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: spec_toolbar_x.png

Hope this helps!

Comment 12

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_search_toolbar_mock3.png

As promised, for the search bar states.

Comment 13

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_searchfield_new.png

Attaching new designs for the search input field

Comment 14

[:Margaret Leibovic]

Attachment: Polish search bar interaction and styling

This patch is a start towards polishing the search bar style and interaction.

A bit of SearchFragment code clean-up ended up in here, but it's mostly just reorganizing a few things, and I'm feeling lazy about splitting this into a different patch :)

A few notes:

* I moved some view-agnostic initialization logic out of onCreateView and into onAttach
* I got rid of the transitionToWaiting() call in onCreateView, and instead just made sure that the views were initialized in the waiting state
* I decided to punt on the EditText background for now (the thing that makes the underling), since it looks like that will require NinePatch images to do correctly

PR here: https://github.com/ericedens/FirefoxSearch/pull/27

(This patch applies to fx-team/FirefoxSearch master, but I can rebase if we want to land any of your work first.)

Comment 15

[Eric Edens [:eedens]]

Comment on attachment 8458919 [details] [diff] [review]
Polish search bar interaction and styling

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

Looks good! Once you land, I'll rebase bug 1022100.

::: mobile/android/search/java/org/mozilla/search/autocomplete/SearchFragment.java
@@ +122,1 @@
>  

I'll open a bug to consolidate the click handlers on the parent. The parent will intercept touch events, and decide whether to deliver the event depending on the state of the widget. If it's RUNNING, deliver. If it's waiting, don't deliver.  Ref: https://developer.android.com/training/gestures/viewgroup.html

@@ +153,5 @@
>          // Attach listener for tapping on a suggestion.
>          suggestionDropdown.setOnItemClickListener(new AdapterView.OnItemClickListener() {
>              @Override
>              public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
>                  String query = (String) suggestionDropdown.getItemAtPosition(position);

Final

::: mobile/android/search/res/layout/search_activity_main.xml
@@ +28,5 @@
>  
>      <fragment
>          android:layout_width="match_parent"
>          android:layout_height="match_parent"
> +        android:name="org.mozilla.search.autocomplete.SearchFragment"/>

Probably don't want to remove this (maybe rename it?). Even though we don't use the id for this fragment ATM, it looks like the fragment lifecycle uses the ids when restoring fragments.  "Note: Each fragment requires a unique identifier that the system can use to restore the fragment if the activity is restarted ...." [1] 


[1] http://developer.android.com/guide/components/fragments.html#Adding

Comment 16

[:Margaret Leibovic]

Pushed to fx-team:

https://hg.mozilla.org/integration/fx-team/rev/becea164aba0
https://hg.mozilla.org/integration/fx-team/rev/0773f2e66747

The PR is up to date, so you can merge it!

https://github.com/ericedens/FirefoxSearch/pull/27

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/becea164aba0
https://hg.mozilla.org/mozilla-central/rev/0773f2e66747

Comment 18

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attaching .APK download link that Eric sent me for this bug: goo.gl/DJ9yIb

I think we should keep this open because it's not really wrapped up yet.

Comment 19

[Eric Edens [:eedens]]

Anthony --

Since the associated patch has landed, I think it's better to leave this closed. Can you open a bug (some bugs?) to document the items that are remaining?

Comment 20

[:Margaret Leibovic]

(In reply to Eric Edens [:eedens] from comment #19)
> Anthony --
> 
> Since the associated patch has landed, I think it's better to leave this
> closed. Can you open a bug (some bugs?) to document the items that are
> remaining?

Yes, I agree with Eric, it's easier to track development work if we work in smaller separate bugs.

Comment 21

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Alright will do :) 

Filed it here: Bug 1041738