Closed Bug 840267 Opened 8 years ago Closed 8 years ago

Sync UI using light-holo in setup/JPAKE UI

Categories

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

ARM
Android
defect
Not set
normal

Tracking

(firefox21 fixed, firefox22 fixed, fennec21+)

RESOLVED FIXED
mozilla22
Tracking Status
firefox21 --- fixed
firefox22 --- fixed
fennec 21+ ---

People

(Reporter: aaronmt, Assigned: wesj)

References

Details

(Keywords: uiwanted)

Attachments

(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?
Sure
Assignee: nobody → wjohnston
Thanks!
Status: NEW → ASSIGNED
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.

http://androidxref.com/4.0.4/xref/frameworks/base/core/res/res/layout/alert_dialog_holo.xml#71

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:

https://github.com/mozilla-services/android-sync/pull/293

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
https://hg.mozilla.org/mozilla-central/rev/f46e28183c02
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
I landed a follow-up to this to fix some whitespace in the landed patch.

https://hg.mozilla.org/integration/mozilla-inbound/rev/12a80c7bb01a
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.