Make scrollbar dragable

[GoogleCodeExporter]

Enable selecting an item from the list of available rides.

Against what version did you make your patch?
1098

Please provide any additional information below.
To get the y-information, I had to modify the OnClick routine - which rippled 
through to several files. Maybe there is a better way? Modifying the scroll bar 
to handle longer lists will be required next in order to fully implement this 
feature.

Original issue reported on code.google.com by jjh...@aol.com on 4 Apr 2014 at 4:32

Attachments:

• src_April_4_2014.patch

[GoogleCodeExporter]

Indeed there is a better way, GuiWindow::Getwidget gives you the widget from 
the widget number.

The comments hardly add anything to the list selection code. Also, you're 
breaking a lot of code style rules; leading spaces instead of leading tabs, use 
of camel case variables, declaration not at first use (not a hard rule, but 
highly recommended).

As you already found out, one of the reasons that this code is not implemented 
yet, is because the scrollbar isn't working. Also, you need at least 2 things 
to select from :)

My guess is that much of your patch can be moved into the generic scrollbar 
handler code.

Original comment by Alberth2...@gmail.com on 9 Apr 2014 at 7:18

[GoogleCodeExporter]

Patch withdrawn.
You can mark this issue closed.

Original comment by jjh...@aol.com on 9 Apr 2014 at 11:22

[GoogleCodeExporter]

Original comment by CharlesP...@googlemail.com on 10 Apr 2014 at 12:19

• Changed state: WontFix

[GoogleCodeExporter]

Re-opening, as having the ability to select a rise from (a scrolled) list would 
be quite useful.

Original comment by Alberth2...@gmail.com on 18 Apr 2014 at 10:26

• Changed state: Confirmed

[GoogleCodeExporter]

This draws the scroll bar slider and responds to up and down clicks. I did not 
see a way to get the scroll bar button height in ridegui so it is temporarily 
set as a hard value. I made a dozen "test shop" files with the same graphics 
but different text for testing. A new problem is that now the onclick event is 
not triggered for the ride list; maybe a recent update interfered with that.. 
But for the moment, I can't make a response to clicking on the ride list.

Original comment by jjh...@aol.com on 20 Apr 2014 at 1:23

Attachments:

• ScrollBarDraw.patch

[GoogleCodeExporter]

Woo, nice!

The idea of your patch seems correct, adding data fields to the scrollbar. If 
you also add some functions to it, the calculations with 'start', 'visible' and 
'item_count' can all be hidden, making it much more readable.

For example, if you look careful in your scrollbar draw code, the 
"slider_length" calculation is the same for both scrollbars, except at the end 
you need to compute the available number of pixels that you have. The first 
part can be extracted out of the if/else of the direction of the scrollbar thus.

I added some code to fix most of your current problems.

r1131 adds a position in the widget clicked in, which should be nicer than your 
current "int V = this->mouse_pos.y - SB->pos.base.y;".

r1132 redirects clicks to the scrollbar widget, so it can update its internal 
data directly, instead of in the window code (we'll have a 100 or so different 
windows, and you don't want to write that code every time :) )

r1133 and r1135 enables a link from the scrollbar widget to the scrolled widget 
(ie the list), so you can query that widget for its properties (size and resize 
step), and initiate a redraw with MarkDirty.

r1134 fixes the not-working OnClick problem that you mentioned, a WT_EMPTY was 
not clickable previously.

The attached file basically contains your added scrollbar data, but I added a 
few proposed methods that you may find useful to implement.

It builds on your idea of the scrollbar data, but one step further. The 
scrollbar widget owns that data, and other objects can set values, or query 
values from it that are needed. A short description of my ideas

void SetItemSize(uint size)  item_count and start have no meaning without 
knowing how high one item is in pixels. One way to get that information is to 
use canvas->resize_[xy] (the resize step of the scrolled widget), but sometimes 
the 'canvas' as a resize step of 0 (ie it cannot be made smaller or bigger). 
For that case, the "SetItemSize(uint size)" can be used to set it manually.

void SetItemCount(uint count) I guess it's clear what it does. It may also need 
to update start to keep things consistent.

void SetStart(uint offset); uint GetStart() const; getting and setting the 
first visible item, should stay within the item_count and visible_count limits.

void ScrollTo(uint offset) Make item number 'offset' visible in the scrolled 
widget. If 'offset' is already visible, you don't need to do anything, if it is 
further down, you only need to scroll such that the bottom visible item becomes 
'offset'.  In other words, just a nicer SetStart(offset) in some cases.

uint GetClickedItem(Point16 pos) const; Since the scrollbar knows its counters, 
visible items, and item height, it can also compute the item that was clicked 
on.

All these functions are just ideas, I haven't checked if you really need them. 
I also didn't check whether you need more such functions (quite likely you do). 
Please feel free to keep what you like, and throw away what you don't like.

The whole idea of moving stuff to the scrollbar widget is to reduce the number 
of lines to write in the window code, such as in ride_gui.cpp.

Finally some details that you should watch out for.
- You seem to have set a tab-size of 4, and use a mix of spaces and tabs in 
your additions. The project uses only tabs for indenting lines.
- Uppercase names, such as SB or V are used for constants only, please use 
lowercase names for variables.
- Single line statements like "if (SB->start > 0) {SB->start--;}" don't use 
curly braces.
- if/else is written with curly braces, where the 'else' line is "} else {" 
rather than "}\n else {" (ie no "}" on its own before an "else").
Nothing major, and quite fixable for small patches.

In all, I think you're on the right track.

Original comment by Alberth2...@gmail.com on 20 Apr 2014 at 1:32

Attachments:

• 1_scrollbar_data.patch

[GoogleCodeExporter]

And of course new ideas appear while rereading the new text.

You may not need the visible_count, and use canvas->pos.{width,height} instead?

Original comment by Alberth2...@gmail.com on 20 Apr 2014 at 1:37

[GoogleCodeExporter]

This includes selecting items in the list. I could not seem to access MakeDirty 
in ScrollbarWidget::OnClick so I temporarily copied the code to there.

Still don't have code to drag the slider but some of the items help prepare for 
that.

Original comment by jjh...@aol.com on 21 Apr 2014 at 5:48

Attachments:

• ScrollBarDraw2.patch

[GoogleCodeExporter]

Similar to above except it includes dragging the slider with the mouse. Sill 
have trouble with MakeDirty and will likely need some style corrections. At 
least my editor is now leaving the tabs as tabs :)

