wxToolbook suppresses all context menu routing (Issue #22153)
David Connet
I have a tabview with context menus. These all work properly when using wxChoicebook, wxListbook, wxNotebook, or wxTreebook. However, in wxToolbook, the wxCommandEvent event handler is not being triggered - however the wxUpdateUIEvent handler is. (the Book is created exactly the same in all cases, I used the notebook sample as an example of how to change them on the fly)
This is occurring in the dev branch and in 3.1.5.
I'll try to modify the notebook sample to repro soon.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
David Connet
This diff will add a context menu in the notebook sample. Just one entry, it will flip the book type back to notebook.
- Under File->Type, Select any type other than notebook or toolbook.
- R-click in view, select "Notebook", see that the view switches to notebook
- Switch to toolbook
- Repeat - nothing happens
`diff --git a/samples/notebook/notebook.cpp b/samples/notebook/notebook.cpp
index f5da371790..8046b395fe 100644
--- a/samples/notebook/notebook.cpp
+++ b/samples/notebook/notebook.cpp
@@ -553,6 +553,13 @@ void MyFrame::RecreateBook()
if ( !m_bookCtrl )
return;
-
m_bookCtrl->Bind(wxEVT_CONTEXT_MENU, [this](wxContextMenuEvent& evt) {
-
wxPoint pt = m_bookCtrl->ScreenToClient(evt.GetPosition()); -
wxMenu menu; -
menu.Append(ID_BOOK_NOTEBOOK, "Notebook"); -
m_bookCtrl->PopupMenu(&menu, pt); -
}); -
m_bookCtrl->Hide();
// wxToolbook doesn't work without icons so always use them for it.
`
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
PB
FWIW, it started working (the patched sample on MSW) when I commented out this line:
https://github.com/wxWidgets/wxWidgets/blob/c3b232dcfd25107903e0447b36794aab9242c99f/src/generic/toolbkg.cpp#L42
When I also added
m_bookctrl->Bind(wxEVT_TOOL, &wxToolbook::OnToolSelected, this);
before returning from wxToolbook::Create(), both switching pages and getting a command event from a pop-up menu seem to work.
I.e., the changes were
src/generic/toolbkg.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/generic/toolbkg.cpp b/src/generic/toolbkg.cpp index b83d4bb14b..286e6d4775 100644 --- a/src/generic/toolbkg.cpp +++ b/src/generic/toolbkg.cpp @@ -39,7 +39,6 @@ wxDEFINE_EVENT( wxEVT_TOOLBOOK_PAGE_CHANGED, wxBookCtrlEvent ); wxBEGIN_EVENT_TABLE(wxToolbook, wxBookCtrlBase) EVT_SIZE(wxToolbook::OnSize) - EVT_TOOL(wxID_ANY, wxToolbook::OnToolSelected) EVT_IDLE(wxToolbook::OnIdle) wxEND_EVENT_TABLE() @@ -110,6 +109,8 @@ bool wxToolbook::Create(wxWindow *parent, ); } + m_bookctrl->Bind(wxEVT_TOOL, &wxToolbook::OnToolSelected, this); + return true; }
However, this is probably not the correct solution and there must be some underlying reason why it does not work. I just found that event table line odd, so I tested what happens when I remove it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
VZ
@PBfordev Thanks for debugging this and you're absolutely correct, this must happen for the reason you described and so this
diff --git a/src/generic/toolbkg.cpp b/src/generic/toolbkg.cpp index b83d4bb14b..2337fe1378 100644 --- a/src/generic/toolbkg.cpp +++ b/src/generic/toolbkg.cpp @@ -372,6 +372,7 @@ void wxToolbook::OnToolSelected(wxCommandEvent& event) if (page == wxNOT_FOUND) { // this happens only of page id has changed afterwards + event.Skip(); return; }
should be enough to fix it. @dconnet Could you please confirm that it does?
A better solution might be to use Bind() in InsertPage() to bind to the events to just the tools that we use, but this would require slightly more changes.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
David Connet
Confirmed. Also confirmed that the same fix works in 3.1.5.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
VZ
Thanks for testing, will push the fix soon.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.