1088458 - [Usage][RTL] Make Data Usage View RTL compatible

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Cost Control, defect, P2)

Description

[Stephany Wilkes]

Attachment: Multiple Cost Control issues

It seems we have a new version of bug #1044299 for 2.2. There a few layout issues on the attached Cost Control screenshot:

Set language to Arabic. Observe that:
* Text on the left side of the chart runs off the edge of the screen.
* Arabic labels and European numerals overlap at the bottom of the chart.
* Alignment of X and Y axis labels appears off.
* "Mobile App Usage" is not translated.
* "Search the web" in Rocketbar up top is not translated.

Comment 1

[Paco Rampas [:paco]]

Hello,

Comment 2

[Paco Rampas [:paco]]

Hello,

About this:
*Text on the left side of the chart runs off the edge of the screen

What should be the approach to solve it? I think the better solution is to use the acronym "MB", "GB"... like in english because the acronym in arabic is to long.

Comment 3

[Juwei Huang[:juwei]]

Yes it would be better to show "MB" "GB" instead.

For the rest of the issue, not only the sting and number should separate, some components on the screen may also need to reverse.
I'll get back later once I got more info about RTL.

Comment 4

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: not blocking.
smart data for 2.2 might cover this. I'm leaving this bug in backlog for now.

Comment 5

[Peter Dolanjski [:pdol]]

Yes, we should definitely plan for RTL support in 2.2.

Comment 6

[Paco Rampas [:paco]]

Attachment: costcontrol-arabic.png

Comment 7

[Juwei Huang[:juwei]]

Comment on attachment 8517988 [details]
costcontrol-arabic.png

Hi Paco! Nice job!!!

I just got RTL guideline here:
https://mozilla.box.com/s/0y1amh4rwpp6brcxd1hk

There are few things need to correct:
1. Text GB 1 may need to reverse back to 1 GB
According to guideline p.4, "Text reads right to left, unless they are words written in an LTR language or numeric digits"
So I think 1GB should still keep the display "1GB"

2. Icons on header don't need to reverse
Please refer to p.11

3. Mobile App Usage and it's description need to translate.
Don't know who we can contact to... need info Wesley.

Other than that, nice job! Thanks,

Comment 8

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Forwarding needinfo to Candice who takes charge of Smart Data.
I suppose we are talking about 2.2 so it will be a new design from that.

Comment 9

[Candice Serran (:cserran)]

removing flag since peter is already aware

Comment 10

[Oleg Zasypkin [:azasypkin]]

Hey Paco,

Could you please let us know if you're still going to work on this issue? If you don't have enough time right now, we're happy to help and take it over :)

Thanks!

Comment 11

[Paco Rampas [:paco]]

Hi,

Oleg feel free to toke it ;)

Thanks for your help.

Comment 12

[Oleg Zasypkin [:azasypkin]]

(In reply to Paco Rampas [:paco] from comment #11)
> Hi,
> 
> Oleg feel free to toke it ;)
> 
> Thanks for your help.

Okay, thanks for reply!

Will reduce scope for this bug to Data Usage View only.

Comment 13

[Jacob Shen]

[Tracking Requested - why for this release]:

Comment 14

[Stephany Wilkes]

Please let me know if there is anything from this bug that we expect to land in 2.2 as an incremental RTL improvement. Bhavana and I are reviewing 2.2 possible RTL candidates right now.

It's not clear to me if (per comment #12) the scope was reduced and we expect that data usage view, at least, to support RTL, or if we expect none of this to land. Either way is OK - thank you!

Comment 15

[Oleg Zasypkin [:azasypkin]]

Hey Stephany,

> Please let me know if there is anything from this bug that we expect to land
> in 2.2 as an incremental RTL improvement. Bhavana and I are reviewing 2.2
> possible RTL candidates right now.

Unfortunately this bug is out of my priority radar since RTL & SmartData isn't v2.2 blocker anymore :( So I don't have plans to land anything here at the moment.

