Closed
Bug 526445
Opened 15 years ago
Closed 14 years ago
Rearrange Sync prefs panel
Categories
(Firefox :: Sync, defect)
Firefox
Sync
Tracking
()
VERIFIED
FIXED
2.0
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: for.bugzilla, Assigned: philikon)
References
Details
(Whiteboard: [qa!][strings][has patch][softblocker])
Attachments
(13 files, 4 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/jpeg
|
Details | |
(deleted),
image/jpeg
|
Details | |
(deleted),
patch
|
mconnor
:
review+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
image/png
|
Details |
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
Comment 2•15 years ago
|
||
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
Comment 3•15 years ago
|
||
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.
Comment 5•15 years ago
|
||
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
Comment 6•15 years ago
|
||
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)
Comment 7•15 years ago
|
||
(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•15 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?
Comment 9•15 years ago
|
||
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•15 years ago
|
Target Milestone: 1.1 → 1.2
Updated•15 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•15 years ago
|
Flags: blocking-weave1.3+ → blocking-weave1.3-
Updated•15 years ago
|
Whiteboard: [b2]
Updated•15 years ago
|
Whiteboard: [b2]
Target Milestone: 1.3 → 1.3b3
Comment 11•15 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•15 years ago
|
Assignee: paul → nobody
Target Milestone: 1.3b3 → 2.0
Comment 12•14 years ago
|
||
This is fixed on trunk on all platforms.
Assignee: nobody → philipp
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Whiteboard: [qa+]
Comment 13•14 years ago
|
||
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.
Assignee | ||
Comment 14•14 years ago
|
||
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 → ---
Comment 15•14 years ago
|
||
(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.)
Updated•14 years ago
|
blocking2.0: --- → ?
Comment 17•14 years ago
|
||
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)?
Comment 18•14 years ago
|
||
I saw this yesterday on Windows.
Updated•14 years ago
|
blocking2.0: ? → beta9+
Comment 19•14 years ago
|
||
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
Assignee | ||
Comment 20•14 years ago
|
||
(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
Comment 21•14 years ago
|
||
(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•14 years ago
|
Summary: Weave preferences panel is not big enough to contain all its contents → Rearrange Sync prefs panel
Comment 22•14 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•14 years ago
|
||
Sorry, please ignore that last comment - I've just noticed that bug 563349 is already filed and depending on this.
Comment 24•14 years ago
|
||
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).
Assignee | ||
Comment 25•14 years ago
|
||
(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?
Assignee | ||
Comment 26•14 years ago
|
||
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.)
Comment 27•14 years ago
|
||
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?).
Assignee | ||
Comment 28•14 years ago
|
||
(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]
Comment 29•14 years ago
|
||
>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•14 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?
Assignee | ||
Comment 31•14 years ago
|
||
(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.
Comment 33•14 years ago
|
||
Comment 34•14 years ago
|
||
Updated•14 years ago
|
Status: REOPENED → ASSIGNED
Comment 36•14 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
Assignee | ||
Comment 37•14 years ago
|
||
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)
Assignee | ||
Comment 38•14 years ago
|
||
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•14 years ago
|
Whiteboard: [qa+][strings] → [qa+][strings][has patch][needs review mconnor]
Assignee | ||
Comment 39•14 years ago
|
||
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)
Assignee | ||
Comment 40•14 years ago
|
||
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.
Assignee | ||
Comment 41•14 years ago
|
||
Assignee | ||
Comment 42•14 years ago
|
||
Assignee | ||
Comment 43•14 years ago
|
||
Assignee | ||
Comment 44•14 years ago
|
||
Assignee | ||
Comment 45•14 years ago
|
||
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•14 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•14 years ago
|
Attachment #501276 -
Attachment description: string change v1 → Part 2: String change (v1)
Comment 46•14 years ago
|
||
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 47•14 years ago
|
||
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. :)
Assignee | ||
Comment 48•14 years ago
|
||
(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•14 years ago
|
Attachment #501276 -
Flags: review?(mconnor) → review+
Assignee | ||
Comment 49•14 years ago
|
||
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•14 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•14 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
Comment 52•14 years ago
|
||
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•14 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•14 years ago
|
Attachment #501276 -
Attachment description: Part 2: String change (v1) → Part 3: String change (v1)
Assignee | ||
Comment 54•14 years ago
|
||
Attachment #501891 -
Flags: review?(mconnor)
Assignee | ||
Comment 55•14 years ago
|
||
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•14 years ago
|
Attachment #501430 -
Flags: review?(mconnor) → review+
Updated•14 years ago
|
Attachment #501891 -
Flags: review?(mconnor) → review+
Updated•14 years ago
|
Whiteboard: [qa+][strings][has patch][needs review mconnor][needs feedback faaborg] → [qa+][strings][has patch][needs feedback faaborg]
Assignee | ||
Comment 56•14 years ago
|
||
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•14 years ago
|
Whiteboard: [qa+][strings][has patch][needs feedback faaborg] → [qa+][strings][has patch]
Assignee | ||
Comment 58•14 years ago
|
||
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
Assignee | ||
Comment 59•14 years ago
|
||
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
Updated•14 years ago
|
Whiteboard: [qa+][strings][has patch] → [qa+][strings][has patch][softblocker]
Assignee | ||
Comment 60•14 years ago
|
||
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
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Comment 61•14 years ago
|
||
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.
Comment 62•14 years ago
|
||
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.
Comment 63•14 years ago
|
||
Comment 64•14 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.
Comment 66•14 years ago
|
||
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...
Comment 68•13 years ago
|
||
yes, covered and recovered in-litmus
Flags: in-litmus?(twalker) → in-litmus+
Updated•13 years ago
|
Whiteboard: [qa+][strings][has patch][softblocker] → [qa!][strings][has patch][softblocker]
Updated•6 years ago
|
Component: Firefox Sync: UI → Sync
Product: Cloud Services → Firefox
You need to log in
before you can comment on or make changes to this bug.
Description
•