Closed Bug 1188069 Opened 7 years ago Closed 7 years ago
Refactor FTU CSS to move RTL rules next to their LTR counterparts
46 bytes, text/x-github-pull-request
|Details | Review|
To minimise regression risk, rules that override behavior for RTL should be proximate to their default/LTR counterparts - so that a change to one is more easily linked to a change to the other.
I refactored apps/ftu/style/style.css to move/remove the rules under the "RTL overrides" block at the bottom of the file. Ran through to confirm both LTR and RTL, using the following checklist: * buttons nav: should always be forward = right / back = left in both RTL & LTR (currently, spec may change) * Wifi list icons (<aside>) should be 1.5rem from the end-edge (right in LTR, left in RTL) * Date/time "change" icon buttons, should float right in LTR, float left in RTL * Import contacts button, should position 0.5rem from left edge in LTR, 0.5rem from right edge in RTL (needs left/right: auto to override buttons.css) * Privacy links in #welcome_browser - markup removed in Bug 1054121 / 941f05b949d61adc263b2bd0df4fb84f45a33ade * The last privacy link in #welcome_browser_privacy_links - markup removed in Bug 1054121 / 941f05b949d61adc263b2bd0df4fb84f45a33ade * #form_share_statistics span:after - uses gaia-checkbox as of Bug 1176905. I removed these rules * Paragraph content in #form_share_statistics - uses gaia-checkbox as of Bug 1176905. I removed these rules * Privacy menu-item:before, should position left in LTR, right in RTL * Date/time select/input list items, should be 1.5rem from end-edge
Assignee: nobody → sfoster
Target Milestone: --- → FxOS-S4 (07Aug)
Comment on attachment 8639540 [details] [review] [gaia] sfoster:ftu-rtl-rules-bug-1188069 > mozilla-b2g:master This is my first pass at refactoring based on conversations we've had around best practices here. There is probably more to do in there that wasn't a part of that RTL block - I can iterate here or in follow-up bugs.
Attachment #8639540 - Flags: review?(nefzaoui)
I ran into and file bug 1188104 along the way. My bad - as the review of that I'm not sure how I missed this regression. Also, while tracking down and testing each change I found orphaned rules that I've removed in the PR (attachment 8639540 [details] [review]) as it seemed less confusing to do so in the same patch than have a follow up and leave the reviewer/QA trying to figure out how to test. In general though I think these patches should have as few diversions from the pure goal of side-effect-free rule rearrangement as possible.
See Also: → 1188104
Comment on attachment 8639540 [details] [review] [gaia] sfoster:ftu-rtl-rules-bug-1188069 > mozilla-b2g:master As far as I'm concerned it's a refactor for what's already there. So me whining about the RTL status for FTU is for another day. And it's nicely done. I'm happy this pattern is coming to life here. Just one simple nit I'd like fixed :) Thanks so much!!
Attachment #8639540 - Flags: review?(nefzaoui) → review+
I didn't change that padding property, I'm not actually a fan of condensing these in general as it seems to me it just makes it the developer have to think harder about what value is going onto which edge. But maybe that's just me. On master: https://github.com/mozilla-b2g/gaia/commit/50fb42ed7e2550c4992bb15ee04d61747c9c1f6f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.