1118893 - [RTL][Email] Update carat styling

Status: VERIFIED FIXED

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

Description

[Yeojin Chung [:YeojinC]]

Attachment: Carats.png

Description:
Carats in "New Account", "Mail Settings", and Settings pages still exists. According to p.13 on Bidirectional Guidelines, carats in list items should be removed to avoid conflicting with the screen load transitions(https://mozilla.app.box.com/s/0y1amh4rwpp6brcxd1hk). 
   
Repro Steps:
1) Update a Flame device to BuildID: 20150106010234.
2) Set the device language in Arabic under Settings > Language.
3) Open Email app, and set up an account.
4) After logging in, observe the carats with "Sent Using Firefox OS.
5) Select "Next" > "Continue to Mail".
6) Open the drawer, and select the settings icon.
7) Observe the carat with the email address.
8) Select the email account, and observe the carats on list items.
  
Actual:
Carats are mirrored and still existing.
  
Expected: 
Carats are removed.
  
Environmental Variables:
Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20150107010216
Gaia: 69ac77cfa938fae2763ac426a80ca6e5feb6ad25
Gecko: 33781a3a5201
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
  
Repro frequency: 100%
See attached: screenshot

Comment 1

[James Burke [:jrburke]]

Asking swilkes for confirmation: when we talked about this on the first set of email RTL changes, this was still an issue under UX discussion, so I left the chevrons in there until something was final. It seemed odd to not give the user any indication that some list items had another card of UI if you tapped on them. 

Did those UX discussions end up deciding not to show any indication?

Comment 2

[Stephany Wilkes]

Flagging Rob per our IRC chat yesterday, as this is a Frameworks decision we need ASAP.

Comment 3

[Dylan Oliver [:doliver]]

RTL triage: P2 -- will make a best effort to get this into the 2.2 release.

Comment 4

[Rob MacDonald [:robmac]]

FYI - The frameworks team is meeting tomorrow (Jan 22) and I've added this to the agenda.

Comment 5

[Delphine Lebédel [:delphine - Use needinfo]]

Hi. I'm not sure this should be a P2. I would consider this as a P1. This is at the time very confusing and inconsistent across apps

Comment 6

[Stephany Wilkes]

Feature landing is in two days. We either need a solution ASAP or agree to not change this.

Comment 7

[Delphine Lebédel [:delphine - Use needinfo]]

Stephany: I am confused. For me, Feature Landing is next month, as per https://wiki.mozilla.org/Release_Management/FirefoxOS/2_2_Schedule

Comment 8

[Stephany Wilkes]

Disregard! FL is 2/23 - thank heavens.

Comment 9

[Dylan Oliver [:doliver]]

RTL update: marking required bugs as feature-b2g:2.2+ (and removing blocking flags)

Comment 10

[Eric Pang [:epang] UX]

Attachment: Settings-Carats.jpg

Mock up of updated carats.  I'll be attaching carat assets shortly.  The positioning of the carat is the same as the current.

Comment 11

[Eric Pang [:epang] UX]

Attachment: carat assets.zip

Assets for carats in 1x, 1.5x, 2x and 2.25x.
If there are any questions please let me know.  Please also flag me for ui-review when ready, thanks!

Comment 12

[Stephany Wilkes]

James, let me know if you can take this bug assignment, or who might be best to do it. We may need to assign multiple people across the apps for this one, depending on all of the places it shows up. Thanks!

Comment 13

[James Burke [:jrburke]]

Thanks :epang!

swilkes: I can do this change for email. Is this carat only to be used in the RTL situation, or should it also replace the carat used in the LTR version of those list items? 

For styling reuse and slightly smaller zip file reasons, it would be nice if it was used for both cases, but also not a big deal if it should just be used for RTL only.

Comment 14

[Stephany Wilkes]

"New carat everywhere" question is for Rob/UX Frameworks team. My answer would be yes. Rob?

Comment 15

[Rob MacDonald [:robmac]]

I know what I'd say but NI'ing Patryk as this is more of a visual question. Patryk?

Comment 16

[Patryk Adamczyk [:patryk] UX]

Attachment: Settings-Carats-regular.jpg

Ok, lets use the new smaller caret within the entire OS, see the mock up on the right as an example.

Comment 17

[GitHub Autolander Bot]

Attachment: [PullReq] jrburke:bug1118893-email-list-carat to mozilla-b2g:master

Comment 18

[James Burke [:jrburke]]

Comment on attachment 8558216 [details] [review]
[PullReq] jrburke:bug1118893-email-list-carat to mozilla-b2g:master

This pull request uses the new carats, see the pull requests for screenshots of them in use. If it passes visual design review, then I will proceed with dev review.

Comment 19

[Patryk Adamczyk [:patryk] UX]

Hey James, thanks. 
We'd want the carets in the button also to use the shrunken down new caret.
All instances of the larger caret should use the new smaller caret.

Comment 20

[James Burke [:jrburke]]

Comment on attachment 8558216 [details] [review]
[PullReq] jrburke:bug1118893-email-list-carat to mozilla-b2g:master

I updated the pull request to fix the carat in the button, updated screenshots are in this pull request comment:

https://github.com/mozilla-b2g/gaia/pull/27865#issuecomment-72964143

Note that these changes only affect the email app -- these were all custom uses of the carats. So, shared components are not affected by this change.

Comment 21

[Dylan Oliver [:doliver]]

Patryk -- just to make sure you see the point in comment 20, you'll need to open up bugs in the other relevant apps to get the new carat everywhere.

Comment 22

[Arthur Chen [:arthurcc][inactive after 6/30]]

There are apps using gaia-icons (forward-light.svg) to display the caret, so the new design should also be included in gaia-icons. Eric, could you share the svg version of it?

Comment 23

