Closed Bug 609245 Opened 9 years ago Closed 8 years ago

usability problem: folder and address book panes may seem "gone" after accidentally collapsing them

Categories

(Thunderbird :: Address Book, defect, major)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 10.0

People

(Reporter: thunderbird, Assigned: squib)

References

(Depends on 1 open bug, Blocks 1 open bug, )

Details

(Whiteboard: [datalossy][gs][lion])

Attachments

(7 files, 3 obsolete files)

1.29 KB, patch
squib
: review+
andreasn
: ui-review+
Details | Diff | Splinter Review
49.70 KB, image/png
Details
25.72 KB, patch
squib
: review+
bwinton
: ui-review+
Details | Diff | Splinter Review
30.92 KB, patch
Details | Diff | Splinter Review
1.29 KB, patch
squib
: review+
Details | Diff | Splinter Review
1.26 KB, patch
Details | Diff | Splinter Review
4.12 KB, patch
squib
: feedback+
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.1.15) Gecko/20101026 Firefox/3.5.15
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.2.12) Gecko/20101027 Lightning/1.0b2 Thunderbird/3.1.6

In the address book window the "address books" pane on the left will suddenly collapse when dragging the vertical splitter to the left.

If this happens accidentally, especially while the user is not paying much attention, the address books appear to gone because the splitter looks just like part of an unusually wide window border. It does not indicate function. And even worse: This UI setting is remembered, so closing an re-opening the window does not help.

It seems natural to review the "view" menu in this case, but it offers no command to restore the address books pane.

For a user who is not actively maintaining his/her address books, the problem is aggravated by the fact that the first default address book "personal addresses" may be empty and all addresses are stored in "collected addresses".
As the first address book is always selected when opening the address books window (regardless of the last selected book), it might appear to the user that all addresses are simply gone.

Reproducible: Always

Steps to Reproduce:
1. open address book window
2. drag vertical splitter to the left beyond the point at which the address books pane instantly collapses
Actual Results:  
Address books appear to be gone and there is no menu command to make them reappear

Expected Results:  
Address books pane should not collapse completely.
Or if it has to:
- This should happen smoothly, not with an instant snap of the splitter to the left.
- The splitter, being at the far left, should have a strong visual cue that something is hidden and can be made to reappear by clicking/dragging.
- There should be a command in the view menu to toggle th address books pane visibility.

The described problem occurred to my girlfriend. After she complained to me that "all my addresses are gone", I investigated and it took me - as a software developer and long-time Thunderbird user - several minutes to spot the splitter in the far left (btw: the window was maximized, which makes it even harder to spot).

