1049600 - Show search engine branding

Status: RESOLVED FIXED

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

Description

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

Attachment: XXHDPI - Mobile - Pre typing 5.png

Using this bug to track the "show current search engine being used" UI elements on the pre-search stage.

Comment 1

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

Attachment: XXHDPI - Mobile - First run 2.png

In light of bug 1045655, this is more along the lines of what it might look like.

Comment 2

[Eric Edens [:eedens]]

Attachment: suggest_rows_covered.png

Hey Anthony --

I overlayed the button portion of your mock onto the current UI, and the proposed search engine icon covers the fourth suggestion.

I'm not comfortable losing 25% of our suggestions. Are there some alternatives that we could explore? Such as moving the icon to the search bar area?

Comment 3

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

I would agree. :) I think that the icon for search engine and settings should only be there when the space allows. This would mean giving precedence to suggestions, and if there weren't that many or any at all, we could show those.

I realize this isn't a perfect solution since there still exists a screen where the user could be without any visual branding on screen (like a Yahoo!, Bing, or Google logo) but I think this will do for MVP, for now. :\

I have a couple WIP mocks but since this touches on so many areas and we're already keeping tabs on the suggestions (typing) stage of this UI, I'd like to tackle this as a part of that. For now, I'd like to focus on the experience of searching and not so much around having branding. It's also important to not go overboard with it (not that we are right now) since it would also be harder to remove later on.

Comment 4

[Eric Edens [:eedens]]

From IRC talk, it sounds like Anthony is still working on these designs, and there are going to be some upcoming meetings regarding partner branding.

Setting needinfo for antlam; no rush -- just ping back when you're ready to move forward.

Comment 5

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

Sorry for the miscommunication! I think that might have been my fault. 

But I think we should land this first and we can iterate later since this is still MVP.

Again, I don't think we need it to be there when the suggestions and history items aren't up. This label isn't as important as the provider icon.

Comment 6

[:Margaret Leibovic]

Making this bug P1 again. NI to antlam to help figure out where we can put search engine branding.

Comment 7

[:Margaret Leibovic]

FYI, bug 1059537 will help us get images out of the search plugin XML.

Comment 8

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

Attachment: prev_branding_overview1.png

Attaching the latest WIP for this bug. I think we should leverage the icon as a launching point for the contextual menu that users see when they switch engines. You might notice the purple underline, that's branding purple so if they use Google, we could ideally switch it out to blue. Non-supported sites would just use our default orange.

One last thing to call out is the text in the cards are not vertically aligned with the input field in this mock but they are aligned with the suggestions (as per bug 1051973). Wouldn't pay too much mind to that yet - that's still an undecided detail. :)

Comment 9

[Eric Edens [:eedens]]

For dynamic underline colors, the work in bug 1042960 could help here -- right now it's inside the private class, org.mozilla.search.ui.FacetBar.FacetButton.

Comment 10

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

Attachment: prev_branding_overview2.png

Attaching the latest screen shot of the design for this bug.

As mentioned, there are concerns about the "typing phase" missing branding but let's start with something first, get it into a build and test that. I definitely get the concerns but we want to avoid scope creeping so I'd like to follow up and solve that screens issue separately.

Also, getting quick access to the contextual menu of Search engines to switch to should be the functionality behind that branding icon.

Comment 11

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

^ I MAY have a better solution after talking with Bryan! will work on this more over the weekend. :) stay tuned!

Comment 12

[:Margaret Leibovic]

I'm updating the summary to make this bug more general, since it seems like we're going to try to address branding throughout the user flow here.

I just landed bug 1059537, so we can get the icon out of the search plugin now, and I started playing around with putting it in the search bar, but I'll stay tuned for new mock-ups!

Comment 13

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

Attachment: XXHDPI - Mobile - First run 4.png

