Closed Bug 840267 Opened 10 years ago Closed 10 years ago

Sync UI using light-holo in setup/JPAKE UI


(Firefox for Android Graveyard :: Android Sync, defect)

Not set


(firefox21 fixed, firefox22 fixed, fennec21+)

Tracking Status
firefox21 --- fixed
firefox22 --- fixed
fennec 21+ ---


(Reporter: aaronmt, Assigned: wesj)



(Keywords: uiwanted)


(2 files, 4 obsolete files)

Attached image Nightly (02/11) - Screenshot (obsolete) —
See screenshot.
See Also: → 836356
Is this just a polish bug?

Is there a trivial fix?
Flags: needinfo?(ibarlow)
Keywords: uiwanted
Attached patch Patch (obsolete) — Splinter Review
If UX is OK with using the light theme here, there's not much to do. This switches us to use standard colors for link text, since we're using a standard background. I'll post a screenshot from my S3 here, but will have to charge some other devices to test on any other themes.
Attached image Screenshot (obsolete) —
I don't think anyone can steal that code for evil.... can they?
J-PAKE codes expire after five minutes.
If possible, I'd like to use a more Holo style title bar and Cancel button area here. Also let's reverse the JPAKE fields/numbers so it is dark numbers on a light background.
Flags: needinfo?(ibarlow)
Wes, are you going to run with this?
Assignee: nobody → wjohnston
Attached patch Patch v1 (obsolete) — Splinter Review
This is me trying to steal from the alert dialog style as much as I can. I had to do some trickery to get lines around the buttonBox.

I'll post some screenshots from a ICS and Gingerbread phone.
Attachment #713725 - Flags: review?(rnewman)
Attached image Screenshots
Attachment #712619 - Attachment is obsolete: true
Attachment #712648 - Attachment is obsolete: true
Attachment #712650 - Attachment is obsolete: true
ship it!
Comment on attachment 713725 [details] [diff] [review]
Patch v1

Review of attachment 713725 [details] [diff] [review]:

Looks good, just trailing whitespace. I will backport this to upstream.

Thanks Wes!

::: mobile/android/base/resources/layout/sync_account.xml
@@ +62,4 @@
> +    <LinearLayout
> +      style="@style/SyncBottom" >
> +  

Trailing whitespace in a couple of places.
Attachment #713725 - Flags: review?(rnewman) → review+
Backport pull request:

Wes, I think you might have forgotten to `hg add` the v11 style file. Take a look?
tracking-fennec: ? → 21+
Attached patch Patch v2Splinter Review
Sorry. Forgot to hg add it.
Attachment #713725 - Attachment is obsolete: true
Attachment #714041 - Flags: review?(rnewman)
Comment on attachment 714041 [details] [diff] [review]
Patch v2

Review of attachment 714041 [details] [diff] [review]:

If you've tested it, looks good to me!
Attachment #714041 - Flags: review?(rnewman) → review+
Merged in GitHub. Waiting for inbound to open so wesj can land (or I can if he disappears).
Target Milestone: --- → mozilla22
Closed: 10 years ago
Resolution: --- → FIXED
I landed a follow-up to this to fix some whitespace in the landed patch.
Comment on attachment 714041 [details] [diff] [review]
Patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 832321
User impact if declined: Sync setup is hard to read
Testing completed (on m-c, etc.): Has been on mc for a few weeks. No regressions
Risk to taking this patch (and alternatives if risky): Low risk. Mostly themeing
String or UUID changes made by this patch: None.
Attachment #714041 - Flags: approval-mozilla-aurora?
Comment on attachment 714041 [details] [diff] [review]
Patch v2

low risk UI polish helping existing sync set-up on android.Approving for uplift
Attachment #714041 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Mozilla Services → Android Background Services
Product: Android Background Services → Firefox for Android
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.