580213 - Enhance the look of Form Assistant and Find-In-Page

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Form Assistant and Find-In-Page with the new theme

Form Assistant and Find-In-Page share the same space in the UI and we could win some space by moving the autofill box in the same place the textbox for find live.
Also it makes sense to replace the next/previous buttons with images and now that we can close easily the Form Assistant we could also remove the close button.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.1

This is the patch that did the previous look'n feel you can see in the attached image.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 458622 [details]
Form Assistant and Find-In-Page with the new theme

Also, bug 580242 add a textbox above the select UI to helps filter the result so you can imagine it with it.

Comment 3

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

I like the direction

Comment 4

[Madhava Enros [:madhava]]

Yes - this is definitely headed in the right direction.

Things to still work on:
- how well does this work in portrait?
- can we move to having the next/prev buttons be by themselves (no full width bar) when there is nothing else to display?

- final styling, button size

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #4)
> Yes - this is definitely headed in the right direction.
> 
> Things to still work on:
> - how well does this w
> - final styling, button size

I will file a followup for ensure these 2 are good and trying to have Sean for the styling.

> - can we move to having the next/prev buttons be by themselves (no full width
> bar) when there is nothing else to display?

Sure, this is planned in a near future. For now the plan is to build on top of this patch.

Comment 6

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

Comment on attachment 458622 [details]
Form Assistant and Find-In-Page with the new theme

madhava gave an IRC "ok"

Comment 7

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

Comment on attachment 458623 [details] [diff] [review]
Patch v0.1

>diff --git a/chrome/content/bindings.xml b/chrome/content/bindings.xml

>+      <xul:hbox class="panel-dark" pack="end">
>+        <children includes="textbox|arrowscrollbox"/>
>+        <xul:toolbarbutton anonid="previous-button" class="button-image" xbl:inherits="command=previous"/>
>+        <xul:toolbarbutton anonid="next-button" class="button-image" xbl:inherits="command=next"/>

anonid is good for getting the element in the XBL, but we typically use a unique classname too, so we can apply CSS to the anonymous element:

class="button-image previous-button"
class="button-image next-button"


>diff --git a/themes/core/browser.css b/themes/core/browser.css

>+#content-navigator > hbox > toolbarbutton[anonid="previous-button"] {

Use #content-navigator > hbox > toolbarbutton.previous-button

>+  list-style-image: url("chrome://browser/skin/images/previous-64.png");

previous-default-64.png ? I think we use that convention for many of our normal buttons

>+#content-navigator > hbox > toolbarbutton[anonid="next-button"] {

Use #content-navigator > hbox > toolbarbutton.next-button

>+  list-style-image: url("chrome://browser/skin/images/next-64.png");

next-default-64.png ? I think we use that convention for many of our normal buttons

r+ with the nits

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/033a63da835d

Comment 9

[Aakash Desai [:aakashd]]

Something's really broken here as the find bar is showing up when the formfill bar should be shown. Bug 581097 was filed. I'm re-opening this until the exact cause is figured out.

Comment 10

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (X11; U; Linux armv71; Nokia N900; en-US; rv:2.0b2pre) Gecko/2010727 Namoroka/4.0b2pre Fennec/2.0a1pre

and

Mozilla/5.0 (Android; U; Linux armv71; en-US; rv:2.0b2pre) Gecko/20100727 Namoroka/4.0b2pre Fennec/2.0a1pre

Comment 11

[Aakash Desai [:aakashd]]

Updated litmus testcases:

https://litmus.mozilla.org/show_test.cgi?id=12142
https://litmus.mozilla.org/show_test.cgi?id=12198
https://litmus.mozilla.org/show_test.cgi?id=12220
https://litmus.mozilla.org/show_test.cgi?id=12253
https://litmus.mozilla.org/show_test.cgi?id=12197
https://litmus.mozilla.org/show_test.cgi?id=12128
https://litmus.mozilla.org/show_test.cgi?id=12115

Comment 12

[Aakash Desai [:aakashd]]

More testcases created: 

https://litmus.mozilla.org/show_test.cgi?id=12585
https://litmus.mozilla.org/show_test.cgi?id=12586
https://litmus.mozilla.org/show_test.cgi?id=12587