Closed Bug 1147653 Opened 9 years ago Closed 9 years ago

Polish "Send tab to device" confirmation

Categories

(Firefox for Android Graveyard :: Overlays, defect)

x86
Android
defect
Not set
normal

Tracking

(firefox44 verified, fennec44+)

RESOLVED FIXED
Firefox 44
Tracking Status
firefox44 --- verified
fennec 44+ ---

People

(Reporter: antlam, Assigned: mcomella)

References

Details

Attachments

(11 files, 1 obsolete file)

1.59 MB, image/png
Details
328.83 KB, image/png
Details
194.79 KB, image/png
Details
2.52 MB, video/mp4
Details
11.90 KB, application/zip
Details
40 bytes, text/x-review-board-request
sebastian
: review+
Details
40 bytes, text/x-review-board-request
sebastian
: review+
Details
40 bytes, text/x-review-board-request
sebastian
: review+
Details
40 bytes, text/x-review-board-request
sebastian
: review+
Details
40 bytes, text/x-review-board-request
sebastian
: review+
Details
245.67 KB, image/png
Details
We punted on this due to time constraints. As a result, we're currently using UI that has some UX issues inherently. Knowing that, I wanted file this to keep track of the work so maybe we can get around to it.
Attaching overview of the flow (with notifications) incorporated
What the flow would look like on slower connections where can't tell right away that the tab failed to send.
CC'ing mhaigh here since we leverage notifications for our Tab Queue UX work. There may be some overlap here and I'll definitely be considering the UX of all notifications as a whole.
Told antlam I'd take care of this one.
Assignee: nobody → michael.l.comella
tracking-fennec: --- → 43+
Meant to do 44, antlam can explain.
tracking-fennec: 43+ → 44+
Moving on bug 1140048 should be done with this. 

