Rearrange Sync prefs panel

VERIFIED FIXED in 2.0

Status

Cloud Services
Firefox Sync: UI
--
major
VERIFIED FIXED
8 years ago
4 years ago

People

(Reporter: Maaren, Assigned: philikon)

Tracking

unspecified
Points:
---
Dependency tree / graph
Bug Flags:
blocking-weave1.0 -
blocking-weave1.3 -
in-litmus +

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: [qa!][strings][has patch][softblocker])

Attachments

(13 attachments, 4 obsolete attachments)

92.66 KB, image/png
Details
63.93 KB, image/png
Details
230.99 KB, image/png
Details
66.78 KB, image/jpeg
Details
64.65 KB, image/jpeg
Details
4.34 KB, patch
mconnor
: review+
Details | Diff | Splinter Review
41.02 KB, image/png
Details
78.73 KB, image/png
Details
33.10 KB, image/png
Details
52.64 KB, image/png
Details
21.39 KB, patch
Details | Diff | Splinter Review
8.59 KB, patch
Details | Diff | Splinter Review
30.24 KB, image/png
Details
(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.4) Gecko/20091016 Firefox/3.5.4
Build Identifier: Weave 0.8

Just expand everything
-> panel isn't big enough
(see image)

just don't work with arrows, and set the other items there by default?
(eventually grayed out like in the privacy pref. panel)

Reproducible: Always
(Reporter)

Updated

8 years ago
Severity: normal → minor
(Reporter)

Comment 1

8 years ago
Created attachment 410187 [details]
The preference panel of Weave 0.8 is to small
Yeah, either the pane needs to expand, or they can't both be open at once. Taking.
Assignee: nobody → mconnor
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-weave1.0+
Target Milestone: --- → 1.0beta
http://hg.mozilla.org/labs/weave/rev/0ca68f1cb53d
fix this properly for Mac (actually animate/resize appropriately)

Still need to make this work well for Windows/Linux.

Updated

8 years ago
Duplicate of this bug: 527526
Going to punt this, it's okay for beta (manage account is rarely used, but when it is, it's fully accessible) for Windows/Linux.
Target Milestone: 1.0beta → 1.0
Created attachment 412869 [details]
screenshot on Ubuntu, with "Unknown error" bar pushing stuff even lower

Note that this bug is exacerbated by the bright-red "Unknown Error" bar described in bug 528855.  See attached screenshot, taken from Weave 1.0b1 on a mozilla-central nightly.

