Opened 14 years ago

Closed 10 months ago

#5678 closed bug (fixed)

BScrollView::MinSize() doesn't account for scroll bar components

Reported by: yourpalal Owned by: nobody
Priority: normal Milestone: R1/beta5
Component: Kits/Interface Kit Version: R1/Development
Keywords: gsoc2010 Cc:
Blocked By: Blocking: #6895
Platform: All

Description

BScrollView's MinSize() method does not account for the space needed for arrows and slider in each ScrollBar it has.

Attachments (2)

scrollView.patch (895 bytes ) - added by yourpalal 14 years ago.
a quick and dirty patch to fix this, and communicate the problem
snapshot1.png (15.8 KB ) - added by yourpalal 14 years ago.
Here is a screenshot of the problem, this is happening in a window using the Layout api and B_AUTO_UPDATE_SIZE_LIMITS. The initial condition of the window can be seen on the left, and the window on the right shows how it should look (I've manually adjusted the window there) I realized that what I've said so far hadn't been very clear, so hopefully this helps.

Download all attachments as: .zip

Change History (11)

by yourpalal, 14 years ago

Attachment: scrollView.patch added

a quick and dirty patch to fix this, and communicate the problem

comment:1 by yourpalal, 14 years ago

The patch I've uploaded addresses the problem but isn't well integrated into the BScrollView class. (eg no connection to _ComputeSize(),_ComputeFrame()) However, I would be glad to mesh these methods together. Anyway, the patch seems like it is the right thing to do, but perhaps someone else knows better? I wanted to get some code attached to this thing, to help explain what I think is the problem. --Alex

comment:2 by stippi, 14 years ago

Yep, the existing code ends up using _ComputeFrame() which already includes the scrollbars (and the required width for the frame unlike your patch). If your solution fixes something, then there must be a bug there. This patch should not be applied. Would be good if you can supply a minimal test case.

comment:3 by yourpalal, 14 years ago

The problem is that adding just B_V_SCROLLBAR_WIDTH and|or B_H_SCROLLBAR_HEIGHT does not (as far as I can tell) accommodate for the extra height on a vertical bar, or width on a horizontal bar. As for a test case, I noticed this while working on the FileTypes preflet, which I am re-writing with the Layout API. I can upload my current work on that and directions to reproduce the bug, or write a little test program. I see I did forget about the border in this patch, good catch!

by yourpalal, 14 years ago

Attachment: snapshot1.png added

Here is a screenshot of the problem, this is happening in a window using the Layout api and B_AUTO_UPDATE_SIZE_LIMITS. The initial condition of the window can be seen on the left, and the window on the right shows how it should look (I've manually adjusted the window there) I realized that what I've said so far hadn't been very clear, so hopefully this helps.

comment:4 by yourpalal, 14 years ago

First of all, I agree that my patch should not be applied, but, when I built with that patch applied locally, it did fix the problem where I'm working. However, it does not take into account whether or not fTarget->MaxSize() < size. I.e. the scroll view could end up with more room than it knows what to do with. Maybe the scroll view should just be drawn differently when there is not enough size to draw both arrows and the slider. Anyway, this is really easy to work around wherever it occurs, so this isn't urgent. Perhaps this should be marked invalid, if the current behaviour is deemed to be okay. Well, anyway, I'm going to stop replying to myself now.. :D

comment:5 by stippi, 14 years ago

You are right of course and now I understand what the problem with the original code is. I would need to read it again in order to see if using _ComputeFrame() for MinSize() is suitable at all. In another words the question is also if the non-layouted mode needs fixing too, I am guessing that's the case. Otherwise your patch, with the added handling of the frame width, would be fine.

comment:6 by mmadia, 14 years ago

Keywords: gsoc2010 added

comment:7 by yourpalal, 13 years ago

Blocking: 6895 added

(In #6895) Looks like this is a duplicate of #5678

comment:8 by axeld, 7 years ago

Owner: changed from axeld to nobody
Status: newassigned

comment:9 by waddlesplash, 10 months ago

Milestone: R1R1/beta5
Resolution: fixed
Status: assignedclosed

Fixed in hrev57055.

Note: See TracTickets for help on using tickets.