581623 - bring back some status UI

Status: VERIFIED FIXED

(Firefox :: Sync, defect)

Description

[Mike Connor [:mconnor]]

Attachment: v1

We've gone around and around on this, but I think this works until we have a better sense of Fx4 plans.

Key changes:

* 16x16 Sync icon in the status bar
** Has "Connect" label when not connected.  Clicking this will log you in. (one click, not two)
** No label when connected.  Clicking the icon will trigger a sync. (again, one click, not two)
** Throbber replaces icon when syncing/logging in (no text labels, but I think that's okay)
** If you want to disconnect, you can use the menu, but this is a fairly rare use-case.

* Notification titles are now displayed in the status bar next to the icon
** This makes the notification much more prominent/identified with Sync, for better or for worse.

Comment 1

[Philipp von Weitershausen [:philikon]]

Comment on attachment 459986 [details] [diff] [review]
v1

I'm not a big fan of the implicit clickability of the "Connect" label. It's sort of a "Connect" button disguised as a label. But it's far better than having no indicator of connectedness at all.

Comment 2

[Mike Connor [:mconnor]]

Attachment: v2 wip

* from feedback, adds (or tries to, not sure why it's not working) a tooltip using the lastSync label
* also hides Connect when setup is needed.  It should also probably call openPrefs in that instance.

Comment 3

[Philipp von Weitershausen [:philikon]]

Attachment: v3

Improves on Mike's WIP as follows:

* When Sync isn't set up yet, clicking on the Sync icon will open the setup wizard.

* Display tooltip with last sync info as soon as it's connected. Don't show a tooltip when disconnected.

Comment 5

[Ed Lee :Mardak]

If it's going to act like a button, why not make it a toolbarbutton that people can customize? I don't think there's existing glyphs that change appearance (other than disabled), so it might be odd..

But potentially there could be a glyph for 1) not connected 2) have data to sync 3) in-sync (disabled?) where the behavior would be 1) connect then sync (or open account creation) 2) sync 3) nothing.

Re: sync throbber. Did Alex mention somewhere that the sync icon shouldn't be spinning anywhere?

Comment 6

[Philipp von Weitershausen [:philikon]]

I'm thinking along similar lines, Ed, but I think the focus of this bug is to fix the 1.4.x line. It all seems a bit elaborate for a stopgap, especially if we don't know what the Fx 4 plans are.

Comment 7

[Philipp von Weitershausen [:philikon]]

Attachment: fix OSX styling

Fix padding issue on OSX in icon-only mode.

Comment 8

[Philipp von Weitershausen [:philikon]]

Attachment: v4

Fix edge case: hide "Connect" label when user chooses "Use different account"

Comment 9

[Ed Lee :Mardak]

Comment on attachment 460844 [details] [diff] [review]
v4

>+++ b/ui/firefox/content/sync.js
>+  updateUI: function updateUI() {
>+    if (window.location.href == getBrowserURL()) {
>+      let showLabel = !Weave.Service.isLoggedIn && !this._needsSetup();
>+      let button = document.getElementById("sync-status-button");
>+      button.setAttribute("class", showLabel ? "statusbarpanel-iconic-text" 
>+                                             : "statusbarpanel-iconic");
>+      button.image = "chrome://weave/skin/firefox/sync-16x16.png";
>+
>+      if (!Weave.Service.isLoggedIn) {
>+        // If we're not set up yet, the tooltip should read "Set up
>+        // sync" or similar, but we threw the string away in 1.4 :(
>+        button.removeAttribute("tooltiptext");
>+      }
Did we want to set the tooltiptext to Connect or Sync Now in various cases just like how you set the last sync time?

>+  handleSyncButton: function WeaveWin_handleSyncButton() {
>+    if (Weave.Service.isLoggedIn)
>+      this.doSync();
>+    else if (this._needsSetup())
>+      this.openSetup();
>+    else
>+      this.doLogin();
Do we want connect-n-sync? We don't have a string for that though. I suppose just a connect is good for now.. and we might trigger a sync anyway if the user hasn't synced for a while?

>   onNotificationAdded: function WeaveWin_onNotificationAdded() {
>+    for (let i = 0;i < notifications.length;i++) {
nit: fix up the spacing while you're modifying

>   onNotificationRemoved: function WeaveWin_onNotificationRemoved() {
>+    if (Weave.Notifications.notifications.length == 0) {
>+    else
>+      this.onNotificationAdded();
A note why you're doing this would be good. Making sure the right icon is shown etc. for the remaining notifications, yeah?

Comment 10

[Philipp von Weitershausen [:philikon]]

(In reply to comment #9)
> Did we want to set the tooltiptext to Connect or Sync Now in various cases just
> like how you set the last sync time?

Indeed.

> Do we want connect-n-sync?

Why?

> We don't have a string for that though. I suppose
> just a connect is good for now.. and we might trigger a sync anyway if the user
> hasn't synced for a while?

Correct.

Comment 11

[Ed Lee :Mardak]

(In reply to comment #10)
> > Do we want connect-n-sync?
> Why?
Feels like people will just as easily click the button multiple times until it starts spinning now. But I suppose it'll spin while connecting.

Comment 12

[Philipp von Weitershausen [:philikon]]

Attachment: v5

Addressed review comments.

Comment 13

[Philipp von Weitershausen [:philikon]]

(In reply to comment #11)
> Feels like people will just as easily click the button multiple times until it
> starts spinning now. But I suppose it'll spin while connecting.

Yes. Not sure about that myself, but then again, the spinning icon is going away sooner or later anyway...

Comment 14

[Philipp von Weitershausen [:philikon]]

1.4.x:
part 1: http://hg.mozilla.org/services/fx-sync/rev/49ed6fd78e75
part 2: http://hg.mozilla.org/services/fx-sync/rev/cdf26164125a

default:
part 1: http://hg.mozilla.org/services/fx-sync/rev/ecc5595d0ac3
part 2: http://hg.mozilla.org/services/fx-sync/rev/bdea0f184ca7

Comment 15

[Tony Chung [:tchung]]

(In reply to comment #0)
> Key changes:
> 
> * 16x16 Sync icon in the status bar
Verified on 1.4.3b1
> ** Has "Connect" label when not connected.  Clicking this will log you in. (one
> click, not two)
Verified on 1.4.3b1
> ** No label when connected.  Clicking the icon will trigger a sync. (again, one
> click, not two)
Verified on 1.4.3b1
> ** Throbber replaces icon when syncing/logging in (no text labels, but I think
> that's okay)
Verified on 1.4.3b1
> ** If you want to disconnect, you can use the menu, but this is a fairly rare
> use-case.
Verified on 1.4.3b1
> 
> * Notification titles are now displayed in the status bar next to the icon
> ** This makes the notification much more prominent/identified with Sync, for
> better or for worse.
Verified on 1.4.3b1

Comment 16

[Tony Chung [:tchung]]

i'll also leave myself a comment to re-verify this after bug 571897 lands on trunk and see how what happens if anything crazy happens