(In reply to :Margaret Leibovic from comment #12)
> I'm updating the summary to make this bug more general, since it seems like
> we're going to try to address branding throughout the user flow here.

Good call! 

Wanted to show a general WIP, I'm working back towards moving the branding at the footer like a "powered by _____" sort of attribution. I know we had these designs filed in another bug but not recalling why we stepped away from that now..

Comment 14

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

Attachment: prev_branding_overview3.png

Pulling this back once again for V1, let's just get some footer attribution going alongside the colored active state.

With this approach, we will have to be smart with the "typing phase" and show suggestions over the branding icon if/when there are a lot of suggested items. I'm picturing a clear 'display or not' type relationship as opposed to a "how much of this icon image should we cover" to keep it simple for now.

On IRC we also mentioned possibly changing the placeholder text to drive home this message "Search with Yahoo!", "Search with Bing" for instance. Could we try that as a part of this bug too?

Thoughts?

Comment 15

[:Margaret Leibovic]

(In reply to Anthony Lam (:antlam) from comment #14)
> Created attachment 8486126 [details]
> prev_branding_overview3.png
> 
> Pulling this back once again for V1, let's just get some footer attribution
> going alongside the colored active state.
> 
> With this approach, we will have to be smart with the "typing phase" and
> show suggestions over the branding icon if/when there are a lot of suggested
> items. I'm picturing a clear 'display or not' type relationship as opposed
> to a "how much of this icon image should we cover" to keep it simple for now.
>
> On IRC we also mentioned possibly changing the placeholder text to drive
> home this message "Search with Yahoo!", "Search with Bing" for instance.
> Could we try that as a part of this bug too?

Sure, that shouldn't be too hard to do.

> Thoughts?

I'll give this a shot. My main concern is that we're taking up some vertical space, but hiding the icon if there isn't room for it seems like a reasonable solution there. Let's just try this, and we can iterate.

It may be a bit hard to change the color of the underline, since that's created in a background drawable, but I'll see what I can do there.

Comment 16

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

Attachment: prev_branding_overview4.png

Going back to branding on top right for V1. 

After feedback and discussing with Margaret some more, it offers the best blend of least amount of issues when dealing with states where keyboard is up, and most visible branding/attribution.

I feel like we're spinning our wheels a bit here. Let's try this in practice and iterate from there.

Comment 17

[:Margaret Leibovic]

Attachment: Show search engine branding

This is broken up into multiple commits in the PR here, but I was too lazy to `grunt export` each of those individually :)
https://github.com/mozilla/fennec-search/pull/91

Outstanding issues that need to be addressed:
* EditText#setBackground is API level 16+. I tried using the deprecated setBackgroundDrawable in an if statement for older versions, but my build was failing with an error about deprecation, and I need to investigate how to fix that. 
* Need to add support for getting colors from search plugins. I'm going to file a separate bug to do this, since it will involve modifying the search plugin XML.
* Need to address the issue of search engine icon resolution, but I think that should also go in a separate bug, since a fix for that would involve updating the search plugins (and this is an issue we've had in Fennec for a long time, so I'm worried it might not be totally straightforward to deal with).

Comment 18

[:Margaret Leibovic]

Comment on attachment 8489748 [details] [diff] [review]
Show search engine branding

liuche can also review this. You two can race to see who reviews it first! :)

Comment 19

[Wesley Johnston (:wesj)]

Comment on attachment 8489748 [details] [diff] [review]
Show search engine branding

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

I think this seems fine. Its a bit hard to review because of the way it removed/added files to hg.

::: mobile/android/search/java/org/mozilla/search/autocomplete/ClearableEditText.java
@@ -22,5 @@
> -import org.mozilla.gecko.Telemetry;
> -import org.mozilla.gecko.TelemetryContract;
> -import org.mozilla.search.R;
> -
> -public class ClearableEditText extends FrameLayout {

Is this an hg move? It looks like a delete/add.

::: mobile/android/search/java/org/mozilla/search/autocomplete/SearchBar.java
@@ +129,5 @@
> +    public void setEngine(SearchEngine engine) {
> +        int color = engine.getColor();
> +        if (color == Color.TRANSPARENT) {
> +            // Fall back to default orange if the search engine doesn't specify a color.
> +            color = getResources().getColor(R.color.highlight_orange);

Why not just have getColor return orange for you?

::: mobile/android/search/res/layout/search_bar.xml
@@ +1,5 @@
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +
> +<merge xmlns:android="http://schemas.android.com/apk/res/android">

Same move/add here?

Comment 20

[:Margaret Leibovic]

(In reply to Wesley Johnston (:wesj) from comment #19)
> Comment on attachment 8489748 [details] [diff] [review]
> Show search engine branding
> 
> Review of attachment 8489748 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think this seems fine. Its a bit hard to review because of the way it
> removed/added files to hg.

Yeah, I'm sorry about that, I think that's a problem with our `grunt export

Comment 21

[:Margaret Leibovic]

Gr, hit enter by accident...

(In reply to :Margaret Leibovic from comment #20)

> > I think this seems fine. Its a bit hard to review because of the way it
> > removed/added files to hg.
> 
> Yeah, I'm sorry about that, I think that's a problem with our `grunt export

` approach. If you look at the pull request, you can see the individual commits, which should be easier to review.

If I had exported each of the commits individually, I think I could have done the rename in hg.

Comment 22

[:Margaret Leibovic]

Just noticed I didn't respond to this...

(In reply to Wesley Johnston (:wesj) from comment #19)

> ::: mobile/android/search/java/org/mozilla/search/autocomplete/SearchBar.java
> @@ +129,5 @@
> > +    public void setEngine(SearchEngine engine) {
> > +        int color = engine.getColor();
> > +        if (color == Color.TRANSPARENT) {
> > +            // Fall back to default orange if the search engine doesn't specify a color.
> > +            color = getResources().getColor(R.color.highlight_orange);
> 
> Why not just have getColor return orange for you?

I thought about that, but I decided that SearchEngine shouldn't be worried about the UI, it should just be providing access to the data stored in the search plugins. It may be that we'll want to use the brand color elsewhere, but default to something other than orange.

I also just added a commit that fixes some things for gingerbread:
https://github.com/leibovic/fennec-search/commit/92be8c8c33f3f6b136068ee10ded7bc1f3880401

I'll upload a new patch that does the hg renames instead, since that's better for hg history. It turns out it wasn't that hard - I just did the hg renames manually, then `grunt export` afterwards.

Comment 23

[:Margaret Leibovic]

Attachment: Show search engine branding (with hg rename)

Comment 24

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/a482af02dd95
https://github.com/mozilla/fennec-search/commit/0b35fdadae29badb3a243496aad129881d71c68e

Comment 25

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/a482af02dd95