1064461 - Clean up snippet visuals

Status: RESOLVED WORKSFORME

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

Description

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

Attachment: prev_snippet_changesearchengine1.png

Our snippet's UI could use a little bit of a clean up in the structure to be more consistent.

Simple hit area/ icon area stuff. Will post a mock up with specs later. Attaching example snippet for the time being.

Seems like a quick win.

Comment 1

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

Attachment: prev_snippetspec.png

Attaching spec!

Comment 2

[Jalpreet Singh Nanda [:imjalpreet]]

I would like to take this bug. 
Can you give a brief summary on what exactly needs to be done in this?

Comment 3

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

This looks like a more overall clean up of the visuals like copy and padding, whereas bug 966654 was specifically about address issues of the 'x' size. 

We could split up the work into these two different bugs to help track that way if you'd like.

Comment 4

[Jalpreet Singh Nanda [:imjalpreet]]

Hi, So how are we going to spilt the work into these two bugs? Can you please elaborate what all changes should I do in both the bugs separately. Thanks :)

Comment 5

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

Attachment: spec_snippet_mock2.png

I was thinking we could split up the "close button hit are is too small" into bug 966654 and leave this one for everything else.

Bug 966654: 
 - Enlarge hit area to hit the edges of the snippet

This bug:
 - tweak padding of everything else
 - make sure all the measurements line up

Not sure if this is necessary, going to NI Margaret here to see what she thinks.

Comment 6

[:Margaret Leibovic]

(In reply to Anthony Lam (:antlam) from comment #5)
> Created attachment 8536478 [details]
> spec_snippet_mock2.png
> 
> I was thinking we could split up the "close button hit are is too small"
> into bug 966654 and leave this one for everything else.
> 
> Bug 966654: 
>  - Enlarge hit area to hit the edges of the snippet
> 
> This bug:
>  - tweak padding of everything else
>  - make sure all the measurements line up
> 
> Not sure if this is necessary, going to NI Margaret here to see what she
> thinks.

Thanks for the mock-up! This clarifies what I've been trying to say in bug 966654, which is that we need the tap area around the close button to extend to the edges of the banner. We can just implement this change over in that bug.

My one piece of feedback on this mock-up is that currently the icon on the left of the banner is 48dp, and I think we should keep that size because this affects the image sizes that the snippets team can use. Can we keep the icon 48dp, but and then reduce the margins around it?

Comment 7

[Jalpreet Singh Nanda [:imjalpreet]]

Now, what other changes should I do in this bug except the margin changes?

Comment 8

[:Margaret Leibovic]

(In reply to Jalpreet Singh Nanda [:imjalpreet] from comment #7)
> Now, what other changes should I do in this bug except the margin changes?

I think we should wait for a response from antlam about the icon size. But yes, it looks like the only thing left here will be updating the margins around the icon.

Comment 9

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

(In reply to :Margaret Leibovic from comment #8)
> (In reply to Jalpreet Singh Nanda [:imjalpreet] from comment #7)
> > Now, what other changes should I do in this bug except the margin changes?
> 
> I think we should wait for a response from antlam about the icon size. But
> yes, it looks like the only thing left here will be updating the margins
> around the icon.

Ah, good point. sure - let's try that. 

That would make the margins there go from 18dp to 10dp on each side.

Comment 10

[Jalpreet Singh Nanda [:imjalpreet]]

Then, I think we dont need to change anything, as the margins are already 10dp or is there anything else to change ?

Comment 11

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

Should be good then.

Comment 12

[Jalpreet Singh Nanda [:imjalpreet]]

So, is there any other change to be done?

Comment 13

[:Margaret Leibovic]

(In reply to Jalpreet Singh Nanda [:imjalpreet] from comment #12)
> So, is there any other change to be done?

Nope, sounds like this bug is solved! Thanks for your help investigating. I'm sorry there wasn't more to do here, but verifying things like this is just as important as writing patches :)

Comment 14

[Jalpreet Singh Nanda [:imjalpreet]]

Happy to help! :)