Original comment by jjh...@aol.com on 22 Apr 2014 at 1:11

Attachments:

• ScrollBarDraw3.patch

[GoogleCodeExporter]

Looking very nice at first sight :)
Dragging looks a bit what the viewport is doing, perhaps the infra structure 
should be merged?

Yesterday I spent polishing your scrollbar2 patch, but got stuck at testing. 
Also, I am still not happy with the window code, it needs to be even smaller 
for a gui. Attached you'll find the result. I hope you like it.

Original comment by Alberth2...@gmail.com on 22 Apr 2014 at 7:47

Attachments:

• scrollbar2_modified.patch

[GoogleCodeExporter]

This fixes some minor problems and also adds block moves for clicking before 
and after the slider bar.

Original comment by jjh...@aol.com on 22 Apr 2014 at 10:23

Attachments:

• scrollbar2_A_modified.patch

[GoogleCodeExporter]

Thanks for the fixes.

I just committed basically scrollbar_2_A_modified.patch to trunk. The only 
thing I left out is removal of "sb->SetStart(0);" at line 354 of ride_gui.cpp. 
Its purpose is to reset the list of rides to the first item after switching to 
a different kind of ride. I am not sure why you don't want that.

I also looked at the dragging patch, but didn't have time to merge it this 
weekend.

I like your additional functions StartDrag etc in the scrollbar widget. I was 
thinking that it might be interesting to make it a abstract base class, so you 
can test against it rather than testing for being a scrollbar.
That might also open the road for merging window moving (by dragging the title 
bar) into the same mechanism (but  I don't know if that would work).
I also spend quite some time pondering about adding a new WMME value for the 
WindowManager, but it has no idea about widgets, so that would be a lot more 
work.

Original comment by Alberth2...@gmail.com on 27 Apr 2014 at 6:33

[GoogleCodeExporter]

Original comment by Alberth2...@gmail.com on 27 Apr 2014 at 6:35

• Changed title: Make scrollbar dragable

[GoogleCodeExporter]

Thanks for commiting this small contribution to the project.

The line: "sb->SetStart(0);" is located in the routine (SetNewRide) called 
whenever the user selects a ride within the list. If the list has been scrolled 
down and an item selected, it will make the list and scrollbar jump up to the 
beginning of the list, no longer showing the selected item. If that line is 
moved to SetNewRideKind, it would do what you want. However, when that routine 
calls SetNewRide with an index of 0, it automatically resets the list and 
scrollbar; so there is no need for that line: the desired feature is already 
implemented by default. Also, the code in SetNewRide to determine the row count 
should probably be moved to SetNewRideKind.

As for dragging, the widget and window dragging should probably be unified in 
some way. I had not considering that earlier. Something to mull over.

Hopefully, the artists in the project can make some more shop images! 

Original comment by jjh...@aol.com on 27 Apr 2014 at 7:38

Attachments:

• scrollbar2_B_modified.patch

[GoogleCodeExporter]

Thanks for the fix, committed in r1151.

Original comment by Alberth2...@gmail.com on 3 May 2014 at 1:12