Anthony what do you think? Wes, you seemed to feel strongly against it on IRC. Thoughts?

I'm not a big fan of fading text. I personally find it difficult to read text that fades out as I'm reading. ellipsis gives me a more definite "Sorry, we're out of room" message. I think I would like fading better if we supported scrolling the title (bug 863845). i.e. the fading makes me feel like I should be able to get the rest. The ellipsis is a more definitive "This is all we're gonna show". That said, UX decided in the past that having one extra (faded) letter was better for usability. I've always been curious if there have been any usability studies done on the subject (based on conversations I've had with Google UX, I get the feeling the answer is no).

Not a strong preference to either one right now but when I think about either truncating or fading, I prefer fading. From a UX stand point, I guess it gives an extra character rather than using that space for '...' . I talked to Yuan and she seems indifferent as well, I'd be open to giving this a try.

FWIW, latest Chrome for Android is using a fading effect in the location entry.

It seems we have both reservations and excitement about this change. Let's experiment this in Nightly to see how we feel about it and hopefully get some wider feedback.

Comment on attachment 8484479 [details] [diff] [review] Change FadedTextView to be a ThemedTextView (r=bnicholson) Because we needs the private mode bits in toolbar.

Comment on attachment 8484480 [details] [diff] [review] BUg 1061508 - Use FadedTextView in the toolbar (r=bnicholson) Actually use it in the toolbar layout.

Comment on attachment 8484479 [details] [diff] [review] Change FadedTextView to be a ThemedTextView (r=bnicholson) Review of attachment 8484479 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/widget/FadedTextView.java @@ +13,5 @@ > import android.graphics.drawable.Drawable; > import android.text.Layout; > import android.util.AttributeSet; > + > +import org.mozilla.gecko.widget.ThemedTextView; Nit: Put this import after org.mozilla.gecko.R and remove empty space between them. @@ -36,5 @@ > public FadedTextView(Context context, AttributeSet attrs) { > - this(context, attrs, android.R.attr.textViewStyle); > - } > - > - public FadedTextView(Context context, AttributeSet attrs, int defStyle) { Why remove this constructor?

Comment on attachment 8484480 [details] [diff] [review] BUg 1061508 - Use FadedTextView in the toolbar (r=bnicholson) Review of attachment 8484480 [details] [diff] [review]: ----------------------------------------------------------------- UX-wise, I agree with Wes that a faded view feels like it should be scrollable. But as you pointed out, Chrome uses this same effect and isn't scrollable, so maybe it doesn't mean what I expected. ::: mobile/android/base/resources/layout/toolbar_display_layout.xml @@ +23,5 @@ > android:contentDescription="@string/site_security" > android:visibility="gone"/> > > + <org.mozilla.gecko.widget.FadedTextView android:id="@+id/url_bar_title" > + style="@style/UrlBar.Title" Hm, I'm confused how this would even work after you dropped the style constructor in the first patch. What am I missing?

tried fading some of the labels/titles in the new tablet's UI (WIP) Showing one more letter (faded) seems better than not (and showing a '...' in it's place)

Fade all things! ;-) Could you please file a separate bug?

^ done! Bug 1070029

(In reply to Brian Nicholson (:bnicholson) from comment #9) > Comment on attachment 8484479 [details] [diff] [review] > Change FadedTextView to be a ThemedTextView (r=bnicholson) > > Review of attachment 8484479 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mobile/android/base/widget/FadedTextView.java > @@ +13,5 @@ > > import android.graphics.drawable.Drawable; > > import android.text.Layout; > > import android.util.AttributeSet; > > + > > +import org.mozilla.gecko.widget.ThemedTextView; > > Nit: Put this import after org.mozilla.gecko.R and remove empty space > between them. Done. > @@ -36,5 @@ > > public FadedTextView(Context context, AttributeSet attrs) { > > - this(context, attrs, android.R.attr.textViewStyle); > > - } > > - > > - public FadedTextView(Context context, AttributeSet attrs, int defStyle) { > > Why remove this constructor? (In reply to Brian Nicholson (:bnicholson) from comment #10) > Comment on attachment 8484480 [details] [diff] [review] > BUg 1061508 - Use FadedTextView in the toolbar (r=bnicholson) > > Review of attachment 8484480 [details] [diff] [review]: > ----------------------------------------------------------------- > > UX-wise, I agree with Wes that a faded view feels like it should be > scrollable. But as you pointed out, Chrome uses this same effect and isn't > scrollable, so maybe it doesn't mean what I expected. > > ::: mobile/android/base/resources/layout/toolbar_display_layout.xml > @@ +23,5 @@ > > android:contentDescription="@string/site_security" > > android:visibility="gone"/> > > > > + <org.mozilla.gecko.widget.FadedTextView android:id="@+id/url_bar_title" > > + style="@style/UrlBar.Title" > > Hm, I'm confused how this would even work after you dropped the style > constructor in the first patch. What am I missing? The 'style' XML attribute is handled by Android's AssetManager. The style constructor is meant to be used to set a default style for the View.

(In reply to Brian Nicholson (:bnicholson) from comment #9) > Comment on attachment 8484479 [details] [diff] [review] > Change FadedTextView to be a ThemedTextView (r=bnicholson) > > Review of attachment 8484479 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mobile/android/base/widget/FadedTextView.java > @@ +13,5 @@ > > import android.graphics.drawable.Drawable; > > import android.text.Layout; > > import android.util.AttributeSet; > > + > > +import org.mozilla.gecko.widget.ThemedTextView; > > Nit: Put this import after org.mozilla.gecko.R and remove empty space > between them. Done. > @@ -36,5 @@ > > public FadedTextView(Context context, AttributeSet attrs) { > > - this(context, attrs, android.R.attr.textViewStyle); > > - } > > - > > - public FadedTextView(Context context, AttributeSet attrs, int defStyle) { > > Why remove this constructor? ThemedView (from which ThemedTextView is generated) doesn't have it. I could add it to ThemedView.frag but we don't really need it here i.e. android.R.attr.textViewStyle is redundant in this case.

https://hg.mozilla.org/integration/fx-team/rev/0f2bceed4532 https://hg.mozilla.org/integration/fx-team/rev/556258d3b939

https://hg.mozilla.org/mozilla-central/rev/0f2bceed4532 https://hg.mozilla.org/mozilla-central/rev/556258d3b939

Verified as fixed in Build: Firefox for Android 35.0a1 (2014-10-05) Devices: Nexus 4 (Android 4.4.4) Asus Transformer Pad TF300T (Android 4.2.1)