939153 - [Action Icons - SMS] Remove shadows from action icons

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect, P1)

Description

[Amy Lee [:amylee] UX]

Hi Pavel,

Please replace the action icons in this app with the ones that I'll be providing (I'll be attaching new icons to this bug). This is a blocker for Bug 939106. 

Thanks!

Comment 1

[Eric Pang [:epang] UX]

Attachment: sms.zip

Hi Pavel,

Here are the sms icons, I've replaced them within the folder structure
gaia/apps/sms...

Thanks!

Comment 2

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: patch for Gaia/master

I remove few icons because we don't use them

Comment 3

[Rick Waldron [:rwaldron]]

LGTM

Comment 4

[Eric Pang [:epang] UX]

Comment on attachment 833468 [details] [review]
patch for Gaia/master

Looks good Pavel!  Hey Kevin, noticed that you said it looked good to you, I've flagged you for review now, can you give us an r+?? :) Thanks!!

Comment 5

[Kevin Grandon :kgrandon]

Comment on attachment 833468 [details] [review]
patch for Gaia/master

I know we're basically twins, but I think you may have meant Rick here - so redirecting the review. :)

Comment 6

[Kevin Grandon :kgrandon]

Ah - I think my mailbox was also threading these bugs under the same root email. That's probably where the confusion comes from :)

Comment 7

[Eric Pang [:epang] UX]

(In reply to Kevin Grandon :kgrandon from comment #6)
> Ah - I think my mailbox was also threading these bugs under the same root
> email. That's probably where the confusion comes from :)

Whoops sorry about that Kevin! :) Thanks for re assigning!

Comment 8

[Julien Wajsberg [:julienw]]

Comment on attachment 833468 [details] [review]
patch for Gaia/master

Rick is busy this week, I'm redirecting to Borja.

Comment 9

[Eric Pang [:epang] UX]

(In reply to Julien Wajsberg [:julienw] from comment #8)
> Comment on attachment 833468 [details] [review]
> patch for Gaia/master
> 
> Rick is busy this week, I'm redirecting to Borja.

Thanks for letting us know and reassigning Julien!

Comment 10

[Borja Salguero [:borjasalguero]]

Hi! Small doubt in Github
https://github.com/mozilla-b2g/gaia/pull/13779#issuecomment-28985112

Could you take a look? THanks!

Comment 11

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Hey :),
sure I think that we fix this in Bug 939152

Comment 12

[Eric Pang [:epang] UX]

(In reply to Pavel Ivanov [:ivanovpavel] from comment #11)
> Hey :),
> sure I think that we fix this in Bug 939152
Thanks Borja! I noticed the same thing, as Pavel mentioned in comment 11 some of the icons are pulled from the shared folder :).  So the combination of the bug 939152 and this bug removes the drop shadows from the app :)

Comment 13

[Borja Salguero [:borjasalguero]]

Attachment: Settings' shadow

As you can see there is a shadow in the settings icon... Is there any bug filed for fixing this?

Comment 14

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Yep Casye works on this (Bug 941620)

Comment 15

[Eric Pang [:epang] UX]

(In reply to Borja Salguero [:borjasalguero] from comment #13)
> Created attachment 8337566 [details]
> Settings' shadow
> 
> As you can see there is a shadow in the settings icon... Is there any bug
> filed for fixing this?

Hi Borja, looks like this icon was added to the shared folder recently.  Thanks for point it out.  I've opened a follow up bug to address is 943321.  This icon does not effect this bug, so if everything else is good can we get an r+?  Thanks!

Comment 16

[Eric Pang [:epang] UX]

Comment on attachment 833468 [details] [review]
patch for Gaia/master

We resolved the issue with the options button, so we'll all good.  Let's land this!

Comment 17

[Borja Salguero [:borjasalguero]]

Comment on attachment 833468 [details] [review]
patch for Gaia/master

Taking a look before merging! :)

Comment 18

[Borja Salguero [:borjasalguero]]

Comment on attachment 833468 [details] [review]
patch for Gaia/master

Now all icons are working as expected! Thanks for the quick fix! :)

Comment 19

[Borja Salguero [:borjasalguero]]

https://github.com/mozilla-b2g/gaia/commit/a02379741e587dfb3c2763818b233429dc71f677
https://github.com/pivanov/gaia/commit/f9df1271d881a4bba5d4b387978b6681522371c3
R+. Merged!