> It's not clear to me if (per comment #12) the scope was reduced and we
> expect that data usage view, at least, to support RTL, or if we expect none
> of this to land. Either way is OK - thank you!

Initially we planned to improve RTL support view by view (one bug for one view), we did so for FTE view and I planned to do the same for Data Usage view only in this bug, but as I said above it doesn't have enough priority anymore.

Thanks

Comment 16

[Dylan Oliver [:doliver]]

Oleg, are you available to jump back into this one? RTL is back on the 2.2 priority feature list.

Comment 17

[Julien Wajsberg [:julienw]]

Oleg is at the 2-week ideation meetup in Paris (as I am) so I think it's better if you can find somebody else to work on this.

Comment 18

[Alissa（leave from Mozilla）]

hi Stephany Wilkes,
In this view,The checkbox should be left-aligned, and the color icon should be right-aligned,What do you think?

Comment 19

[Stephany Wilkes]

If this is ready for review, please re-set the ui-review? to Juwei, who owns the UX for this app. 

At first glance, it still looks like the data size is cut off: I only see "B", not MB or GB.

We should also test what this looks like with data in the chart, instead of flat line, to make sure the chart "reads" properly. 

Please note the spec link provided earlier (in comment #7) is actually not correct. This is the link:
https://mozilla.box.com/s/bcm3s5i2v6js5uk0ws3tsywse8bgncgo

Comment 20

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #17)
> Oleg is at the 2-week ideation meetup in Paris (as I am) so I think it's
> better if you can find somebody else to work on this.

Thanks for the update, Julien! Upcoming weeks are going to be quite busy, so yeah it would be better if someone else can work on this.

Comment 21

[Julien Wajsberg [:julienw]]

Oleg, I wonder if you have a look at this now?

Comment 22

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #21)
> Oleg, I wonder if you have a look at this now?

Yep, looking into it right now.

Comment 23

[GitHub Autolander Bot]

Attachment: [gaia] azasypkin:bug-1088458-usage-rtl > mozilla-b2g:master

Comment 24

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8569844 [details] [review]
[gaia] azasypkin:bug-1088458-usage-rtl > mozilla-b2g:master

Hey Marina,

Could you please take a look at the patch? I tried to follow the latest RTL spec, but I may not be aware of all app-specific agreements you have, so please let me know if something looks wrong for you :)

I left few comments at GitHub that explain some changes I propose.

Thanks!

Comment 25

[Josh Cheng [:josh]]

Dear Oleg,
Could you kindly provide ETA date (Target Milestone) for this issue? Thank you very much!

Comment 26

[Marina Rodríguez [:mai]]

Comment on attachment 8569844 [details] [review]
[gaia] azasypkin:bug-1088458-usage-rtl > mozilla-b2g:master

Looks good! Thanks

Comment 27

[Oleg Zasypkin [:azasypkin]]

