1094450 - [rtl][ringtones] CSS changes

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Ringtones, defect)

Description

[Jim Porter (:squib)]

The ringtones app needs some CSS for RTL. Blah blah.

Comment 1

[Jim Porter (:squib)]

Attachment: Pull request

This mostly works. There's something funky about the checkbox in the share activity, but I think that's a building blocks issue.

Comment 2

[Jim Porter (:squib)]

Comment on attachment 8517723 [details] [review]
Pull request

Ok, this is done.

Comment 3

[Jim Porter (:squib)]

Comment on attachment 8517723 [details] [review]
Pull request

Fernando: You reviewed bug 1082494, and I'm modifying something from the shared CSS in that bug. Can you take a quick look at this?

Katie: Mind taking a quick pass over this to make sure everything is in the right spot? I think I've handled everything inside the ringtones app, and it looks like the ringtones bits in the settings app are already ok.

Comment 4

[David Flanagan [:djf]]

Comment on attachment 8517723 [details] [review]
Pull request

Passing the review request on to Punam who will be working on RTL compliance for Gallery, so is better qualified to review this than I am.

Comment 5

[Fernando Campo (:fcampo)]

Comment on attachment 8517723 [details] [review]
Pull request

Hi Jim,

even though the changes to switches look ok to me, I'm not entitled to make this review. The code modifications in shared/switches.css affects a lot of apps, and should be reviewed by someone with better knowledge about the building blocks. 

I'm using Arnau, who usually review Building Blocks.

Comment 6

[Jim Porter (:squib)]

Thanks for the review redirection.

I also fixed some of the styles since I broke them. And, I would just like to say: I hate CSS.

Comment 7

[katieC]

Comment on attachment 8517723 [details] [review]
Pull request

ui+ review - looks good, matches ringtone behaviour and rtl spec. NI'ing Rob to take a quick pass as he was heading up the rtl media effort on the ux side.

Comment 8

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Comment on attachment 8517723 [details] [review]
Pull request

The BB part looks good to me, nothing breaks in other rtl layouts.

Comment 9

[Punam Dahiya [:pdahiya]]

Comment on attachment 8517723 [details] [review]
Pull request

Hi Jim
Patch looks good and has my r+. One minor thing noticed in the message notification indicating success on creating new ringtone. 

In RTL mode, success notification message '<file name> Ringtone set as default.'  has period showing on the right and as per spec should show up on left. Is this  a fix in notifications and outside scope of ringtone app? If yes, it will be good to file a bug for it. Thanks

Comment 10

[Jim Porter (:squib)]

(In reply to Punam Dahiya from comment #9)
> In RTL mode, success notification message '<file name> Ringtone set as
> default.'  has period showing on the right and as per spec should show up on
> left. Is this  a fix in notifications and outside scope of ringtone app? If
> yes, it will be good to file a bug for it. Thanks

I think that's just how punctuation works in Unicode when you force RTL on. It happens in a few other spots across Firefox OS too.

Comment 11

[Jim Porter (:squib)]

Landed: https://github.com/mozilla-b2g/gaia/commit/80f5d4543e1216ecfe23a0ec631257a0addc0d20

in-testsuite- because this is just a CSS change

Comment 13

[Lancy(Leave from Mozilla)]

Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15501/

Comment 14

[Sue]

Attachment: verify_pass.png

This issue has been verified as pass on latest build of Flmae 2.2/3.0 and Nexus 5 2.2/3.0 by STRs in comment 0.
Result:The CSS of ringtones app follows RTL.
See attachment:verify_pass.png
Rate:0/5

Device: Flame 2.2 (pass)
Build ID               20150521002508
Gaia Revision          bc42fbc12d622bffd7e8afcb8d56f8a1d9773c60
Gaia Date              2015-05-20 22:32:56
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6e4eaf59efda
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150521.040918
Firmware Date          Thu May 21 04:09:27 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 (pass)
Build ID               20150521160241
Gaia Revision          1126d8bee559f7cde675df2fcc6c196da9cfeba1
Gaia Date              2015-05-21 21:23:56
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/3e737d30f842
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150521.193021
Firmware Date          Thu May 21 19:30:31 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 2.2 (pass)
Build ID               20150521002508
Gaia Revision          bc42fbc12d622bffd7e8afcb8d56f8a1d9773c60
Gaia Date              2015-05-20 22:32:56
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6e4eaf59efda
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150521.040628
Firmware Date          Thu May 21 04:06:43 EDT 2015
Bootloader             HHZ12f

Device: Nexus 5 3.0 (pass)
Build ID               20150521160241
Gaia Revision          1126d8bee559f7cde675df2fcc6c196da9cfeba1
Gaia Date              2015-05-21 21:23:56
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/3e737d30f842
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150521.193039
Firmware Date          Thu May 21 19:30:54 EDT 2015
Bootloader             HHZ12f