[Patryk Adamczyk [:patryk] UX]

Ok Dylan, Eric (epang) will do an audit and file the bugs for the all the other instances. Thanks.

Comment 24

[Eric Pang [:epang] UX]

Comment on attachment 8558216 [details] [review]
[PullReq] jrburke:bug1118893-email-list-carat to mozilla-b2g:master

Hi James,

Thanks for working on this.  The icon looks fuzzy in the screen shots, I'm worried it's the asset I provided. 

I'm planning to work on the audit tomorrow.  If I export an svg can it be used instead of the pngs?  I'll also provide a spec.  Would be great if we can get all apps to use the same shared asset in the set of gaia icons.

Arthur, I'll get the icon to you tomorrow, do you have a link to the current icon?

Comment 25

[Arthur Chen [:arthurcc][inactive after 6/30]]

Eric, here is the link of the current icon that we are using in settings app [1].

[1]: https://github.com/gaia-components/gaia-icons/blob/master/images/forward-light.svg

Comment 26

[Eric Pang [:epang] UX]

Attachment: forward-light.svg

Hi James, I've created a svg version of the caret to replace the one in gaia-icons.

I've opened a meta to have it updated in other apps (1131641).  Is it possible to use the shared svg once it's been updated?

Updated carat specs:

Height 1.3 rem on HVGA 
Colour: #667174
Opacity: 70%
Margins: Same as current

Comment 27

[Stephany Wilkes]

James, please switch the graphic for all cases. The "bonus" of this graphic is that it works a bit better with the header carat not changing in RTL, but it's easier to use it everywhere all the time.

Comment 28

[Stephany Wilkes]

I'm wondering - should we resolve this one invalid (since it should be that new carats are seen, not that they are gone)?

Comment 29

[Dylan Oliver [:doliver]]

Let's just try a new title since we already have the long & winding history in this bug and it should be closed out soon.

Comment 30

[James Burke [:jrburke]]

Comment on attachment 8558216 [details] [review]
[PullReq] jrburke:bug1118893-email-list-carat to mozilla-b2g:master

Hi Eric, I put in the SVG icon into email, here is how it looks now:

https://github.com/mozilla-b2g/gaia/pull/27865#issuecomment-74357197

Comment 31

[Eric Pang [:epang] UX]

Comment on attachment 8558216 [details] [review]
[PullReq] jrburke:bug1118893-email-list-carat to mozilla-b2g:master

the icon is looking thicker then it should.  I think it might have some to do with the 30 x 30 px size.  Pavel can probably help figure out what's going on since he implemented the new carats in the other apps what did you do different with the other bugs? Thanks!

Comment 32

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

Hey James,

see my comments on github:
https://github.com/mozilla-b2g/gaia/pull/27865/files#diff-dcbffab9bed0fd786a4cd742c9c06e35R153
https://github.com/mozilla-b2g/gaia/pull/27865/files#diff-dcbffab9bed0fd786a4cd742c9c06e35R350

you can use this SVG (already in master):
https://github.com/mozilla-b2g/gaia/blob/master/shared/style/buttons/images/forward-light.svg

if you need info or help for something pls ping me :)

Comment 33

[James Burke [:jrburke]]

Comment on attachment 8558216 [details] [review]
[PullReq] jrburke:bug1118893-email-list-carat to mozilla-b2g:master

Thanks :pivanov for the help. I used it along with looking at what settings did, here is the latest state of it now:

https://github.com/mozilla-b2g/gaia/pull/27865#issuecomment-75308978

Comment 34

[Eric Pang [:epang] UX]

Comment on attachment 8558216 [details] [review]
[PullReq] jrburke:bug1118893-email-list-carat to mozilla-b2g:master

Looks great, thanks James and Pavel! R+

Comment 35

[James Burke [:jrburke]]

Comment on attachment 8558216 [details] [review]
[PullReq] jrburke:bug1118893-email-list-carat to mozilla-b2g:master

Moving on to dev review: just a CSS change. See my last comment in the pull request for how it looks.

Comment 36

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

Comment on attachment 8558216 [details] [review]
[PullReq] jrburke:bug1118893-email-list-carat to mozilla-b2g:master

r+ by inspection and things seeming reasonable CSS-wise and ux-review having happened based on the pretty screenshots.

Comment 37

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/d5b6fe8fde0516e96b39a8c751d0e2ed272dd4de

Comment 38

[James Burke [:jrburke]]

Comment on attachment 8558216 [details] [review]
[PullReq] jrburke:bug1118893-email-list-carat to mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
Change desired for RTL to have indicators that there are sub-screens, but without the bigger thicker carats used before, tracked in meta bug 1131641.

[User impact] if declined:
The desired look of these carats will not be part of the RTL effort.

[Testing completed]:
Tested on device. Screenshots in the pull request show the results, which got ui-review+.

[Risk to taking this patch] (and alternatives if risky):
Very low, just a CSS change. It depends on bug 1131665 landing first on 2.2 though, as that bug has the image used in this CSS.

[String changes made]:
None

DO NOT LAND ON 2.2 until bug 1131665 lands there first.

Comment 39

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/349a947775d5eb361a88bd5e3b651327511c1204

Comment 40

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue is verified fixed on the latest Nightly Flame 3.0 and 2.2 builds.  
	
Actual Results: The email app now uses a smaller Carat symbol.

Environmental Variables:
Device: Flame 3.0 KK (319MB)(Full Flash)
BuildID: 20150225010244
Gaia: f6bfd854fe4746f21bc006eac145365e85f98808
Gecko: 0a8b3b67715a
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Environmental Variables:
Device: Flame 2.2 KK (319MB)(Full Flash)
BuildID: 20150225002505
Gaia: ca64f2fe145909f31af266b1730874051ba76c78
Gecko: 16804008c29f
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0