Closed Bug 1101772 Opened 6 years ago Closed 6 years ago

[RTL] Tile view need the RTL ordering.

Categories

(Firefox OS Graveyard :: Gaia::Music, defect, P2)

x86_64
Linux
defect

Tracking

(b2g-v2.1 wontfix, b2g-v2.1S wontfix, b2g-v2.2 verified, b2g-master verified)

VERIFIED FIXED
2.2 S4 (23jan)
Tracking Status
b2g-v2.1 --- wontfix
b2g-v2.1S --- wontfix
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: hub, Assigned: hub)

References

Details

(Whiteboard: [ft:media])

Attachments

(3 files)

The tile view isn't ordered rgith to left in RTL locale as per the UX spec.
Blocks: music-rtl
Assignee: nobody → hub
Status: NEW → ASSIGNED
Target Milestone: --- → 2.1 S9 (21Nov)
Target Milestone: 2.1 S9 (21Nov) → 2.2 S1 (5dec)
feature-b2g: --- → 2.2+
Whiteboard: [ft:media]
This change is done in the JS instead of CSS because adding float to the CSS trigger a linter error:

 Too many floats (10), you're probably using them for layout. Consider using a grid system instead. line undefined, col undefined 


Thank you kindly
Attachment #8528576 - Flags: review?(dkuo)
feature-b2g: 2.2+ → ---
Hi Dominic - Bhavana and I reviewing RTL 2.2 bugs to continue incremental improvements. Please let us know if this is good to go. Thanks!
Flags: needinfo?(dkuo)
Comment on attachment 8528576 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26457

Hubert, sorry for the late review, I think before fixing the RTL part, the css of the tiles needed to be refine first, then we should have a easy way to address the RTL issue, without using the JavaScript to control the layout. Please read my github comment and let me know if you have questions, thanks.
Flags: needinfo?(dkuo)
Attachment #8528576 - Flags: review?(dkuo)
delhpine, can you please help test this ? Per triage we mentioned there may be other changes that have landed that may have improved the situation.

Hub OTOH, can we just try to get a closure on this for 2.2 if we are close to resolving this ?
Flags: needinfo?(lebedel.delphine)
Flags: needinfo?(hub)
Sorry I seem to have dropped the ball on that. I'll get right on it.
Flags: needinfo?(hub)
Comment on attachment 8528576 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26457

Dominic,

I changed the alignment to use CSS. And added RTL support at the same time.
Attachment #8528576 - Flags: review?(dkuo)
Comment on attachment 8528576 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26457

Thanks Hub, the patch now looks good to me, there is only a minor glitch and please read the github comments then just address it before landing it.
Attachment #8528576 - Flags: review?(dkuo) → review+
I fixed the glitch.

Will land it once green. Thanks !
Merged
https://github.com/mozilla-b2g/gaia/commit/0fbf2922da2889147f70f5e12500594d4d9cb446
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Comment on attachment 8528576 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26457

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): RTL support
[User impact] if declined: tiles won't match the spec
[Testing completed]: manually tested with ltr and rtl locales
[Risk to taking this patch] (and alternatives if risky): none
[String changes made]: none
Attachment #8528576 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8528576 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
RTL triage: P2 -- will make a best effort to get this into the 2.2 release.
Priority: -- → P2
(In reply to bhavana bajaj [:bajaj] from comment #4)
> delhpine, can you please help test this ? Per triage we mentioned there may
> be other changes that have landed that may have improved the situation.
> 
> Hub OTOH, can we just try to get a closure on this for 2.2 if we are close
> to resolving this ?

Hi Bhavana,
This problem is verified pass on latest build of Flame2.2
See attachments: Flame2.2_screenshot.PNG
Rate: 0/5

Flame 2.2 build:
Gaia-Rev        e4f9b5da3751798f9cc5d95f302c30722cc11fca
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/4a90da67661e
Build-ID        20150122002808
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150122.041326
FW-Date         Thu Jan 22 04:13:37 EST 2015
Bootloader      L1TC000118D0
Flags: needinfo?(lebedel.delphine)
Flags: in-moztrap+
This issue has been verified successfully on Flame 3.0.
Reproduce rate:0/5.
Attachment:Verify_RTL.png

Flame 3.0 build:
Gaia-Rev        ae5a1580da948c3b9f93528146b007fc4f6a712b
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/ae5d04409cd9
Build-ID        20150203055658
Version         38.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150203.093120
FW-Date         Tue Feb  3 09:31:32 EST 2015
Bootloader      L1TC000118D0
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]
Attached image Verify_RTL.png
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15195/
You need to log in before you can comment on or make changes to this bug.