Opened 15 years ago
Closed 21 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)
Change History (11)
by , 15 years ago
Attachment: | scrollView.patch added |
---|
comment:1 by , 15 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 , 15 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 , 15 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 , 15 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 , 15 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 , 15 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 , 15 years ago
Keywords: | gsoc2010 added |
---|
comment:8 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:9 by , 21 months ago
Milestone: | R1 → R1/beta5 |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
Fixed in hrev57055.
a quick and dirty patch to fix this, and communicate the problem