71770 - Outliner widget: need to hide scrollbars when they aren't needed

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Scott MacGregor]

From our messenger to-do list:
49) When the thread pane can display all the message headers then the scroll bar
should disappear.
 Currently the scroll bar remains although the widget to move the focus up or
down is not present.
 The folder pane works as expected. 

If the outliner view is small enough that scrollbars aren't required I think the
outliner widget needs to hide it's scrollbar.

Comment 1

[David Hyatt]

Giving arik some work.

Comment 2

[arik devens]

Attachment: first crack at a patch for this

Comment 3

[arik devens]

Attachment: heh, forgot to use -up sorry bout that.

Comment 4

[Dan Rosen]

r=dr

Comment 5

[David Hyatt]

(1) No curly braces with single lines in if/else.
(2) Change NS_IMETHODIMP in the header and cpp to nsresult.
(3) Use PRBool as the arg to your new function
(4) Change arg from setVisible to aSetVisible
(5) Add an Invalidate() call following the changing of the scrollbar visibility.

Comment 6

[arik devens]

Attachment: take 2

Comment 7

[Brendan Eich [:brendan]]

Nits, answer for my edification if you like:

+  
+    if (rowCount < mPageCount)
+      SetVisibleScrollbar(PR_FALSE);
+    else
+      SetVisibleScrollbar(PR_TRUE);

My favorite kind of if statement -- how about just 'SetVisibleScrollbar(rowCount
>= mPageCount);' instead?

Isn't the new string-fu to use NS_LITERAL_STRING("true") here:

+    nsAutoString visible; visible.AssignWithConversion ("true");
+    scrollbarContent->SetAttribute(kNameSpaceID_None, nsXULAtoms::collapsed,
visible, PR_TRUE);

eliminating the need for an nsAutoString and (on savvy platforms) a converting
copy altogether?

/be

Comment 8

[arik devens]

i am not sure what you mean about the if statement...

Comment 9

[arik devens]

*** Bug 76466 has been marked as a duplicate of this bug. ***

Comment 10

[Håkan Waara]

Arik, Brendan means that you can optimize your if statement to be
|SetVisibleScrollbar(rowCount>= mPageCount);|. Which in words mean: Set the
scrollbar to be visible if rowCount is greater or equals mPageCount.

Clearer now?  :)

Comment 11

[Peter Trudelle]

->P3/0.9.2

Comment 12

[Brian Ryner (not reading)]

I'll take this one.  It's making the outliner filepicker look particularly bad.

Comment 13

[Brian Ryner (not reading)]

Attachment: patch revised per brendan's comments

Comment 14

[Brian Ryner (not reading)]

dr and brendan, can you re-review?  Thanks.

Comment 15

[Brian Ryner (not reading)]

Oops, meant to cc you guys.  Up for reviewing this again?

Comment 16

[Blake Ross]

r=blake

Comment 17

[Brian Ryner (not reading)]

Attachment: only set the attribute if it needs to change

Comment 18

[David Hyatt]

Change the nsAutoString isVisible to isCollapsed.  NO change in logic 
required.  Just substitute the text.  sr=hyatt

Comment 19

[Brian Ryner (not reading)]

checked in.

Comment 20

[(not reading, please use [e-mail] instead)]

verified.

thanks for fixing it!

cc laurel, I know this drove her nuts.

Comment 21

[(not reading, please use [e-mail] instead)]

the scroll bar will show up instantly (if I shrink the thread pane) but it 
disappears in 3 stages (if I grow the thread pane.)

want a new bug on that?

Comment 22

[Håkan Waara]

*** Bug 72278 has been marked as a duplicate of this bug. ***