Closed Bug 737922 Opened 8 years ago Closed 8 years ago

Invert form validation message arrow if the message is above the input box


(Firefox for Android :: General, defect)

Not set



Firefox 14
Tracking Status
blocking-fennec1.0 --- soft


(Reporter: Margaret, Assigned: sriram)




(5 files, 1 obsolete file)

Follow-up to bug 731654. If a validation message appears for an input box at the bottom of the screen, we place the message above the input, but we still need to deal with inverting the arrow.
Assignee: nobody → sriram
noming for soft-block
blocking-fennec1.0: --- → ?
blocking-fennec1.0: ? → soft
Attached patch Patch (obsolete) — Splinter Review
This patch shows normal/inverted arrows based on where it is placed.

It's better to move the logic of AutocompleteList and ValidationMessage into two separate files (which can optionally "extend" InContentHelper -- which holds the logic).

The FormValidation would have mAutoCompleteList and mValidationMessage and knows when to display what. So, it would be basically be splitting the existing code for better readability and reusability. Probably we can take it after 1.0.
Attachment #615918 - Flags: review?(mark.finkle)
Comment on attachment 615918 [details] [diff] [review]

>diff --git a/mobile/android/base/ b/mobile/android/base/

>+  res/drawable/validation_arrow_inverted.png \

>+  res/drawable-hdpi/validation_arrow_inverted.png \

>+  res/drawable-xhdpi-v11/validation_arrow_inverted.png \

Did I miss these images or did you not add them?

>diff --git a/mobile/android/base/resources/layout/validation_message.xml b/mobile/android/base/resources/layout/validation_message.xml

>-    <ImageView android:layout_width="24dip"
>+    <ImageView android:id="@+id/validation_message_arrow"
>+               android:layout_width="24dip"
>                android:layout_height="10dip"
>                android:layout_centerHorizontal="true"
>                android:layout_alignParentTop="true"
>                android:src="@drawable/validation_arrow"
>                android:scaleType="fitXY"/>
>+    <ImageView android:id="@+id/validation_message_arrow_inverted"
>+               android:layout_width="24dip"
>+               android:layout_height="10dip"
>+               android:layout_centerHorizontal="true"
>+               android:layout_marginTop="35dip"
>+               android:layout_alignParentTop="true"
>+               android:src="@drawable/validation_arrow_inverted"
>+               android:scaleType="fitXY"
>+               android:visibility="gone"/>

Do we need the second imageview? It's the same as the first except for the src and the layout_marginTop.

I'd like to see some screenshots of this in action. Also, please make sure you have tested this on multiple phones.
Attachment #615918 - Flags: review?(mark.finkle) → review-
Attached patch PatchSplinter Review
I had forgotten to "hg add" the images. This patch has the required images.

The reason for using a new ImageView instead of reusing the old one is,
Everytime the arrow changes, we need to change the source and the layoutparams. The drawable for the arrows should be held in two variables, and so are layoutparams. This gets messier while handling it. Instead I felt like having one variable to control the arrows.

Ideally I wanted two textviews, so that the visibility can be changed based on need. But then felt like changing the layoutparams. But changing 2 different variables (drawable and layoutparams), controlled by 4 variables to hold the options will get messier.
Attachment #615918 - Attachment is obsolete: true
Attachment #616254 - Flags: review?(mark.finkle)
Attachment #616254 - Flags: review?(mark.finkle) → review+
Comment on attachment 616254 [details] [diff] [review]

good to land
Attachment #616254 - Flags: approval-mozilla-central+
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 14
When the  form validation message is above the field the arrow is pointed down as 
you can see here:

Marking as verified fixed on: Firefox 15.0a1 (2012-05-21)
Device: LG Optimus 2X (Android 2.2.2)
You need to log in before you can comment on or make changes to this bug.