Closed Bug 1015421 Opened 6 years ago Closed 6 years ago

Add ability to force update ButtonToast

Categories

(Firefox for Android :: General, defect)

30 Branch
ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 32
Tracking Status
fennec 32+ ---

People

(Reporter: Margaret, Assigned: wesj)

References

Details

Attachments

(2 files)

Splitting this off of bug 701725. With the patch in that bug, I found that multiple successive toasts can queue up and make the UI feel laggy. I think we should add a way for consumers to immediately update the toast. I suppose we wouldn't want consumers to be able to pave over other notifications, but maybe we can augment the API such that consumers can modify a single message.

What do you guys think we should do here?
Flags: needinfo?(wjohnston)
Flags: needinfo?(bnicholson)
tracking-fennec: --- → ?
Having the ability to immediately update the toast seems like a useful feature to have. If we're worried about interfering toasts, perhaps toasts can have an optional id field: if we show a toast and there's an existing toast with the same id, the old toast gets replaced.
Flags: needinfo?(bnicholson)
This also applies to bug 997288. Currently if you open multiple links in a row in background tabs, the toasts pile up and the browser feels laggy.
Blocks: 997288
I thought I replied here once, but I wrote basically the same thing as brian. I think an ID is probably our best first bet. It may mean toasts will update as the user does stuff which is weird, but given its in direct response to a user action, that doesn't seem quite as awful (points if we can animate/transition the text...)

Alternatively, we could make the toast a listview and show a series of them instead of queueing...
Flags: needinfo?(wjohnston)
Blocks: 1017045
Duplicate of this bug: 1017582
Duplicate of this bug: 1017608
tracking-fennec: ? → 32+
Going to grab this. Me and mark talked. I think maybe the best solution is to just not have a queue here. If you're doing a second action while one of these is showing, you probably don't care about the toast. That also sorta matches the interaction described here:

http://developer.android.com/design/patterns/confirming-acknowledging.html
"After the user deletes a conversation from the list in Gmail, an acknowledgment appears with an undo option. The acknowledgment remains until the user takes an unrelated action, such as scrolling the list."

I think that will contradict normal toasts, which do queue up.
Assignee: margaret.leibovic → wjohnston
Attached patch PatchSplinter Review
This kills off the queue and makes us update toasts with "new" info in-place. I removed some other safety things that aren't really needed anymore either. i.e. handling situations where the toast changes while you're tapping the button (it should be difficult for that to happen anymore). It also don't store toasts when we go in the background (because our contention here is that if you do anything on screen, you're effectively ignoring the toast).
Attachment #8431360 - Flags: review?(margaret.leibovic)
I started to play with animating the text switch to see what was simple. This only animates the message text using Android's built in slide left/right animations:

http://people.mozilla.org/~wjohnston/toastAnim.apk

I'm not sure if we care about this or not, but I figured I'd toss it up for UX. We can also try doing an animation on the button + image. Just requires a little more work. Or we could play with animating the entire toast...

Do you think we need a transition when the toast switches from one view to a different one Anthony?
Flags: needinfo?(alam)
Comment on attachment 8431360 [details] [diff] [review]
Patch

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

Looks good, just a few random comments.

::: mobile/android/base/GeckoApp.java
@@ -500,5 @@
>          super.onSaveInstanceState(outState);
>  
> -        if (mToast != null) {
> -            mToast.onSaveInstanceState(outState);
> -        }

Nice to remove this dependency.

::: mobile/android/base/widget/ButtonToast.java
@@ +26,5 @@
>  import android.os.Handler;
>  import android.view.View;
>  import android.widget.Button;
>  import android.widget.TextView;
> +import android.util.Log;

Unused import.

@@ +31,4 @@
>  
>  public class ButtonToast {
>      @SuppressWarnings("unused")
>      private final static String LOGTAG = "GeckoButtonToast";

Should we just get rid of this, too, since it's unused?

@@ +43,5 @@
>  
>      public enum ReasonHidden {
>          CLICKED,
>          TIMEOUT,
> +        NEW_TOAST,

Maybe REPLACED would be better?

@@ +104,5 @@
>      }
>  
>      private void show(Toast t, boolean immediate) {
> +        // If we're already showing a toast, replace it with the new one by hiding the old one and quickly showing the new one.
> +        if (mCurrentToast != null) {

The data types in here are confusingly named. I would assume that ButtonToast *is* a Toast, but really it's a container that displays Toast objects.
Attachment #8431360 - Flags: review?(margaret.leibovic) → review+
(In reply to Wesley Johnston (:wesj) from comment #8)
> I'm not sure if we care about this or not, but I figured I'd toss it up for
> UX. We can also try doing an animation on the button + image. Just requires
> a little more work. Or we could play with animating the entire toast...

Definitely helpful to know this now! Thanks for that Wes.

> Do you think we need a transition when the toast switches from one view to a
> different one Anthony?

Do you mean switching of older toasts in lieu of newer toasts that come in? If so, I'd love to try some simple animations that look like (http://daneden.github.io/animate.css/) a quick "flipOutX" or "bounceOutRight/Left" (it would be extra cool if they went left/right depending on which way the tab flies out the tray :)
Flags: needinfo?(alam) → needinfo?(wjohnston)
(In reply to Anthony Lam (:antlam) from comment #10)
> Do you mean switching of older toasts in lieu of newer toasts that come in?
> If so, I'd love to try some simple animations that look like
> (http://daneden.github.io/animate.css/) a quick "flipOutX" or
> "bounceOutRight/Left" (it would be extra cool if they went left/right
> depending on which way the tab flies out the tray :)

Yeah, I think so. With this patch, if you try and show a new button-toast while one is already up, we'll just instantly flip to the new one. I originally thought we might want to animate the text on the toast out-in to provide a smoother transition, but maybe we should just hide the toast first, before we show the new one.... I'll try that real quick.

The animations here are stolen from Android's toast animations to match. We can do whatever we want for these ButtonToasts though.
Flags: needinfo?(wjohnston)
(In reply to Wesley Johnston (:wesj) from comment #12)
> originally thought we might want to animate the text on the toast out-in to
> provide a smoother transition, but maybe we should just hide the toast
> first, before we show the new one.... I'll try that real quick.

Animating just the text might be a bit much - but I could be wrong :) I think it'd be nice to animate the "panel" in which the toast is presented on. (animating the hiding of the previous and the showing of the new).
(In reply to Anthony Lam (:antlam) from comment #13)
> (In reply to Wesley Johnston (:wesj) from comment #12)
> > originally thought we might want to animate the text on the toast out-in to
> > provide a smoother transition, but maybe we should just hide the toast
> > first, before we show the new one.... I'll try that real quick.
> 
> Animating just the text might be a bit much - but I could be wrong :) I
> think it'd be nice to animate the "panel" in which the toast is presented
> on. (animating the hiding of the previous and the showing of the new).

Chrome just animates the text, but pretty quickly/subtly. I feel like making the toast appear/disappear might be a bit jarring, since we'd want this to happen quickly. However, we don't need to copy chrome, so we could play around with the alternatives.
https://hg.mozilla.org/mozilla-central/rev/5d9621220437
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
You need to log in before you can comment on or make changes to this bug.