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

VERIFIED FIXED

Status

defect
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: vingtetun, Assigned: vingtetun)

Tracking

Dependency tree / graph
Bug Flags:
in-litmus +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: formfill)

Attachments

(2 attachments)

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.
Posted patch Patch v0.1Splinter Review
This is the patch that did the previous look'n feel you can see in the attached image.
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.
Attachment #458622 - Flags: ui-review?(madhava)
I like the direction
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
(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 on attachment 458622 [details]
Form Assistant and Find-In-Page with the new theme

madhava gave an IRC "ok"
Attachment #458622 - Flags: ui-review?(madhava) → ui-review+
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
Attachment #458623 - Flags: review?(mark.finkle) → review+
http://hg.mozilla.org/mobile-browser/rev/033a63da835d
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.