(So far this bug hasn't a problem on Ubuntu -- everything fits, even when expanded -- but the error bar pushes us just beyond the available space.)

Any solution to this bug needs to take that (usually-hidden) error bar into consideration. (assuming that it's still a part of Weave when this gets fixed)
(In reply to comment #6)
> Any solution to this bug needs to take that (usually-hidden) error bar into
> consideration.

s/Any solution/A complete solution/

Comment 8

8 years ago
In the Browser Sync -> Use my custom settings, instead of a VBOX put the five checkboxes in a two column grid.

Browser Sync? Doesn't Weave support Thunderbird?
This is a lot better now, just a bit of minor cosmetic stuff now.  Punting to 1.1.

(In reply to comment #8)
> In the Browser Sync -> Use my custom settings, instead of a VBOX put the five
> checkboxes in a two column grid.
> 
> Browser Sync? Doesn't Weave support Thunderbird?

Not really, removing support for now in Bug 533580
Flags: blocking-weave1.0+ → blocking-weave1.0-
Priority: -- → P3
Target Milestone: 1.0 → 1.1

Updated

8 years ago
Target Milestone: 1.1 → 1.2

Updated

7 years ago
Assignee: mconnor → paul
Flags: blocking-weave1.3+
OS: Mac OS X → All
Priority: P3 → --
Hardware: x86_64 → All
Target Milestone: 1.2 → 1.3

Updated

7 years ago
Flags: blocking-weave1.3+ → blocking-weave1.3-

Updated

7 years ago
Whiteboard: [b2]

Updated

7 years ago
Duplicate of this bug: 561463

Updated

7 years ago
Whiteboard: [b2]
Target Milestone: 1.3 → 1.3b3
(Reporter)

Updated

7 years ago
Depends on: 563349

Comment 11

7 years ago
I tested 1.3b5, and still this bug occurs. Now aren't displayed also Terms of Service, and Privacy Policy.
Severity: minor → major

Updated

7 years ago
Assignee: paul → nobody
Target Milestone: 1.3b3 → 2.0
This is fixed on trunk on all platforms.
Assignee: nobody → philipp
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Whiteboard: [qa+]
I'm using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100929 Firefox/4.0b7pre Built from http://hg.mozilla.org/mozilla-central/rev/942b5b6e74e4
and it's still not fixed.
Correct, when you open the Manage Account drop down, the stuff below still gets pushed off the screen on Windows. We have plans to redesign this prefpane, though, don't we? Perhaps we should morph this bug into that.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #14)
> Perhaps we should morph this bug into that.

(Or, if that ends up as a separate bug, then this one should depend on that one.)
(Assignee)

Updated

7 years ago
Duplicate of this bug: 617002
blocking2.0: --- → ?
No update in a couple of months. Is this still a problem? Sync people, can you comment as to why this should block (or not)?
I saw this yesterday on Windows.

Updated

7 years ago
blocking2.0: ? → beta9+

Updated

7 years ago
Keywords: uiwanted
I assume what needs to be fixed here is that the pref pane needs to expand further to accommodate the error message? 

If you want a new UI for this, add back "uiwanted", but I think the bug is in the preference pane size calculation at the moment? Tested this using the current nightly on OS X, and it works for me — but maybe it's platform-specific? (I was unable to trigger the Sync error message, though)
Keywords: uiwanted
(In reply to comment #19)
> I assume what needs to be fixed here is that the pref pane needs to expand
> further to accommodate the error message? 

No. We want to redesign the Sync prefpane. It's a clusterfuck right now with that Manage Account expander. mconnor, faaborg and I sat down at the All Hands and discussed the redesign and I believe faaborg had some mockups in the works already.

> If you want a new UI for this, add back "uiwanted", but I think the bug is in
> the preference pane size calculation at the moment?

It's *a* bug, but not this bug.

> Tested this using the
> current nightly on OS X, and it works for me — but maybe it's
> platform-specific? (I was unable to trigger the Sync error message, though)

It is platform specific. I think we can't dynamically resize dialogs on Windows.
Keywords: uiwanted
(In reply to comment #20)
> No. We want to redesign the Sync prefpane. It's a clusterfuck right now with
> that Manage Account expander. mconnor, faaborg and I sat down at the All Hands
> and discussed the redesign and I believe faaborg had some mockups in the works
> already.

Aha. I didn't know about that, sorry. Just trying to resolve the uiwanted bugs for the current blockers. :)
(Assignee)

Updated

7 years ago
Summary: Weave preferences panel is not big enough to contain all its contents → Rearrange Sync prefs panel

Comment 22

7 years ago
(In reply to comment #20)
> > If you want a new UI for this, add back "uiwanted", but I think the bug is in
> > the preference pane size calculation at the moment?
> 
> It's *a* bug, but not this bug.

Hrm... well, it *was* this bug, until it got morphed into what it is now. Should someone file a new bug for the stuff getting pushed out of the dialog? In my view, that bug is more likely to remain a blocker than redesigning the UI (although if that happens, then great).

Comment 23

7 years ago
Sorry, please ignore that last comment - I've just noticed that bug 563349 is already filed and depending on this.
Created attachment 500428 [details]
New prefpane layout

This layout ensures that the UI won't overflow (pushing the terms of service and privacy policy off scree), even if we add more account options or data types.

It also more logically groups actions that are taken on the account, versus actions that are taken on the machine (at the moment it is easy to believe that a data type is for the specific machine, and not all machines associated with the account).
Keywords: uiwanted
(In reply to comment #24)
> Created attachment 500428 [details]
> New prefpane layout

Lovely, thank you! What's the difference between "Stop Using This Account" in the drop down menu and the "Deactivate this device" link down at the bottom?
I should also note that the Deactivate this device link would require a new string and that the glyphs in the drop down menu don't exist yet either. (We don't have any of faaborg's nice mockup glyphs yet in the wizards -- I guess it's all part of bug 591122.)
Sorry, forgot to add a note about that.  I added "stop using this account" for the time being, since we have the string.  But later, I think we should remove that and switch to "deactivate this device" since the action is being carried out on the device, and not the whole account.  Also it has a nice symmetry to the control for adding a device.  At the moment "stop using this account" seems to imply that it will deactivate all of your devices, and possibly destroy all of your data, when in reality it just cuts off access for the current device, and removes the local data (right?).
(In reply to comment #27)
> Sorry, forgot to add a note about that.  I added "stop using this account" for
> the time being, since we have the string.  But later, I think we should remove
> that and switch to "deactivate this device" since the action is being carried
> out on the device, and not the whole account.  Also it has a nice symmetry to
> the control for adding a device.

I agree! Apart from the string freeze, is there anything that's keeping us from doing the change now? I'd say let's change this while we're at it... tentatively adding [strings].

> At the moment "stop using this account" seems
> to imply that it will deactivate all of your devices, and possibly destroy all
> of your data, when in reality it just cuts off access for the current device,
> and removes the local data (right?).

Right (if by removing local data you mean removing Sync settings -- it doesn't remove anything you've synced).
Whiteboard: [qa+] → [qa+][strings]
>Apart from the string freeze, is there anything that's keeping us from
>doing the change now?

Nope, just the string freeze

>Right (if by removing local data you mean removing Sync settings -- it doesn't
>remove anything you've synced).

Right, much later if we implement multiple user accounts, we'll at that point probably want to actually clear off the device (the assumption is that you are giving the computer away).  You can of course then undo the operation  by simply logging in again.

Comment 30

7 years ago
Addons.mozilla.org just mass mailed addon authors that it's safe to update their extensions to 4.0.*
<http://blog.mozilla.com/addons/2010/12/29/add-ons-can-now-be-compatible-with-4-0/>
Presumably this includes extensions that add their own entry to the list of services synced?
(In reply to comment #30)
> Addons.mozilla.org just mass mailed addon authors that it's safe to update
> their extensions to 4.0.*
> <http://blog.mozilla.com/addons/2010/12/29/add-ons-can-now-be-compatible-with-4-0/>
> Presumably this includes extensions that add their own entry to the list of
> services synced?

Currently we have no plans making that list extensible. That doesn't mean we can't do it, but it's a separate issue. Please file a new bug if you want to discuss this, this bug is purely about rearranging the existing UI.

Updated

7 years ago
Duplicate of this bug: 563349

Comment 33

7 years ago
Created attachment 500518 [details]
tabs option is cropped

Comment 34

7 years ago
Created attachment 500519 [details]
both tabs and history options are cropped

Updated

7 years ago
Blocks: 615950
(Assignee)

Updated

7 years ago
Duplicate of this bug: 583441

Updated

7 years ago
Status: REOPENED → ASSIGNED

Comment 36

7 years ago
As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if  you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)
blocking2.0: beta9+ → betaN+
No longer depends on: 563349

Updated

7 years ago
Depends on: 563349
(Assignee)

Updated

7 years ago
Blocks: 616488
Created attachment 501275 [details] [diff] [review]
v1

Rearranges the Sync prefs panel according to faaborg's mockup. It does not add any of the additional glyphs the mockup contains. It also does not contain the string change. I will do this in a separate patch so that we can always decide whether we want to take it or not (I think we should.)

This also fixes bug 616488 since I had to move that code around anyway.
Attachment #501275 - Flags: review?(mconnor)
Created attachment 501276 [details] [diff] [review]
Part 3: String change (v1)

Move the "Stop Using This Account" menu item to a "Deactivate This Device" label, as suggested by the mockups.
Attachment #501276 - Flags: review?(mconnor)
(Assignee)

Updated

7 years ago
Whiteboard: [qa+][strings] → [qa+][strings][has patch][needs review mconnor]
Created attachment 501430 [details] [diff] [review]
Part 1 (v1.1): Rearrange prefs panel

Move the CSS for the height of the engines list into theme code, that way it can be different on various platforms. Make it a tad bigger on Windows.
Attachment #501275 - Attachment is obsolete: true
Attachment #501430 - Flags: review?(mconnor)
Attachment #501275 - Flags: review?(mconnor)
One thing that the v1.1 doesn't do yet is remove the Connect button. I've thought long and hard about this, but I can't find a case where we'd still need it in the UI, especially with bug 616488 fixed in this patch as well.
Created attachment 501438 [details]
Win7 screenshot of v1.1
Created attachment 501441 [details]
Mac screenshot of v1.1
Created attachment 501442 [details]
screenshot of Wrong Sync Key error (v1.1 patch)
Created attachment 501444 [details]
screenshot of Incorrect Password error (v1.1 patch)
Comparing the screenshots with faaborg's mockups, I see that we could use a bit more padding for the groupboxes and maybe a little less around the checkboxes. I'd like to get faaborg's feedback first, though.

Alex?
Whiteboard: [qa+][strings][has patch][needs review mconnor] → [qa+][strings][has patch][needs review mconnor][needs feedback faaborg]
(Assignee)

Updated

7 years ago
Attachment #501430 - Attachment description: v1.1 → Part 1 (v1.1): Rearrange prefs panel
Attachment #501430 - Attachment is patch: true
Attachment #501430 - Attachment mime type: application/octet-stream → text/plain
(Assignee)

Updated

7 years ago
Attachment #501276 - Attachment description: string change v1 → Part 2: String change (v1)
Comment on attachment 501438 [details]
Win7 screenshot of v1.1

faaborg: There's an instance of inconsistent wording in this dialog.  It should be either "Device Name", or "Deactivate This Computer", but not the current mix of "Computer" and "Device" both referencing the same thing.  "Add a Computer" doesn't make any sense at all, so I suggest "Device Name" as the fix.
Comment on attachment 501441 [details]
Mac screenshot of v1.1

Actually, this isn't a select pulldown, so it shouldn't look like one. It's an "inline menu", so should look more like the Bookmarks button in our UI — an equivalent in OS X is the little "Gear" menu in the Finder.

The select pulldown implies that you're making one of the options active, which isn't what is happening here. :)
(In reply to comment #47)
> Actually, this isn't a select pulldown, so it shouldn't look like one.

I know! It *is* a <button type="menu">, so I think I'm using the right widget and it's just wrongly styled by default. So yell at the XUL guys plz :p

> It's an "inline menu", so should look more like the Bookmarks button in our
> UI — an equivalent in OS X is the little "Gear" menu in the Finder.

These don't have text, though.

> The select pulldown implies that you're making one of the options active, which
> isn't what is happening here. :)

Yup, agreed. Will see if we can steal a style from the toolbar buttons which seem to be appropriately styled. This still feels like an upstream XUL bug that we maybe should address. Will investigate and possibly file follow-up.

Updated

7 years ago
Attachment #501276 - Flags: review?(mconnor) → review+
Ok, I dabbled with the Mac style last night, but I couldn't get it to do what you wanted. At this point we should defer this a follow up. I filed bug 623722 on the XUL/widget guys.

Comment 50

7 years ago
As I said in bug 623722 the currently styled button looks like a "Command Pop-Down Menu" Note that it differs from a menulist.

From the Apple HIH;
"A command pop-down menu is similar to a pull-down menu, but it appears within
a window rather than in the menu bar. A command pop-down menu presents a list
of commands that affect the containing window’s contents.

The colors window (in for example textedit) has an example (the palette view).

So, I'm not sure the current styling is wrong. It might not be what you want, though.

Comment 51

7 years ago
Fwiw, here's a link to an image of the colors window:
http://developer.apple.com/library/mac/documentation/UserExperience/Conceptual/AppleHIGuidelines/art/ct_openpopdownmenu.jpg
But even in that screen shot there is another visually identical control that behaves differently (selecting the image that is displayed, currently set to spectrum).  While this perhaps isn't technically a problem, a toolbar style drop down seems like it would disambiguate the type of control a bit more. (oh how I wish these HIGs were wikis... :)

Comment 53

7 years ago
(In reply to comment #52)
> But even in that screen shot there is another visually identical control that
> behaves differently (selecting the image that is displayed, currently set to
> spectrum).

Hey, it has 2 arrows ;-)
(Assignee)

Updated

7 years ago
Attachment #501276 - Attachment description: Part 2: String change (v1) → Part 3: String change (v1)
Created attachment 501891 [details] [diff] [review]
Part 2 (v1): Get rid of Connect/Disconnect button
Attachment #501891 - Flags: review?(mconnor)
(Assignee)

Updated

7 years ago
Blocks: 590763
Limi just said in bug 623722 comment 9:

> I wouldn't make it block anything, and I'm fine with shipping the aqua-style
> down arrow inline menu in FF4 — but it does look weird and feel weird. :)

So pending reviews from mconnor and ui feedback from faaborg, I think this is good to go.

Updated

7 years ago
Attachment #501430 - Flags: review?(mconnor) → review+

Updated

7 years ago
Attachment #501891 - Flags: review?(mconnor) → review+

Updated

7 years ago
Whiteboard: [qa+][strings][has patch][needs review mconnor][needs feedback faaborg] → [qa+][strings][has patch][needs feedback faaborg]
Created attachment 502989 [details] [diff] [review]
Part 1 (v1.2): Rearrange prefs panel

Update Part 1 slightly to provide 10px padding on Windows for the groupboxes, as per faaborg's offline feedback. This is ready for landing now.
Attachment #501430 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Whiteboard: [qa+][strings][has patch][needs feedback faaborg] → [qa+][strings][has patch]
needs litmus tests when this is fixed
Flags: in-litmus?(twalker)
Created attachment 503042 [details] [diff] [review]
Part 1 (v1.3): Rearrange prefs panel

Discovered a small bug: we would switch to the Update/Reset page on any login error, even when it was due to the network or server. This now makes sure we only go to that page on wrong password or sync key.
Attachment #502989 - Attachment is obsolete: true
Created attachment 503043 [details] [diff] [review]
Part 2 (v1.1): Get rid of Connect/Disconnect button

Rebase on top of Part 1 (v1.3). All observers now go through updateWeavePrefs, so simplified the observer registering logic as well.
Attachment #501891 - Attachment is obsolete: true
Whiteboard: [qa+][strings][has patch] → [qa+][strings][has patch][softblocker]
Got r+ from mconnor via IRC for the updated bits, landed:

Part 1: https://hg.mozilla.org/mozilla-central/rev/b1a940a22511
Part 2: https://hg.mozilla.org/mozilla-central/rev/2ec3f1dcd2c4
Part 3: https://hg.mozilla.org/mozilla-central/rev/c0e05d518f57
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
Not sure if this is the right place to ask, but this bug is the only reference I have from Mercuarial's history of /browser/preferences/sync.dtd. 

Why exactly did you choose the word "deactivate"? If the original action is "Add a device", the logical opposite should be "Remove this device". An alternative, knowing what the label does, could be "Disconnect this device". 

I'm asking because I have to translate this string and I'd like to know the reasons for this choice.
Disconnect might imply that it is a momentary action (especially since we previously had a button for connecting and disconnecting).  So we needed a stronger word.  Remove would work just as well, but deactivate seemed even stronger, and got across that the device would definitely no longer have access to the account.
(Assignee)

Updated

7 years ago
Blocks: 623737

Comment 63

7 years ago
Created attachment 504404 [details]
Sync panel - new design

Comment 64

7 years ago
Maybe is too late, but I  have new mockup for Sync window. See attached image. What is a plus:
Clear window
Less clicks for get function
Accesskeys - very important for many users
Buttons width going to translated strings automatically
Clear XUL code
Minus:
Have to add accesskeys for translation.

I have ready to use sync.xul file.
verified with current nightly build
Status: RESOLVED → VERIFIED
No longer blocks: 615950
Blocks: 634419
Blocks: 636839
Comment on attachment 503042 [details] [diff] [review]
Part 1 (v1.3): Rearrange prefs panel

>+              <richlistbox id="syncEnginesList"
>+                           orient="vertical"
>+                           onselect="if (this.selectedCount) this.clearSelection();">
What's the point of a richlistbox if you're not going to use it as a list?
If you want a scrollable region, just set an overflow style...
(Assignee)

Updated

6 years ago
Duplicate of this bug: 597170
yes, covered and recovered in-litmus
Flags: in-litmus?(twalker) → in-litmus+

Updated

5 years ago
Whiteboard: [qa+][strings][has patch][softblocker] → [qa!][strings][has patch][softblocker]
You need to log in before you can comment on or make changes to this bug.