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

VERIFIED FIXED in 2.2 S7 (6mar)

Status

defect
P2
normal
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: swilkes, Assigned: azasypkin)

Tracking

({rtl})

unspecified
2.2 S7 (6mar)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(feature-b2g:2.2+, tracking-b2g:backlog, ux-b2g:2.2, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [2.1-bug-bash] [NaBfT])

Attachments

(4 attachments)

Reporter

Description

5 years ago
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.
Whiteboard: [2.1-FC-bug-bash] → [2.1-bug-bash]
Hello,
Depends on: 1086988
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.
Flags: needinfo?(jhuang)

Updated

5 years ago
Assignee: nobody → pacorampas
blocking-b2g: --- → 2.2?
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.
Flags: needinfo?(jhuang)
triage: not blocking.
smart data for 2.2 might cover this. I'm leaving this bug in backlog for now.
blocking-b2g: 2.2? → backlog
Flags: needinfo?(pdolanjski)
Yes, we should definitely plan for RTL support in 2.2.
Flags: needinfo?(pdolanjski)
Attachment #8517988 - Flags: ui-review?(jhuang)
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,
Flags: needinfo?(whuang)
Attachment #8517988 - Flags: ui-review?(jhuang) → ui-review-
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.
Flags: needinfo?(whuang) → needinfo?(cserran)
blocking-b2g: backlog → 2.2?
blocking-b2g: 2.2? → backlog
feature-b2g: --- → 2.2?
removing flag since peter is already aware
Flags: needinfo?(cserran)
feature-b2g: 2.2? → 2.2+
QA Whiteboard: [2.2-feature-qa+]
QA Contact: jlorenzo
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!
Flags: needinfo?(pacorampas)
Hi,

Oleg feel free to toke it ;)

Thanks for your help.
Flags: needinfo?(pacorampas)
(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.
Assignee: pacorampas → nobody
Keywords: rtl
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Summary: Cost Control layout needs adjustment in Arabic. → [Usage][RTL] Make Data Usage View RTL compatible

Comment 13

5 years ago
[Tracking Requested - why for this release]:
feature-b2g: 2.2+ → ---
tracking-b2g: --- → ?
QA Whiteboard: [2.2-feature-qa+]
Reporter

Comment 14

5 years ago
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!
Flags: needinfo?(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
Flags: needinfo?(azasypkin)
Whiteboard: [2.1-bug-bash] → [2.1-bug-bash] [NaBfT]
feature-b2g: --- → 2.2+
Priority: -- → P2
Oleg, are you available to jump back into this one? RTL is back on the 2.2 priority feature list.
Flags: needinfo?(azasypkin)
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.
hi Stephany Wilkes,
In this view,The checkbox should be left-aligned, and the color icon should be right-aligned,What do you think?
Flags: needinfo?(swilkes)
Reporter

Comment 19

4 years ago
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
Flags: needinfo?(swilkes)
Depends on: 1132571
(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.
Flags: needinfo?(azasypkin)
Oleg, I wonder if you have a look at this now?
Flags: needinfo?(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.
Assignee: nobody → azasypkin
Flags: needinfo?(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!
Attachment #8569844 - Flags: feedback?(marina.rodrigueziglesias)
Status: NEW → ASSIGNED
QA Contact: jlorenzo
Dear Oleg,
Could you kindly provide ETA date (Target Milestone) for this issue? Thank you very much!
Flags: needinfo?(azasypkin)
Comment on attachment 8569844 [details] [review]
[gaia] azasypkin:bug-1088458-usage-rtl > mozilla-b2g:master

Looks good! Thanks
Attachment #8569844 - Flags: feedback?(marina.rodrigueziglesias) → feedback+
(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!
Flags: needinfo?(azasypkin)
Target Milestone: --- → 2.2 S7 (6mar)
Blocks: 1138432
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).
Attachment #8569844 - Flags: review?(marina.rodrigueziglesias)
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
Attachment #8569844 - Flags: review?(marina.rodrigueziglesias) → review+
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
Attachment #8569844 - Flags: review?(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?
Attachment #8569844 - Flags: review?(kgrandon)
(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
Treeherder is green, merged!

Master: https://github.com/mozilla-b2g/gaia/commit/b487407d3fd925d3cfa5cdd97b5e74282de37974
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
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?
Flags: needinfo?(marina.rodrigueziglesias)
Hi Oleg, 

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

Best regards.
Flags: needinfo?(marina.rodrigueziglesias)
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
Attachment #8569844 - Flags: approval-gaia-v2.2?
Attachment #8569844 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
blocking-b2g: backlog → ---

Comment 38

4 years ago
Posted image 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

Updated

4 years ago
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.