689545 - Retheme locale picker on tablets

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[Ioana Chiorean]

Build id : Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110927
Firefox/9.0a1 Fennec/9.0a1
Device: Acer Iconia A500  & Google Nexus S
OS: Android 3.1 Android 2.3.6

Steps to reproduce:
1. Open Fennec app 
2. Press option menu button in upper right and choose preferences / Go to preferences (for phones)
3. Press language button
4. try several times to change language

Expected result:
- the chosen language is highlighted in blue

Actual result:
- not all languages are highlighted 

Note : please see the following video
http://www.youtube.com/user/qaioana#p/a/u/0/YgSNme1v0Xs

Comment 1

[Aaron Train [:aaronmt]]

I can reproduce on the Galaxy Tab 10.1 (Android 3.1)

Comment 2

[Wesley Johnston (:wesj)]

Attachment: WIP Patch

Comment 3

[Wesley Johnston (:wesj)]

Assigning to Sriram. The highlight thing is still not fixed entirely by this. I suspect that is caused because we don't highlight selected items when you click them:

http://mxr.mozilla.org/mozilla-central/source/mobile/themes/core/honeycomb/platform.css#443

not sure why its so intermittent though.... or only shows up here...

Comment 4

[Sriram Ramasubramanian [:sriram]]

Attachment: WIP Patch

This patch uses honeycomb styling.
There is an issue with the picker-title's width not expanding to the enclosing container's width.
I tried for hours fixing it, but couldn't.

Comment 5

[Wesley Johnston (:wesj)]

Comment on attachment 562874 [details] [diff] [review]
WIP Patch

Review of attachment 562874 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! A few nits. Can you put up the revised patch for me to test again?

I'm fine with putting off a lot of the styling work to a different bug. This is about the selection states and just happens to require bringing in some baseline styling to fix it.

Note, this requires the settings patch to work as well.

::: mobile/themes/core/honeycomb/localePicker.css
@@ +43,5 @@
> +  -moz-box-align: center;
> +  -moz-box-flex: 1;
> +  background-color: @color_background_settings@ !important;
> +  border: @border_width_large@ solid @color_background_active@; 
> +  box-shadow: 0 0 @shadow_width_tiny@ @shadow_width_medium@ @color_background_button_overlay@;  

I don't see this shadow or border anywhere. Do we need them? Until we have a design that needs them, I'm fine with leaving them out.

@@ +61,5 @@
> +}
> +
> +#picker-title {
> +  margin: 0;
> +  padding: @padding_small@ @padding_normal@ @padding_xlarge@ @padding_normal@;

The last padding number isn't needed here.

@@ +65,5 @@
> +  padding: @padding_small@ @padding_normal@ @padding_xlarge@ @padding_normal@;
> +  border-bottom: @border_width_tiny@ solid @color_text_default@;
> +  background-color: transparent;
> +  color: @color_text_panel_header@;
> +  font-size: @font_snormal@;

I don't think background-color or color should be necessary either. And maybe we can hoist font-size up and have everyone inherit (except buttons)?

@@ +82,5 @@
> +
> +#installer-page {
> +  background-color: black;
> +  color: white;
> +}

The background-color here is being overridden by the background-color above. So you wind up with white text on light gray. Remove the !important above and you're good.

@@ +87,5 @@
> +
> +richlistitem {
> +  font-size: @font_snormal@;
> +  -moz-box-align: center;
> +  border-bottom: @border_width_tiny@ solid @color_background_button@;

We have @color_button_border@ for this.

@@ +118,5 @@
> +}
> +
> +button {
> +  font-size: @font_xtiny@ !important;
> +}

Why are the font-sizes on these buttons different than others? Can we not do this? Adjusting the min-width will also help ensure that the text looks a bit more centered.

Comment 6

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

This patch makes it look like a dialog. The dialog centers on the screen for honeycomb UI.
However, the title and the buttons are getting messed up. It's hard for me to make the <description> or <hbox> inside a <vbox> (.pane) take the entire width of the <vbox>
Once that is fixed, I could take another pass on other screens (esp. the "installing screen").

Comment 7

[Wesley Johnston (:wesj)]

Attachment: WIP Patch

Getting close. Some screen shots at:

http://people.mozilla.com/~wjohnston/screenshots/localePicker/

Still polishing up the front-screen and installing screen.

Comment 8

[Wesley Johnston (:wesj)]

Attachment: Patch v1

This looks and works pretty well. Screenshots are the ones without numbers in http://people.mozilla.com/~wjohnston/screenshots/localePicker/ (one's with numbers are mostly me playing around with different ideas).

I had to add a container inside the richlistitems in order to theme them up semi-correctly. I REALLY want to just move this style to be the default for richlistitems on Honeycomb, but doing so breaks some stuff, and fixing that breaks other things, etc. etc. So I think I'm going to file a follow up bug to chase that down.

Also fixed a non-bug I ran into testing. This now just ensures that we keep the pending install's name in the UI, even after the install finishes.

I'll put up a diff of the css file. I think we might be better, if we're going for "dialog" appearance here, to import over some more dialog classes and xul structure, which will likely alter the appearance on other themes.

So I'm holding off on asking for a full review, and just asking for feedback. Would be happy to take a review instead though, if you'd rather check something like this in now and do the rest in follow ups. Or I can just dig into this right away.

Comment 9

[Wesley Johnston (:wesj)]

Attachment: Diff of gingerbread and honeycomb

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 564979 [details] [diff] [review]
Patch v1

Why can't "locale-container" (or "locale-item") be a class on the richlistitem ?

Comment 11

[Wesley Johnston (:wesj)]

I'm trying to make it so that the selection state of the item will extend to the edges of the "dialog", while the borders do not. In some places I've been able to but the border of the text and image inside the box, but to be honest, its a headache and seems likely to break to me.

Comment 12

[BMO Automation]

Closing all opened bug in a graveyard component