Others might be driven to reinstall Thunderbird, restore their profile from a backup or whatever.
Thanks for a great description. It is really hard, in fact impossible, to see that the splitter is all the way to the left.  And confirming - although I would have thought a bug already existed on this (besides bug 555423). (I took a short spin in getsatisfaction and it's too hard to find specific items there about AB because there are so many)

bug 555423 would help this.
Severity: minor → normal
Status: UNCONFIRMED → NEW
Depends on: 555423
Ever confirmed: true
Summary: usability problem: address book may seem "gone" after accidentally collapsing "address books" pane → usability problem: address book may seem "gone" after accidentally collapsing "address books" pane - need min size on AB sub-pane
Whiteboard: [datalossy]
I hit a state where I can't move the hidden splitter at all. Don't know why it got that way - earlier today I collapsed it and was able to move the splitter. I'm hosed till I reset to defaults.

http://getsatisfaction.com/mozilla_messaging/topics/why_cant_i_see_my_address_books may be an example of this bug. I'm sure there others but can't find any just now.
Severity: normal → major
The splitter is no more attainable in collapsed state because the negative margin is on the wrong side. I'll provide later a patch for this. For the other functionality I can't help, but maybe it is no more needed after this patch.
This patch makes the horizontal and vertical splitter attainable in collapsed state. The splitter are showing in highlight color when hovering.

Patch from BUG 549306 should be applied before this patch can be applied.

I tried with -moz-transition to slow down the collapsing/uncollapsing, but this didn't work. If this is still desired, someone with enough knowledge can do this in a second patch.
Attachment #498078 - Flags: ui-review?(nisses.mail)
Attachment #498078 - Flags: review?(bwinton)
Comment on attachment 498078 [details] [diff] [review]
Patch making the splitters attainable

Looks good to me. I think we need this for the main window as well. Will file a new bug about that.
Attachment #498078 - Flags: ui-review?(nisses.mail) → ui-review+
So, why is this behaviour worse than it is on the Mac?  And if it isn't why do we want a Windows-only patch?

(I'm lazy, so I'm not going to boot up my Windows computer to review the patch until I get a good answer for this.  ;)

Thanks,
Blake.
Blake: part of the problem with Windows is that the splitter in the Aero theme is 1 (visible) pixel wide.

That said, I think being able to collapse the panes by dragging them is nice, and a better way to bring them back would be like so: when the pane is collapsed by dragging, uncheck the menu item in the View menu (e.g. View -> Folder pane, or wherever it is), and then to bring it back, you can either drag it back, or go into the view menu.
Comment on attachment 498078 [details] [diff] [review]
Patch making the splitters attainable

(In reply to comment #8)
> Blake: part of the problem with Windows is that the splitter in the Aero theme
> is 1 (visible) pixel wide.

Something like this:
http://o7.no/2301433/miniFolderPane.png

(Yeah, it's the folder list, not the address book, but it looks like the same thing to me.)

Since I can see the same problem on the Mac, I'm going to r- this, cause we really want to fix this on all the platforms.  (I do like the diff you posted.  It just doesn't go far enough for me.)

> That said, I think being able to collapse the panes by dragging them is nice,
> and a better way to bring them back would be like so: when the pane is
> collapsed by dragging, uncheck the menu item in the View menu (e.g. View ->
> Folder pane, or wherever it is), and then to bring it back, you can either
> drag it back, or go into the view menu.

That sounds _way_ better to me, and I would love to review a patch that added that to all the platforms.  ;)

Thanks,
Blake.
Attachment #498078 - Flags: review?(bwinton) → review-
(In reply to comment #9)
> Comment on attachment 498078 [details] [diff] [review]
> Patch making the splitters attainable
> Since I can see the same problem on the Mac, I'm going to r- this, cause we
> really want to fix this on all the platforms.

In attachment 498917 [details] the collapsed splitters are attainable on Mac. Now on Vista/Win7 they are in no way attainable. So the patch is needed for minimal usability now.

> (I do like the diff you posted. It just doesn't go far enough for me.)

This needs somebody with JS skills, which I don't have. I'm happy, if somebody want's to take this bug.
Just a quick note that this don't need a fix on Linux.
(In reply to comment #9)
> > That said, I think being able to collapse the panes by dragging them is nice,
> > and a better way to bring them back would be like so: when the pane is
> > collapsed by dragging, uncheck the menu item in the View menu (e.g. View ->
> > Folder pane, or wherever it is), and then to bring it back, you can either
> > drag it back, or go into the view menu.
> 
> That sounds _way_ better to me, and I would love to review a patch that added
> that to all the platforms.  ;)

Incidentally, this is what the message pane does, and I think it works pretty well, although when you drag it into nothing and then click "View -> Layout -> Message Pane", the message pane is very small since it's taking up the minimum height.
timeless also reports this issue on IRC.
His STR might be:
1. create a new profile
2. in that profile, don't create any accounts
3. open addressbook
4. do not create any accounts
5. quit
Taking this, but it might be a little while until I get to it, since my queue is pretty full right now...
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
Since this bug applies equally to the 3pane, I also added a menuitem for that. There are still some bugs here, namely that the checkboxes for the contact summary pane and the address book pane get out of sync if you collapse/expand the panes by dragging.

Mostly, I just want to make sure that the code here is moving in the right direction. I have a feeling some of this is stuff that should get refactored (maybe into messageDisplay and folderDisplay?). Any thoughts?
Attachment #546477 - Flags: feedback?(bugmail)
Duplicate of this bug: 340982
Comment on attachment 546477 [details] [diff] [review]
WIP: Add menuitems to uncollapse the address book and folder panes

I don't really have an opinion on where the logic should go; FolderDisplayWidget will persist the state when being made inactive regardless of how the toggles happen.  If you were planning on getting a review from me for this, feel free to carry forward r=asuth unless you make some kind of radical change.
Attachment #546477 - Flags: feedback?(bugmail) → review+
Summary: usability problem: address book may seem "gone" after accidentally collapsing "address books" pane - need min size on AB sub-pane → usability problem: folder and address book panes may seem "gone" after accidentally collapsing them
I cleaned this up a bit more and removed some duplicate code for collapsing splitters.

Also, the checkboxes for the panes in the address book are now initialized when the View menu is opened; before, their state was persisted across sessions, which was bad since they got out of sync if you dragged the pane to collapse it.

Asking for review again since I restructured most of the splitter toggling code.
Attachment #546477 - Attachment is obsolete: true
Attachment #546948 - Flags: review?(bugmail)
Comment on attachment 546948 [details] [diff] [review]
Fix inconsistency with checkboxes in address book

Hooray for fewer sharp, jabby edges!
Attachment #546948 - Flags: review?(bugmail) → review+
Attachment #546948 - Flags: ui-review?(bwinton)
Comment on attachment 546948 [details] [diff] [review]
Fix inconsistency with checkboxes in address book

Okay, I like it but…

In the address book, have both the address book and contact summary panes hidden.
Drag them out of the edges to show them.
Go to the View menu, and see that the entries don't have checks.

I think I can do the same with the Folder Pane (i.e. get into a state where it's showing, but the menu item isn't checked).

I'm also not really happy with the fact that the Pane items are in the Layout menu of the 3-pane, but directly in the View menu of the Address book.  I think perhaps we should move them (along with the Status Bar item) into the Layout menu in both places.

So, ui-r- until those are fixed, I think.  (Also, I didn't see any tests…  How much did you pay asuth to approve this without any tests?  ;)

Thanks,
Blake.
Attachment #546948 - Flags: ui-review?(bwinton) → ui-review-
Oh, one more thing, I think we should call it "Contact Pane" instead of "Contact Summary Pane" to match "Message Pane".  Then it would be nice if we could get a single word for "Address Book" in "Address Book Pane", but I wouldn't block on that…
I fixed the checkbox issue. The address book doesn't have a Layout submenu though; should I add one? Also, should I put the status bar item under Layout, or should I put it under Toolbars (which is where Firefox put the Add-on Bar)? I was thinking about doing the latter, but planned to file a followup for it.
(In reply to comment #21)
> Then it would be nice if we could get a single word for "Address Book" in
> "Address Book Pane", but I wouldn't block on that…

"Directory Pane"? (Note that we're getting rid of the "Address Books" sidebar header, so there won't be an inconsistency there.)
Yeah, add one.

Putting it under Toolbars for consistency makes sense to me.

I could live with "Directory Pane", although it sounds a little more technical than I would like.  Having said that, I can't think of anything better.

Thanks,
Blake.
Attached patch Address UI review comments (obsolete) — Splinter Review
Well, this patch got huge... here's what's changed:

* Tests!
* Moved "Status Bar" under "Toolbars" for 3pane, standalone message, address
  book, and compose windows
* Changed "Directory Pane" and "Contact Pane" names
* Moved "Directory Pane" and "Contact Pane" to "Layout" submenu
* Moved "Formatting Bar" to "Toolbars" submenu
* Fixed logic for setting the checkboxes in the address book
* Switched to using gFolderDisplay.folderPaneVisible for the folder pane
  checkbox

One thing of note is that in the 3pane, we now have "Mail Toolbar", "Status Bar", and "Quick Filter Bar" in that order listed under the toolbars menu. I'm not sure if that's good or bad; maybe the last two should be swapped since then the order would match their vertical position in Thunderbird.
Attachment #546948 - Attachment is obsolete: true
Attachment #547626 - Flags: ui-review?(bwinton)
Attachment #547626 - Flags: review?(bugmail)
Comment on attachment 547626 [details] [diff] [review]
Address UI review comments

Hooray for unit tests!

Please update the file comment (the one that closes with **/ in addition to opening with /**) in test-message-pane-visibility.js to mention that it tests the folder pane too.  Also, please do an hg rename to test-pane-visibility.js; I think mq may have or have had problems with renames, so I guess be careful if you do the rename and follow it up with queue manipulation.
Attachment #547626 - Flags: review?(bugmail) → review+
Ok, clearly I need to not write tests late at night, since apparently there was already a test-folder-pane-visibility.js that does everything I was doing in my patch (for the 3pane)...
Here's a version without the redundant tests. That'll teach me not to be careful about looking at all the filenames in mail/test/mozmill/folder-display...

I'm pulling forward r+ since the only real difference is that I removed test code and moved a couple of lines of tests to the proper file. Otherwise, the test is essentially identical (almost word-for-word even).
Attachment #547626 - Attachment is obsolete: true
Attachment #547892 - Flags: ui-review?(bwinton)
Attachment #547892 - Flags: review+
Attachment #547626 - Flags: ui-review?(bwinton)
Whiteboard: [datalossy] → [datalossy][gs]
Comment on attachment 547892 [details] [diff] [review]
Remove redundant tests

Okay, I've played around with this for a while, and I like it.

It would be even nicer if we could make the Contacts Sidebar in the Compose window work the same, but I'm happy to leave that to a separate patch.

ui-r=me!

Thanks,
Blake.
Attachment #547892 - Flags: ui-review?(bwinton) → ui-review+
Comment on attachment 547892 [details] [diff] [review]
Remove redundant tests

Okay, I've played around with this for a while, and I like it.

It would be even nicer if we could make the Contacts Sidebar in the Compose window work the same, but I'm happy to leave that to a separate patch.

ui-r=me!

Thanks,
Blake.
Checked in: http://hg.mozilla.org/comm-central/rev/9ee393ca9926
Flags: in-testsuite+
Target Milestone: --- → Thunderbird 8.0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
I had to back this out due to permanent orange on Mac and near-permanent on Windows:

http://hg.mozilla.org/comm-central/rev/899474322251
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Duplicate of this bug: 606045
(In reply to Mark Banner (:standard8) from comment #32)
> I had to back this out due to permanent orange on Mac and near-permanent on
> Windows:
> 
> http://hg.mozilla.org/comm-central/rev/899474322251

Could someone take a look at this on Windows/Mac? I have a feeling it's just a problem with the test code, but I'm not sure what exactly.
With the new resizing-with-any-edge feature in Lion, it's now impossible to re-expand the folder pane if you've collapsed it, so this bug has become a little more important to me, and thus I'll be looking at it today, if I can.

Thanks,
Blake.
(In reply to Jim Porter (:squib) from comment #36)
> Pushed to try:
> http://build.mozillamessaging.com/tinderboxpushlog/
> ?tree=ThunderbirdTry&rev=8068ec433904

all failed except mac64, looks like
I re-ran the try build and it looks like the only errors are unrelated to the changes in this bug: http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=6ef2e96b30ef
(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #35)
> With the new resizing-with-any-edge feature in Lion, it's now impossible

also mentioned in http://getsatisfaction.com/mozilla_messaging/topics/folders_sidebar_vanished?utm_content=reply_link&utm_medium=email&utm_source=reply_notification#reply_6587830
Whiteboard: [datalossy][gs] → [datalossy][gs][lion]
Duplicate of this bug: 686040
Blake, how do you plan to fix this bug? As mentioned in the duplicate, it would be good to also fix the sidebar container on the right side where we put our today pane, if your fix is general enough to be applied there too.
Philipp,

there should be a View menu option to re-expand the pane, and I think we may have to make the splitter a little bigger, possibly appearing only when it's collapsed.  Hopefully the second part of that will be applicable to the Today pane, too.
For anyone suffering from this and needing a workaround now, do the following:
1) Exit Thunderbird
2) Edit localstore.rdf in your profile directory
3) Search for "dirTree"
4) Under the dirTree-splitter heading that has the 'state' element, change it to state="open" (you may not see this on Linux.)
5) Under dirTreeBox, change 'collapsed' to collapsed="false" and set 'width' to something sensible if it isn't already (like 199)
6) Save, close, and restart Thunderbird. Breathe a sigh of relief.
Any objections to me pushing the patch for this to comm-central again? Having run try builds a few times[1], it seems that everything works. I believe I've made a couple of test-only changes to my local version of the patch, but it's fairly late, so I'll check on that tomorrow.

[1] Latest one is here: http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=739e877ad87b
I'ld really like to see this land before the merge, so I'll say yeah, push it, but we should watch the tree carefully once it lands.

Thanks,
Blake.
Checked in: http://hg.mozilla.org/comm-central/rev/a68fc7427f0d

Let's hope it sticks this time. :)
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Argh, looks like this broke some of the address book tests on Windows and Mac. I've got a try build[1] pending that will hopefully fix the failures. Otherwise, I'll just back it out (again).

[1] http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=ef6129a2f0b3
Backed out again: http://hg.mozilla.org/comm-central/rev/23f39809c091
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch Up-to-date patchSplinter Review
Mconley, could you take a look at this patch? The tests are causing failures in test_writing_to_mailing_list on Windows/Mac. Everything works for me on Linux though...
Attachment #572484 - Flags: feedback?(mconley)
Sure thing (kicks off Mac build)...
Squib:

Ok, did some poking around - what's happening is that the collapsed state of the directory pane is being persisted from test to test, and the test-address-book.js tests assume that the directory pane is visible.

What you need to do, I guess, is just ensure the panes are visible during test teardown.  That should allow the test-address-book.js tests to pass.

Hope that helps,

-Mike
If you toss this on top of 572484: Up-to-date patch, you should be good to go.
Attachment #572519 - Flags: review?(squibblyflabbetydoo)
Comment on attachment 572519 [details] [diff] [review]
Subpatch for 572484: Up-to-date patch to make address book Mozmill tests pass.

Looks good! I'll be pushing shortly.
Attachment #572519 - Flags: review?(squibblyflabbetydoo) → review+
Checked in: http://hg.mozilla.org/comm-central/rev/0337ecd9b600
            http://hg.mozilla.org/comm-central/rev/c185e3d53e5b
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: Thunderbird 8.0 → Thunderbird 10.0
Comment on attachment 498078 [details] [diff] [review]
Patch making the splitters attainable

I was thinking: should this patch also be committed? It would make it possible to uncollapse the address book pane under Aero by clicking-and-dragging and would bring it in line with the 3-pane. We could also probably get this fix out faster to users since there aren't any string changes.
Attachment #498078 - Flags: review- → review?(bwinton)
Comment on attachment 572484 [details] [diff] [review]
Up-to-date patch

Removing obsolete request.
Attachment #572484 - Flags: feedback?(mconley)
Duplicate of this bug: 703892
Reopening to track attachment 498078 [details] [diff] [review].
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 498078 [details] [diff] [review]
Patch making the splitters attainable

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

So, I think this is better, but I'm kind of thinking that it's still not discoverable enough.

I think I'm now leaning towards a 1-px splitter when it's not collapsed, and a 4-px or 5-px splitter when it is collapsed (not only on hover).

What do you think about that?

(Also, I'm sorry for changing my mind on this so often.)
Attachment #498078 - Flags: review?(bwinton) → review-
This patch gives the splitters in collapsed state a width of 4px or a height of 3px (4px is to much with status bar enabled).
Attachment #498078 - Attachment is obsolete: true
Attachment #576524 - Flags: ui-review?(bwinton)
Attachment #576524 - Flags: review?(bwinton)
(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #60)
> Comment on attachment 498078 [details] [diff] [review] [diff] [details] [review]
> Patch making the splitters attainable
> 
> Review of attachment 498078 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> So, I think this is better, but I'm kind of thinking that it's still not
> discoverable enough.
> 
> I think I'm now leaning towards a 1-px splitter when it's not collapsed, and
> a 4-px or 5-px splitter when it is collapsed (not only on hover).
> 
> What do you think about that?

I think I disagree, since by 10.0, we'll have a good solution for this (the menus). While making the collapsed splitters thicker helps discovery, I think it hurts people who intentionally collapse them (ok, not by much). For comparison, Ubuntu's file explorer has a similar pane can be closed by hitting the X, and the *only* way to get it back is to go into the menus (or hit F9).

Personally, I'd lean towards seeing if support requests are reduced as of 10.0, and if that's not sufficient, then we can try to make this more obvious for people.
(In reply to Jim Porter (:squib) from comment #62)
> (In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #60)
> > Comment on attachment 498078 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]
> > Patch making the splitters attainable
> > 
> > Review of attachment 498078 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]:
> > -----------------------------------------------------------------
> > 
> > So, I think this is better, but I'm kind of thinking that it's still not
> > discoverable enough.
> > 
> > I think I'm now leaning towards a 1-px splitter when it's not collapsed, and
> > a 4-px or 5-px splitter when it is collapsed (not only on hover).
> > 
> > What do you think about that?
> 
> I think I disagree, since by 10.0, we'll have a good solution for this (the
> menus). While making the collapsed splitters thicker helps discovery, I
> think it hurts people who intentionally collapse them (ok, not by much). For
> comparison, Ubuntu's file explorer has a similar pane can be closed by
> hitting the X, and the *only* way to get it back is to go into the menus (or
> hit F9).
> 
> Personally, I'd lean towards seeing if support requests are reduced as of
> 10.0, and if that's not sufficient, then we can try to make this more
> obvious for people.

I sort of see your point, but that argument could go the other way, too…  ;)

In a different vein, what do you think about this: You can't collapse the splitters by dragging them, only by the menu items, at which point they totally disappear until you re-enable them again.  (So, you could drag them until they were, say, 100 pixels wide, but no smaller.)
(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #63)
> In a different vein, what do you think about this: You can't collapse the
> splitters by dragging them, only by the menu items, at which point they
> totally disappear until you re-enable them again.  (So, you could drag them
> until they were, say, 100 pixels wide, but no smaller.)

Assuming there's a straightforward way to do this, I think this would be ideal. I do think it would be useful to fix the address book splitters under Aero in the short term, though.
Comment on attachment 576524 [details] [diff] [review]
Splitter after bwinton's proposal

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

This certainly seems like a simple fix which we could put into TB 9.  I'ld like to see something similar for Mac, because since Lion was released it suffers from the same problem, but we don't need to let that prevent us from landing this.

For TB 10, I would love it if someone could try the fix I requested in comment 63...  Ah what the hell, I'll give it a try.
Attachment #576524 - Flags: ui-review?(bwinton)
Attachment #576524 - Flags: ui-review+
Attachment #576524 - Flags: review?(bwinton)
Attachment #576524 - Flags: review+
Okay, that wasn't _too_ bad.

Squib, could you give this a try, and let me know what you think?  If you like it, I think we'll want to extend the behaviour to the folder pane and message preview window, and fix the odd pixel alignment in the collapsed dirTree-splitter.

Thanks,
Blake.
Attachment #578282 - Flags: review?(squibblyflabbetydoo)
Comment on attachment 576524 [details] [diff] [review]
Splitter after bwinton's proposal

After comment 65 I'm asking for approval for aurora and beta.
Attachment #576524 - Flags: approval-comm-beta?
Attachment #576524 - Flags: approval-comm-aurora?
(In reply to Richard Marti [:paenglab] from comment #67)
> Comment on attachment 576524 [details] [diff] [review] [diff] [details] [review]
> Splitter after bwinton's proposal
> 
> After comment 65 I'm asking for approval for aurora and beta.

Shouldn't we make sure the splitters work the same across all windows? This patch just seems to update the address book, so now the 3pane is out-of-sync (and it doesn't help Mac or Linux).
Comment on attachment 576524 [details] [diff] [review]
Splitter after bwinton's proposal

(In reply to Jim Porter (:squib) from comment #68)
> (In reply to Richard Marti [:paenglab] from comment #67)
> > Comment on attachment 576524 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]
> > Splitter after bwinton's proposal
> > 
> > After comment 65 I'm asking for approval for aurora and beta.
> 
> Shouldn't we make sure the splitters work the same across all windows? This
> patch just seems to update the address book, so now the 3pane is out-of-sync
> (and it doesn't help Mac or Linux).

Ideally, yes, but this is intended to be a quick fix to get into TB 9.

But I've noticed that I'm not really doing a good job here, so I'm going to let you take over the reviews and direction for this bug.  Paenglab, I'm sorry to have failed you on this, but I think you'll probably get better results out of Squib.

Thanks,
Blake.
Attachment #576524 - Flags: ui-review?(squibblyflabbetydoo)
Attachment #576524 - Flags: ui-review+
Attachment #576524 - Flags: review?(squibblyflabbetydoo)
Attachment #576524 - Flags: review+
Attachment #576524 - Flags: approval-comm-beta?
Attachment #576524 - Flags: approval-comm-aurora?
Comment on attachment 498078 [details] [diff] [review]
Patch making the splitters attainable

So, I think the right thing to do here in the short term is just to make the address book splitters work like the 3pane splitters in Aero. We can file a followup for making the splitters work even better, but this is helpful at least.

Blake, feel free to yell at me if you think this is a horrible idea. :)
Attachment #498078 - Attachment is obsolete: false
Attachment #498078 - Flags: review-
Attachment #498078 - Flags: review+
Attachment #498078 - Flags: approval-comm-beta?
Attachment #498078 - Flags: approval-comm-aurora?
Comment on attachment 576524 [details] [diff] [review]
Splitter after bwinton's proposal

Clearing these reviews out for now, since I think this patch belongs in a followup.
Attachment #576524 - Flags: ui-review?(squibblyflabbetydoo)
Attachment #576524 - Flags: review?(squibblyflabbetydoo)
Comment on attachment 578282 [details] [diff] [review]
A first stab at the behaviour I'ld like to see.

Ditto on this patch, though based on a quick look of the code, I think this is pretty close to what we want.
Attachment #578282 - Flags: review?(squibblyflabbetydoo) → feedback+
Comment on attachment 498078 [details] [diff] [review]
Patch making the splitters attainable

Not going to take this for 9 as it is too close to release.

Can you also clarify what's landed, and which patches to take where - as it looks like test bustage fixes landed as well? Thanks.
Attachment #498078 - Flags: approval-comm-beta? → approval-comm-beta-
Comment on attachment 498078 [details] [diff] [review]
Patch making the splitters attainable

I need clarification as per my previous comment before I can agree to this.
Attachment #498078 - Flags: approval-comm-aurora? → approval-comm-aurora-
(In reply to Mark Banner (:standard8) from comment #73)
> Comment on attachment 498078 [details] [diff] [review]
> Patch making the splitters attainable
> 
> Not going to take this for 9 as it is too close to release.
> 
> Can you also clarify what's landed, and which patches to take where - as it
> looks like test bustage fixes landed as well? Thanks.

Ok, so currently on 10+, we have new menu items under "View" to show/hide the various panes. All that's done (I believe there was the main patch plus a bustage fix for some tests).

For Aero, we also have a patch that makes the splitters in the address book expand when they're hovered over; that's the patch I asked for approval for. All that does is make the address book work like the 3pane does now.

Finally, bwinton has a WIP patch to change the behavior further (the splitter is visible when dragged to the edge of the screen, and invisible when hidden via the View menu). I was going to handle that in a followup bug.
Comment on attachment 498078 [details] [diff] [review]
Patch making the splitters attainable

Adding back approval flag that was mistakenly removed. dkl
Attachment #498078 - Flags: approval-comm-beta-
Blocks: 729016
Since we fixed this most of the way in this bug, let's close it off and do followup work in bug 729016.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
See Also: → 605359
You need to log in before you can comment on or make changes to this bug.