Talked to mcomella, we could put this in 44.
Please don't block browsing for a simple confirmation.
(In reply to Paul [pwd] from comment #7)
> Please don't block browsing for a simple confirmation.

Anthony, thoughts? I assume we'd make this transition quite fast though I don't know if this would still be an issue.

We might be able to use a SnackBar, but I don't think that animation would work with our share overlay exit animation.
Flags: needinfo?(alam)
(In reply to Paul [pwd] from comment #7)
> Please don't block browsing for a simple confirmation.

We're going to fade this away at the same speed as our toasts currently do so I'm not worried about this.
Flags: needinfo?(alam)
(In reply to Anthony Lam (:antlam) from comment #9)
> (In reply to Paul [pwd] from comment #7)
> > Please don't block browsing for a simple confirmation.
> 
> We're going to fade this away at the same speed as our toasts currently do
> so I'm not worried about this.

I fear this would an approach more akin to decorating than designing. Especially given what's happening on the platform in regards to sharing and Android M. We'll soon be able to have our users open the share dialog and be able to share to devices directly cutting out the whole middle step of first tapping the share plane. Android has taken some significant strides forward in terms of usability and one of the better ones of those are the snackbar, if for no other reason than they're unobtrusive. The proposal here is gaudy. I don't even feel it scales well. Certainly not all the way up to desktop.

If we are to tackle a problem of pages failing to send then the solution there isn't to make the failure notice even larger, it is in fact to work on preventing such a scenario. i.e. queuing said pages until a network connection is regained (maybe via a minimum importance notification in the notification drawer). This isn't something we should be bothering users with, Sync should just work.

The onboarding process is what could do with a more attention than this. "Already have an account" is ridiculously small and hard to find let alone press. Despite being on mobile, it doesn't offer any way to link an account other than username and password, why not qrcode? Why not NFC? These are actual design problems that will eventually need to be fixed.
(In reply to Paul [pwd] from comment #10)
> If we are to tackle a problem of pages failing to send then the solution
> there isn't to make the failure notice even larger, it is in fact to work on
> preventing such a scenario. i.e. queuing said pages until a network
> connection is regained

I'm under the impression that we do do this (and the mocks just don't represent this). Richard?
Flags: needinfo?(rnewman)
Pwd - I understand your concern. 

But first of all, that's not all we are trying to tackle. This is a part of the polish that we wanted to get done for the first redesign of the Share overlay. It's still an important part of the experience to see and feel, that feedback. 

So, let's get a build to test this before we over rotate.
antlam, can I suggest we delay this work until just after Android M is released and see what backwards compatibility we get for Direct Share* and then double down on this work then. If we can make our changes in a timely fashion at that point, we're likely to be able to consolidate these changes rather than be back here in a years time playing catch up once again. This is also likely to be a huge boon in terms of marketing as the Android news sites love implementation of the newest features available.


* http://www.androidpolice.com/2015/06/02/android-m-feature-spotlight-direct-share-can-provide-share-links-to-specific-contacts-or-conversations-inside-apps/
(In reply to Michael Comella (:mcomella) from comment #11)

> I'm under the impression that we do do this (and the mocks just don't
> represent this). Richard?

Indeed, the *only* behavior we have is to queue pages, which is one of the reasons it's non-trivial to show immediate success or failure. The code that handles your share is not the code that talks to the Sync server.

Send Tab adds a command to a database table, then asks Android to trigger a sync. Android usually does so… but not if you have background data checkboxes flipped, or if you're on bad wifi, or low battery, or….

Sync will then run, and as a side-effect of its normal syncing behavior it'll upload outgoing commands. It might also be in a back-off state or otherwise unable to sync, which isn't necessarily an error.

Whether the sent page actually appears on another device is another matter. That device needs to exist, to have the same client ID, working credentials, etc… and finally, it needs to sync before you look for the tab!
Flags: needinfo?(rnewman)
With that, can I suggest rather than use a full-page confirmation, we switch to a snackbar given that ultimately we're only doing so to provide users feedback that something has happened.

Can I once again suggest that we delay all sync cosmetic work until we get a better picture of direct share. In bug 1140048 we're moving the share plane to the top level menu, but once direct share becomes available that's likely to simply get in the way of a much nicer user experience. That said, we can still take our overall sharing experience in the right direction by switching to bottom sheets: https://www.google.com/design/spec/components/bottom-sheets.html#bottom-sheets-usage
Originally proposed in bug 1122511
Flags: needinfo?(alam)
(In reply to Paul [pwd] from comment #15)
> With that, can I suggest rather than use a full-page confirmation, we switch
> to a snackbar given that ultimately we're only doing so to provide users
> feedback that something has happened.

That work is being tracked in bug 1157526.

> Can I once again suggest that we delay all sync cosmetic work until we get a
> better picture of direct share. In bug 1140048 we're moving the share plane
> to the top level menu, but once direct share becomes available that's likely
> to simply get in the way of a much nicer user experience. 

Sorry, I don't think that this is within the scope of this bug. This is about cleaning up existing UI/UX that we had issues with earlier.

I'm also having trouble understanding how our UI gets in the way of the system's UI? User choice is an important thing here. This does not replace anything in the OS, nor does it disable any feature either.

> That said, we can
> still take our overall sharing experience in the right direction by
> switching to bottom sheets:
> https://www.google.com/design/spec/components/bottom-sheets.html#bottom-
> sheets-usage
> Originally proposed in bug 1122511

Actually, we began design on this UX before these guidelines were published. In many ways, the benefit of using our own visuals here are also quite important. It's an opportunity to brand the experience to be "Firefox", and help the user recognize the product in a different context. But also, to extend that visual consistency to other parts of the user's phone (like Sharing to Firefox from inside another application).
Flags: needinfo?(alam)
(In reply to Anthony Lam (:antlam) from comment #16)
> That work is being tracked in bug 1157526.
So this is bug will be WONTFIX'd?

> Sorry, I don't think that this is within the scope of this bug. This is
> about cleaning up existing UI/UX that we had issues with earlier.
It's not in the scope of this bug, but given that this bug is for all intents and purposes invalid and raises the issues of our Firefox Share user experience, I thought that since I had you here, it'd make since to engage you in a discussion about the wider UX approach rather than try and raise the issue in bug 1140048 which due to direct share will become obsolete in a year anyway.

> I'm also having trouble understanding how our UI gets in the way of the
> system's UI? User choice is an important thing here. This does not replace
> anything in the OS, nor does it disable any feature either.
> 
> Actually, we began design on this UX before these guidelines were published.
> In many ways, the benefit of using our own visuals here are also quite
> important. It's an opportunity to brand the experience to be "Firefox", and
> help the user recognize the product in a different context. But also, to
> extend that visual consistency to other parts of the user's phone (like
> Sharing to Firefox from inside another application).
While I get that this design was proposed before the guidelines were published, the reality is that they exist now and we're undertaking this work now. By surfacing share in a bottom sheet above all other intents as Google has done in some of their products, we're able to push the brand in a more meaningful way. Quite literally any time a user wants to share anything to another app from Firefox they will see Firefox Share. By being agile in our design choices here at this stage, we'll be able to leverage the OS and its conventions to its fullest. The fact is we'll end up implementing it down the line anyway as it's a great usage pattern, it's a matter of when not if.
Did I manage to make a compelling enough point to refocus our energy into a longer term, heightened Firefox Share exposure plan or are you planning to press ahead with bug 1140048?
Flags: needinfo?(alam)
(In reply to Paul [pwd] from comment #17)
> (In reply to Anthony Lam (:antlam) from comment #16)
> > That work is being tracked in bug 1157526.
> So this is bug will be WONTFIX'd?

Sorry, I don't follow.

(In reply to Paul [pwd] from comment #18)
> Did I manage to make a compelling enough point to refocus our energy into a
> longer term, heightened Firefox Share exposure plan or are you planning to
> press ahead with bug 1140048?

Since we agree that this is not what this bug is about, we could probably find a better medium to facilitate these types of discussion. 

Though, I still am not seeing the issue with improving our own product here. Nor, a clear enough explanation of what you are prescribing as a "longer term, heightened Firefox Share exposure plan".
Flags: needinfo?(alam)
^ As I've stated before, I am always available to be reached via email for open discussions. That would help avoid confusion for everyone else as to what is going on in this particular bug.
I filed bug 1211955 for the unrelated discussion so we can take it out of this bug.
See Also: → 1211955
Anthony, do you have any specs for this?
Flags: needinfo?(alam)
Talked on IRC
Flags: needinfo?(alam)
Anthony requested the animation to be:
  https://dribbble.com/shots/1874460--Add-to-Firefox-animations-mock

But it'll take more time to implement than we want to invest here. Instead, as the dialog slides out, we fade in the check for 250ms, pause it for 250ms, and fade it out for 250ms. The user can interrupt this at any point by tapping on the animation.

We'll tweak the numbers once we have a build.

Note that the current implementation runs the share method in a background thread and displays a toast when the action completes. However, this screws up our animation (e.g. we need to leave the black background!) so antlam said it'd be fine for this to happen in conjunction with the dialog slide out, rather than in reaction to the background share event.
While I don't have a large enough check asset, I think this looks a little silly Anthony:
  https://people.mozilla.com/~mcomella/apks/mcomella-1147653_02.apk
Flags: needinfo?(alam)
You mean "whimsical"? :P

Actually, I like this a lot more than what we have. It's more obvious but also feels a lot more satisfying and completes this sharing experience nicely.

I found a "check" asset in the same folder so I'll just reattach it here. Try these (coming up next)!
Flags: needinfo?(alam)
Attached file icon_greencheck2.zip (obsolete) —
Looks like I've created it with a drop shadow. Not sure if it's more helpful.. let me know!
hey Mcomella,

I was just about to attach the 'x' assets but I recall a conversation about that not being feasible due to our current Sync capabilities. Can you confirm that's still the case, if so, I won't worry about that for now.
Flags: needinfo?(michael.l.comella)
(In reply to Anthony Lam (:antlam) from comment #28)
> I was just about to attach the 'x' assets but I recall a conversation about
> that not being feasible due to our current Sync capabilities. Can you
> confirm that's still the case, if so, I won't worry about that for now.

Correct. The new UI doesn't wait for the share action to complete either so we don't have any error states at all.

(In reply to Anthony Lam (:antlam) from comment #27)
> Looks like I've created it with a drop shadow. Not sure if it's more
> helpful.. let me know!

It's worth noting we have several other check assets already – can we reuse one of them? They are from bug 1212629 comment 0:
* fxaccount_checkbox.png
* img_check.png
* overlay_check.png

The drop shadow will likely make us need two assets so I'd like to avoid it if possible (or figure out a way to do it dynamically, but I'm not sure that's feasible).
Flags: needinfo?(michael.l.comella) → needinfo?(alam)
(In reply to Michael Comella (:mcomella) from comment #29)
> It's worth noting we have several other check assets already – can we reuse
> one of them? They are from bug 1212629 comment 0:
> * fxaccount_checkbox.png
> * img_check.png
> * overlay_check.png

Hm, likely. 

> The drop shadow will likely make us need two assets so I'd like to avoid it
> if possible (or figure out a way to do it dynamically, but I'm not sure
> that's feasible).

I definitely do not wont two assets for this drop shadow.

I'll ping you on IRC as I'll need to know exactly where these 3 checks are being used in order to consolidate them with low risk. :)
Flags: needinfo?(alam)
I used the check from comment 14 and put this in a video because it's more easily shared than an APK.

It still feels off to me. :\
Attached file icon_greencheck3.zip
Checks, as discussed.
Attachment #8672191 - Attachment is obsolete: true
Flags: needinfo?(michael.l.comella)
Summary: Polish "Send tab to device" confirmation and failure screen → Polish "Send tab to device" confirmation
Can't do "failure" right now so renaming ^
We discussed timing of the new animation and settled on 250ms in (concurrent with the slide out), 500ms pause, 250ms out.
Flags: needinfo?(michael.l.comella)
Bug 1147653 - Add larger green check assets. r=sebastian

Note that these assets will be consolidated in bug 1212629.
Attachment #8673970 - Flags: review?(s.kaspari)
Bug 1147653 - Remove share overlay toast code. r=sebastian

The upcoming commits will replace the toast with an alternative confirmation.
Note that this new confirmation will occur *before* the share event, meaning we
can't show the user whether the share actually passed or failed. However,
failure is essentially non-existent so we don't care.
Attachment #8673971 - Flags: review?(s.kaspari)
Bug 1147653 - Remove newly unused Strings. r=sebastian

These Strings' callsites were removed in the previous commit.
Attachment #8673972 - Flags: review?(s.kaspari)
Bug 1147653 - Update share overlay confirmation screen. r=sebastian

This new screen better fits the style of the share overlay.
Attachment #8673973 - Flags: review?(s.kaspari)
Bug 1147653 - Set fullscreen click listener to dismiss animation early. r=sebastian
Attachment #8673974 - Flags: review?(s.kaspari)
Comment on attachment 8673970 [details]
MozReview Request: Bug 1147653 - Add larger green check assets. r=sebastian

https://reviewboard.mozilla.org/r/22169/#review19805
Attachment #8673970 - Flags: review?(s.kaspari) → review+
Attachment #8673971 - Flags: review?(s.kaspari) → review+
Comment on attachment 8673971 [details]
MozReview Request: Bug 1147653 - Remove share overlay toast code. r=sebastian

https://reviewboard.mozilla.org/r/22171/#review19815
Comment on attachment 8673972 [details]
MozReview Request: Bug 1147653 - Remove newly unused Strings. r=sebastian

https://reviewboard.mozilla.org/r/22173/#review19817
Attachment #8673972 - Flags: review?(s.kaspari) → review+
Comment on attachment 8673973 [details]
MozReview Request: Bug 1147653 - Update share overlay confirmation screen. r=sebastian

https://reviewboard.mozilla.org/r/22175/#review19901

r+ with AnimationSet (or is there a compelling reason to use a Thread?)

::: mobile/android/base/overlays/ui/ShareDialog.java:453
(Diff revision 1)
> +            ThreadUtils.postDelayedToUiThread(new Runnable() {
> -            @Override
> +                @Override
> -            public void onAnimationStart(Animation animation) {
> -                // Unused. I can haz Miranda method?
> +                public void run() {
> +                    check.startAnimation(checkExitAnim);
> +                }
> +            }, exitWaitMillis);

Instead of creating a background thread, you could use setStartOffset() on checkExitAnim, then add both to an AnimationSet and just start the set.
Attachment #8673973 - Flags: review?(s.kaspari) → review+
Comment on attachment 8673974 [details]
MozReview Request: Bug 1147653 - Set fullscreen click listener to dismiss animation early. r=sebastian

https://reviewboard.mozilla.org/r/22177/#review19905

Nice! Unfortunately I can't login into sync on my own builds currently. So I could not see the result. :)
Attachment #8673974 - Flags: review?(s.kaspari) → review+
(In reply to Sebastian Kaspari (:sebastian) from comment #43)
> r+ with AnimationSet (or is there a compelling reason to use a Thread?)

No, I was just unaware of the AnimationSet APIs.
Tested using:
Device: LG G4 (Android 5.1)
Build: Firefox for Android 44.0a1 (2015-10-19)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: