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

VERIFIED FIXED in mozilla0.9.2

Status

()

Core
XUL
P3
normal
VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: Scott MacGregor, Assigned: Brian Ryner (not reading))

Tracking

Trunk
mozilla0.9.2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

17 years ago
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.

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9

Comment 1

17 years ago
Giving arik some work.
Assignee: hyatt → arik
Status: ASSIGNED → NEW

Updated

17 years ago
Status: NEW → ASSIGNED

Comment 2

17 years ago
Created attachment 29292 [details] [diff] [review]
first crack at a patch for this

Comment 3

17 years ago
Created attachment 29302 [details] [diff] [review]
heh, forgot to use -up sorry bout that.

Comment 4

17 years ago
r=dr
OS: Windows NT → All
Hardware: PC → All

Comment 5

17 years ago
(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.

Updated

17 years ago
Target Milestone: mozilla0.9 → mozilla0.9.1
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

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

Comment 9

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

Comment 10

17 years ago
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?  :)
Keywords: patch

Comment 11

17 years ago
->P3/0.9.2
Priority: -- → P3
Target Milestone: mozilla0.9.1 → mozilla0.9.2
(Assignee)

Comment 12

17 years ago
I'll take this one.  It's making the outliner filepicker look particularly bad.
Assignee: arik → bryner
Status: ASSIGNED → NEW
(Assignee)

Comment 13

17 years ago
Created attachment 34245 [details] [diff] [review]
patch revised per brendan's comments
(Assignee)

Comment 14

17 years ago
dr and brendan, can you re-review?  Thanks.
Status: NEW → ASSIGNED
(Assignee)

Comment 15

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

Comment 16

17 years ago
r=blake
(Assignee)

Comment 17

17 years ago
Created attachment 34265 [details] [diff] [review]
only set the attribute if it needs to change

Comment 18

17 years ago
Change the nsAutoString isVisible to isCollapsed.  NO change in logic 
required.  Just substitute the text.  sr=hyatt
(Assignee)

Comment 19

17 years ago
checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
verified.

thanks for fixing it!

cc laurel, I know this drove her nuts.
Status: RESOLVED → VERIFIED
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

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

Updated

10 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.