Support drag-and-drop autoscroll (PR #26237)
Bill Su
As stated in #25978 (comment), I want to implement autoscroll for drag-and-drop operations, and https://github.com/wxWidgets/wxWidgets/blob/61300d78d215e576576e7dc951f3147ef1785db2/samples/dnd/d_and_d.txt#L163 implies that I'm not the only one who wants this feature.
Now that the wxScrolled<> autoscroll region is configurable, this PR is a suggestion for another wxScrolled<> extension to support autoscrolling during drag-and-drop. Commit 9054302 implements the support, and commit e2439c1 modifies the dnd sample to demonstrate dragging a DnDShape from one DnDShapeFrame to another DnDShapeFrame with the destination DnDShapeFrame autoscrolling.
I realize that commit e2439c1 leaves the sample with some imperfections, but I think the current form is sufficient to demonstrate the API. If the API is approved, then I will go back and refine the sample.
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26237
Commit Summary
- 9054302 wxScrolled<>: support for autoscroll during drag
- e2439c1 dnd: extend sample to demonstrate autscroll
File Changes
(3 files)
- M include/wx/scrolwin.h (13)
- M samples/dnd/dnd.cpp (45)
- M src/generic/scrlwing.cpp (31)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26237.patch
- https://github.com/wxWidgets/wxWidgets/pull/26237.diff
—
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 left a comment (wxWidgets/wxWidgets#26237)If the API is approved, then I will go back and refine the sample.
And add documentation to interface/wx/scrolwin.h.
—
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 1 commit.
- 23c4e89 dnd: fix unused param warnings/errors
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Bill Su
@wsu-cb pushed 1 commit.
- b915c7d dnd: add missing override indicator
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
Thanks, the implementation looks good but I'm not sure about the API. Could it be better to have an object which enable autoscroll without capture in its ctor, disables it in its dtor and has a member function for informing the window about the mouse position change? Not sure if it's worth it, but I'd like to at least have such a function somewhere, i.e. either in this object or in wxScrolled itself.
In build/cmake/samples/CMakeLists.txt:
> @@ -35,13 +35,13 @@ endif()
wx_add_sample(dialogs ${SAMPLE_DIALOGS_SRC} DATA tips.txt)
wx_add_sample(dialup nettest.cpp LIBRARIES wxnet DEPENDS wxUSE_DIALUP_MANAGER)
wx_add_sample(display)
-wx_add_sample(dnd dnd.cpp RES dnd.rc DATA wxwin.png DEPENDS wxUSE_DRAG_AND_DROP)
+wx_add_sample(dnd dnd.cpp RES dnd.rc DATA wxwin.png DEPENDS wxUSE_DRAG_AND_DROP IMPORTANT)
I'm not sure we want to do this for this one, of all the modified samples I think only drawing potentially deserves being "IMPORTANT".
I'd make it and dialogs important and remove it from dataview, but I'm not really sure.
Any (other) opinions?
> + wxMouseEvent event(wxEVT_MOTION); + event.SetPosition(wxPoint(x, y)); + m_frame->ProcessWindowEvent(event);
I'd really like to avoid having to do it in application code, e.g. by adding some InformMousePositionForAutoscroll() or something with better name.
The rationale is that we can't guarantee that wx windows always react to events in the expected way (e.g. native controls do not and can't), so we advise to never do this and so this is showing a bad example.
> @@ -168,6 +168,18 @@ class WXDLLIMPEXP_CORE wxScrollHelperBase : public wxAnyScrollHelperBase
wxEventType& evtHorzScroll,
wxEventType& evtVertScroll) const;
+ // wxWidgets <= 3.3.1 mostly restricted autoscroll to the
+ // window holding the mouse capture. However, when dragging
+ // objects between windows, the destination window should be
+ // the window that is scrolling (to allow positioning the
+ // dragged object). The intent is that a window will call
+ // EnableAutoscrollWithoutCapture() when processing a
+ // drag-enter, and DisableAutoscrollWithoutCapture() when
+ // processing a drag-exit or drag-drop.
+ bool GetAutoscrollWithoutCapture() const;
Does this have to be public? It looks like it's only used (and useful) internally?
—
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 1 commit.
- 4e79f07 dnd: do not mark IMPORTANT
—
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.
In build/cmake/samples/CMakeLists.txt:
> @@ -35,13 +35,13 @@ endif()
wx_add_sample(dialogs ${SAMPLE_DIALOGS_SRC} DATA tips.txt)
wx_add_sample(dialup nettest.cpp LIBRARIES wxnet DEPENDS wxUSE_DIALUP_MANAGER)
wx_add_sample(display)
-wx_add_sample(dnd dnd.cpp RES dnd.rc DATA wxwin.png DEPENDS wxUSE_DRAG_AND_DROP)
+wx_add_sample(dnd dnd.cpp RES dnd.rc DATA wxwin.png DEPENDS wxUSE_DRAG_AND_DROP IMPORTANT)
Oops. I turned on IMPORTANT to configure the sample to build on my local system, but I didn't intend to commit this.
Undone in 4e79f07.
—
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.
> @@ -168,6 +168,18 @@ class WXDLLIMPEXP_CORE wxScrollHelperBase : public wxAnyScrollHelperBase
wxEventType& evtHorzScroll,
wxEventType& evtVertScroll) const;
+ // wxWidgets <= 3.3.1 mostly restricted autoscroll to the
+ // window holding the mouse capture. However, when dragging
+ // objects between windows, the destination window should be
+ // the window that is scrolling (to allow positioning the
+ // dragged object). The intent is that a window will call
+ // EnableAutoscrollWithoutCapture() when processing a
+ // drag-enter, and DisableAutoscrollWithoutCapture() when
+ // processing a drag-exit or drag-drop.
+ bool GetAutoscrollWithoutCapture() const;
It's declared in wxScrollHelperBase, but it's called from both wxScrollHelperBase and wxAutoScrollTimer. Similar to #25978 (comment), after the desired functionality is complete, we can discuss public vs friend.
—
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.
> + wxMouseEvent event(wxEVT_MOTION); + event.SetPosition(wxPoint(x, y)); + m_frame->ProcessWindowEvent(event);
MSW has TTM_RELAYEVENT. How about void RelayPosition(wxPoint pt)? Or do you think Autoscroll is a necessary part of the name? AutoscrollRelayPosition?
—
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 left a comment (wxWidgets/wxWidgets#26237)Thanks, the implementation looks good but I'm not sure about the API. Could it be better to have an object which enable autoscroll without capture in its ctor, disables it in its dtor and has a member function for informing the window about the mouse position change? Not sure if it's worth it, but I'd like to at least have such a function somewhere, i.e. either in this object or in
wxScrolleditself.
Can you explain how you think this object would be used? Since the enable and disable are not called within a single function invocation, I don't see the advantage to RAII-style.
—
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 1 commit.
- 64eab60 dnd: remove unnecessary code
—
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.
> + wxMouseEvent event(wxEVT_MOTION); + event.SetPosition(wxPoint(x, y)); + m_frame->ProcessWindowEvent(event);
Actually, now that I look at this again, I don't think there's any reason for this code. The wxAutoScrollTimer::Notify() takes care of tracking the mouse position.
Unnecessary code removed in 64eab60
—
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 1 commit.
- 91f3483 samples: remove unintended commit of IMPORTANT flags
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Bill Su
@wsu-cb pushed 1 commit.
- 5efd01d dnd: remove move unecessary code
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26237)Can you explain how you think this object would be used? Since the enable and disable are not called within a single function invocation, I don't see the advantage to RAII-style.
Yes, as I wrote, I wasn't sure if it was really worth it, and with the code for forwarding mouse events removed, I'm even less sure now (i.e. I don't think it's worth it any more). But, for the record, this could be still used by using std::unique_ptr<> and calling reset() on it — not automatic, as with RAII, but still maybe a bit more convenient than remembering to call specific initialization/cleanup functions.
Anyhow, I'll merge this as is now, thanks!
—
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 left a comment (wxWidgets/wxWidgets#26237)Anyhow, I'll merge this as is now, thanks!
This can be merged now without causing any problems. However, as noted in earlier comments, I plan to improve the implementation of the dnd sample and the interface documentation, so I suggest waiting to merge this until I finish that work.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26237)Oops, sorry, I had missed that comment and already pushed 2484d14 (wxScrolled<>: add support for autoscroll during drag, 2026-02-22) which merged this, but forgot to mention the PR number in its commit message.
I'll close it now, please open a new PR for further improvements, sorry for the inconvenience...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.