(In reply to Josh Cheng [:josh] from comment #25)
> Dear Oleg,
> Could you kindly provide ETA date (Target Milestone) for this issue? Thank
> you very much!

Hey Josh, I think we should be able to handle it by EOW.

Thanks!

Comment 28

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8569844 [details] [review]
[gaia] azasypkin:bug-1088458-usage-rtl > mozilla-b2g:master

Thanks for feedback, Marina!

As we discussed, I've filed a bug 1138432 to migrate to bower gaia-checkbox web component once it becomes maturer (left comment at GitHub about that as well).

Comment 29

[Marina Rodríguez [:mai]]

Comment on attachment 8569844 [details] [review]
[gaia] azasypkin:bug-1088458-usage-rtl > mozilla-b2g:master

LGTM, only a nit. Please don't forget asking BB review for the changes of this parts.

Thanks for your effort

Comment 30

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8569844 [details] [review]
[gaia] azasypkin:bug-1088458-usage-rtl > mozilla-b2g:master

(In reply to Marina Rodríguez [:mai] from comment #29)
> Comment on attachment 8569844 [details] [review]
> [gaia] azasypkin:bug-1088458-usage-rtl > mozilla-b2g:master
> 
> LGTM, only a nit. Please don't forget asking BB review for the changes of
> this parts.
> 
> Thanks for your effort

Thanks for review! Nits fixed.

Hey Kevin,

In this patch we temporarily switched back from shared/elements/gaia_checkbox to BB checkboxes to have proper RTL support. Later on we're going to start using the latest gaia-checkbox web component [1] via bower - see bug 1138432.

CostControl was the only app that used shared/elements/gaia_checkbox and Wilson advised us to get rid of it entirely since it became obsolete if favor of [1]. So are you ok with removing of this component?

Thanks!

[1] https://github.com/gaia-components/gaia-checkbox

Comment 31

[Kevin Grandon :kgrandon]

Comment on attachment 8569844 [details] [review]
[gaia] azasypkin:bug-1088458-usage-rtl > mozilla-b2g:master

I would personally rather leave it in until we port to gaia-checkbox. I would say just land your patch without removing the code from shared/ for now?

Comment 32

[Oleg Zasypkin [:azasypkin]]

(In reply to Kevin Grandon :kgrandon from comment #31)
> Comment on attachment 8569844 [details] [review]
> [gaia] azasypkin:bug-1088458-usage-rtl > mozilla-b2g:master
> 
> I would personally rather leave it in until we port to gaia-checkbox. I
> would say just land your patch without removing the code from shared/ for
> now?

Wilson probably just wanted to avoid any future dependencies on obsoleted shared/elements/gaia_checkbox. But it doesn't really matter to me, will leave it then.

Thanks1

Comment 33

[Oleg Zasypkin [:azasypkin]]

Treeherder is green, merged!

Master: https://github.com/mozilla-b2g/gaia/commit/b487407d3fd925d3cfa5cdd97b5e74282de37974

Comment 34

[Oleg Zasypkin [:azasypkin]]

It slipped under my radar, but it probably should be uplifted to v2.2 as well. 

Hey Marina, do you want this fix for v2.2?

Comment 35

[Marina Rodríguez [:mai]]

Hi Oleg, 

Please, ask for approval, because of RTL is on the 2.2 priority feature list.

Best regards.

Comment 36

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8569844 [details] [review]
[gaia] azasypkin:bug-1088458-usage-rtl > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): n/a
[User impact] if declined: DataUsage view will have a few RTL issues (eg. checkboxes for data sources won't be reversed, minor padding and margin inconsistencies).
[Testing completed]: yes, manual
[Risk to taking this patch] (and alternatives if risky): relatively low
[String changes made]: n/a

Comment 37

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/6e6fa2c9bcb117a946a2388c9a7d8f298839b17b

Comment 38

[Sue]

Attachment: Usage.png

This issue has been verified seccussfully on Flame 2.2/3.0
* Text on the left side of the chart shown as "1GB" in Arabic language.
* Icons on header remain their position.
* The checkbox is left-aligned, and the color icon is right-aligned.
* "Mobile App Usage" is right-aligned.
* "Search the web" in Rocketbar up top is translated.
See attachment:Usage.png
Rate:0/5

Flame 2.2 build: pass
Build ID               20150317162504
Gaia Revision          306772a58335ac4cad285d27c3805090a8cc6886
Gaia Date              2015-03-17 17:12:36
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/83251e534b33
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150317.195036
Firmware Date          Tue Mar 17 19:50:45 EDT 2015
Bootloader             L1TC000118D0

Flame 3.0 build: pass
Build ID               20150317073344
Gaia Revision          738987bd80b0ddb4ccf853855388c2627e19dcc1
Gaia Date              2015-03-17 02:27:51
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/008b3f65a7e0
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150317.111450
Firmware Date          Tue Mar 17 11:15:00 EDT 2015
Bootloader             L1TC000118D0