809076 - [UX VD] Drawer. Contact icon should not be there, is not a valid button.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect, P3)

Description

[Victoria Gerchinhoren [:vicky]]

The "contact" icon next to the Email users name is not correct, is not ment to have an icon there, and if needed is not that one. Please specify why is it placed there.

Comment 1

[Steve Chung [:steveck]]

Hi Peter,
Based on the https://www.dropbox.com/sh/ygwfxk6chpshxdj/ct4nwRk2r3/Apps/Email/mail-settings.pdf
We will need an account button for account selection. Is there any icon for the account button? Thanks.

Comment 2

[Casey Yee]

This should be changed to a "<" button that we use in other headers.

http://mozilla-b2g.github.com/Gaia-UI-Building-Blocks/index.html#http://mozilla-b2g.github.com/gaia/shared/style/headers/

Comment 3

[Peter La]

Hi Steve,

Yes, as Casey said, this should be the back arrow we use in the other headers.

I do find it still feels odd in that you tap "<" to go to accounts, which makes the screen pan right (this is fine), but then when you tap "<" again, the screen pans to the left to go back...

Steve, can you make the arrow swap?  Please use a common component if you can.

Comment 5

[Steve Chung [:steveck]]

Attachment: Patch for applying the correct button image

NOTE: If blocking-basecamp+ is set, just land it for now.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: User will see the button with misleading icon.
Testing completed: 
Risk to taking this patch (and alternatives if risky): None

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 711233 [details]
Patch for applying the correct button image

Go ahead with uplift once this is reviewed +

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

It has been decided that the UX bugs will track, not block for v1.0.1 and we'll consider uplift (please nominate when review is given) but we'll have to discuss targeting v1.1 depending on risk/reward.

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Peter La from comment #3)
> Steve, can you make the arrow swap?  Please use a common component if you
> can.

There's no explicit building block style for this.  There is a back-rtl.png, but that's only referenced by an rtl selector, so I don't think we want to hard-code that hack in and then get regressed if building blocks changes internally.

We can do "transform: scaleX(-1)" but I'm a little worried about such a hack having a performance impact on us.  Can you get the building blocks updated to have an explicit icon-forward or something like that?

I'll r+ this as-is since Casey is in charge of this call.

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

landed on gaia/master:
https://github.com/mozilla-b2g/gaia/pull/8005

Comment 10

[Alex Keybl [:akeybl]]

Comment on attachment 711233 [details]
Patch for applying the correct button image

Critical & low risk TEF UX change, approving for v1.0.1.

Comment 11

[John Ford [:jhford] CET/CEST Berlin Time]

This commit does not apply cleanly to v1-train.  If this patch depends on another bug, please comment here and I will retry when that bug is approved to land on all branches that this bug needs to land on.  If the merge conflict needs to be resolved by hand, the following commands could be a useful starting point:

cd gaia
git checkout v1-train
git cherry-pick -x -m1 8024254338e230e8635602ce0015beb4e9bb28ef
<resolve merge conflict>

Comment 12

[alberto.pastor]

Wow, was not easy to find out dependencies here, but finally I got it:

It seems that you need to apply Bug 798258, that needs Bug 798292 to apply,  in v1-train for cleanly apply this one.

So after applying in v1:

git cherry-pick d6cae68b2b7396b4708eab765b5c8164b5a75091
git cherry-pick 11c850e81064996147704aeef94c3043a3f9240a

your STR will cleanly apply.

I wouldn't say that is dependency, cause the whole problem comes when cleaning a trailing space at the end of a line that was inserted by the Commits of the previous bugs. I don't know what's the better way of fixing this...

Ideas?

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

Alberto and I discussed this on IRC.  Alberto merged the patch by hand, thanks!

v1-train: b31a0fa9fa27638c8eb14c7c4d8abfd5511cb310

Comment 14

[Andrew Sutherland [:asuth] (he/him)]

So, it looks like we didn't want one of those images to be removed yet since we haven't uplifted the fix for bug 808576 that changes us to use the built-in spinners.

Comment 15

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Andrew Sutherland (:asuth) from comment #14)
> So, it looks like we didn't want one of those images to be removed yet since
> we haven't uplifted the fix for bug 808576 that changes us to use the
> built-in spinners.

Problem solved by way of uplifting bug 808576.

Comment 16

[Lukas Blakk [:lsblakk] use ?needinfo]

Batch edit: bugs fixed on b2g18 since 1/25 branch of v1.0 are fixed on v1.0.1