make wxScrolled<> autoscroll region configurable (PR #25978)
Bill Su
This is an initial attempt at configuring wxScrolled<>'s autoscroll region. The default configuration emulates the existing behavior, but can be overridden to change the autoscroll region. In particular, my personal requirement is for the region to be inside the window, but near the edge, but the configuration functions allow for some other possibilities.
Would anyone prefer a different API? Or have any other comments?
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/25978
Commit Summary
- 19b7337 make autoscroll region configurable
File Changes
(3 files)
- M include/wx/scrolwin.h (22)
- M samples/drawing/drawing.cpp (19)
- M src/generic/scrlwing.cpp (122)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/25978.patch
- https://github.com/wxWidgets/wxWidgets/pull/25978.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
Sorry, I'm really confused by the API, so I'd appreciate some clarification/examples. TIA!
> @@ -171,6 +171,17 @@ bool wxScrollHelperEvtHandler::ProcessEvent(wxEvent& event)
{
wxEventType evType = event.GetEventType();
+ // always process these mouse events ourselves, even if the user code handles
+ // them as well, as we need to autoscroll
+ if ( evType == wxEVT_LEFT_DOWN )
+ {
+ m_scrollHelper->HandleOnLButtonUp((wxMouseEvent&)event);
It seems counterintuitive to process wxEVT_LEFT_DOWN events in a function called OnLButtonUp. I think wx convention would be to use just OnLeftDown (with "Mouse" and "Button" being implicit).
> @@ -152,6 +152,20 @@ class WXDLLIMPEXP_CORE wxScrollHelperBase : public wxAnyScrollHelperBase
// returns 0 if no scrollbars are there.
int GetScrollLines( int orient ) const;
+ // wxWidgets <= 3.3.1 autoscrolled exactly when the (captured) mouse cursor
I admit I have a lot of trouble parsing this. Maybe I could manage to do it, but it's easier to just ask questions, so here I go:
- Why do we have 2 functions if they are not independent (or are they?)? It would be better to have a single function with 2 parameters instead, I think. It's annoying that they're of the same type and so it would be simple to give them in wrong order, but it would be also simpler to check for invalid combinations (e.g. I think that valid inner and invalid outer are invalid?).
- Perhaps we should have 2 functions but based on whether they operate on window or client coordinates/rectangle basis? I think this would also make it simpler to use this.
- Can this be used to disable auto-scroll entirely? It seems like it should be possible to, but it's not obvious how.
- What is your actual use case, i.e. how do you use it?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Bill Su
@wsu-cb pushed 2 commits.
- 77389a3 fixup! make autoscroll region configurable
- 16ea5d5 fixup! make autoscroll region configurable
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Bill Su
@wsu-cb commented on this pull request.
> @@ -171,6 +171,17 @@ bool wxScrollHelperEvtHandler::ProcessEvent(wxEvent& event)
{
wxEventType evType = event.GetEventType();
+ // always process these mouse events ourselves, even if the user code handles
+ // them as well, as we need to autoscroll
+ if ( evType == wxEVT_LEFT_DOWN )
+ {
+ m_scrollHelper->HandleOnLButtonUp((wxMouseEvent&)event);
Fixed in 77389a3
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Bill Su
@wsu-cb commented on this pull request.
> @@ -152,6 +152,20 @@ class WXDLLIMPEXP_CORE wxScrollHelperBase : public wxAnyScrollHelperBase
// returns 0 if no scrollbars are there.
int GetScrollLines( int orient ) const;
+ // wxWidgets <= 3.3.1 autoscrolled exactly when the (captured) mouse cursor
It is entirely possible that this interface is overly complicated by attempting to be both (unnecessarily?) general and default to backward compatible behavior. Personally, I don't like the original behavior, but I assumed it had to remain the default. The only behavior that I actually care about is having the window autoscroll when the mouse is in a picture-frame-shaped region (16 pixels wide in the example in the drawing sample) that covers the outermost portion of the client rectangle. However, for generality, I chose an interface that allows using a picture-frame-shaped region outside the (window) rectangle rather than the original behavior's infinitely large region. Also for generality, the interface allows for the autoscroll region to extend both inside and outside the client rectangle. If it would make things easier to understand, I would be happy to restrict the options to the original behavior and the picture-frame-in-client-space region. (I would be even happier to remove the old behavior!)
To answer the specific questions about the proposed interface:
1: The two functions in this interface are mostly, but not entirely, independent. The dependency is that I want the inside-the-window autoscroll region to be based on the CLIENT rect so that no one has to worry about how to deal with making the autoscroll region account for the scrollbars. However, the original code has the autoscroll region based on the WINDOW rect. My attempt to resolve this inconsistency is to have the outside-the-window autoscroll region be relative to the WINDOW rect by default, but relative to the CLIENT rect if there is an inside-window autoscroll region.
1: I have no objection to a single function, but since I considered the zones mostly independent, I thought it was plausible that some API users would only want to change one of them.
1: I don't know what you mean by invalid combinations. The invalid values are negative sizes other than wxDefaultCoord, and that is invalid no matter what the other parameter is. In the case of a strictly positive value K inner region and a wxDefaultCoord outer region, then the inner region is the expected picture-frame with width K, and the outer region is the entire plane outside the client rect, so the total autoscroll region is the entire plane minus a rectangular hole in the middle of the client rect.
2: We could have both client and window rect based versions of the zones. I don't want to define an inside-the-window zone that extends into the scroll bar, but, now that you suggest it, I see that Windows Explorer's autoscroll zone includes the scrollbar, I had not previously noticed that.
3: Yes, this can disable autoscroll. I have added a demonstration of that in 16ea5d5.
4: My use case is dragging objects both within a window and from one window to another. Therefore, like Windows Explorer (see https://groups.google.com/g/wx-dev/c/9tJ3mpkJ4NQ/m/ysvIr7llAQAJ), I want the autoscroll region to be INSIDE the window, not outside it. While dragging inside the original window, the original window should autoscroll, but when dragging out of the window, the original window should stop autoscrolling, and when the dragged object is over a destination window, the destination window should autoscroll. This PR is only taking care of changing the autscrolling of the original window; once this is done, I will have to implement autoscrolling the destination window.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
trogmaniac
trogmaniac left a comment (wxWidgets/wxWidgets#25978)If the autoscroll code is touched at all, i think it should also fix the (IMHO) only flaw it currently has: It can't scroll in both directions at the same time. E.g. if you drag and move the mouse outside at the bottom, it starts scrolling down. But if you then also move the mouse outside to the right side, it won't start also scrolling in that directions.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.