Including the source of a wxCommandEvent in the event

[Ian McInerney]

For some wxCommandEvent events there are two possible sources (e.g. the EVT_MENU can be dispatched by both an accelerator key press and a menu item click), but there is no way of knowing which source triggered the event. I would like to add an event source field to the wxCommandEvent (with getters/setters GetEventSource/SetEventSource), but currently I only know that EVT_MENU has multiple sources. Are there other command events that have multiple sources that should also be included in the enum for this? Currently, the sources would be the following

DEFAULT

MENU_CLICK

ACCEL_KEY

with default being the default initialization value, and the value for events with only 1 source.

Thanks,

-Ian

[Vadim Zeitlin]

On Sun, 25 Aug 2019 03:08:04 -0700 (PDT) Ian McInerney wrote:

IM> For some wxCommandEvent events there are two possible sources (e.g.
IM> the EVT_MENU can be dispatched by both an accelerator key press and a menu
IM> item click), but there is no way of knowing which source triggered the
IM> event.

I'm not exactly opposed to this but, as always, when adding a new feature
(which has a cost in terms of extra code, extra maintenance later, extra
effort to implement it in any new ports etc), it would be useful to know
why exactly would you need this? I.e. what difference can it make to the
program that the menu item was activated directly (with further
possibilities of activating it with the mouse, or with the keyboard, and
this can be done either by pressing Enter on it or by pressing its
mnemonic without selecting it first) or using an accelerator?

IM> I would like to add an event source field to the wxCommandEvent (with
IM> getters/setters GetEventSource/SetEventSource), but currently I only
IM> know that EVT_MENU has multiple sources. Are there other command events
IM> that have multiple sources that should also be included in the enum for
IM> this? Currently, the sources would be the following
IM> DEFAULT

This should probably be called "unspecified".

IM> MENU_CLICK
IM> ACCEL_KEY
IM> with default being the default initialization value, and the value for
IM> events with only 1 source.

For wxCommandEvents, another possible source is wxToolBar: even though its
events are called wxEVT_TOOL, this is actually exactly the same thing as
wxEVT_MENU and this is intentional, in order to allow reusing the same
handler for both the menu and toolbar items. It's already possible to
distinguish between menu and toolbar events if you really want to by
examining event.GetEventObject(), but I guess it could make sense to have
this as another kind of source too.

I can't think of any other event that could come from multiple sources.
Many events can be generated using either keyboard or mouse, but I don't
think this really counts, as it's still the same action.

Regards,
VZ

[Ian McInerney]

Vadim,

The main reason I wish to add the event source to the menu events is the following workflow:

You have a menu item that opens a selection window for a new object to add to a drawing display. You want this selection window to be activated by either the keyboard (through accelerator keys) or the menu item, but you want some different behavior with each. For instance, if activated through the menu item you want to go into a state that will display the window on the next mouse click in the drawing frame, while when activated through the accelerator key you want to immediately open the selection window (without requiring further clicks). Currently, to do this you must keep track of if the menu item was highlighted to see if the menu button was pressed during the activation. This is very cumbersome, and it would be greatly simplified by adding the event source flag to the event (since you could then just check the flag in the command handler instead of having to share data between the menu handlers and the command handler). This is being used by some programs, specifically KiCad, which uses the workflow I just outlined and has to implement the user-level workaround I described.

I guess since the EVT_TOOL and EVT_MENU command events are the same, it would make sense to add a toolbar source, since the same handler could be linked to both events (and it is possible to want the same workflow described above with a toolbar button as well). That would simplify the user-level handling logic.

So then the event sources would be:

UNSPECIFIED

MENU_CLICK

TOOL_CLICK

ACCEL_KEY

Thanks for your feedback.

-Ian

[Vadim Zeitlin]

On Sun, 25 Aug 2019 04:14:05 -0700 (PDT) Ian McInerney wrote:

IM> The main reason I wish to add the event source to the menu events is the
IM> following workflow:
IM> You have a menu item that opens a selection window for a new object to add
IM> to a drawing display. You want this selection window to be activated by
IM> either the keyboard (through accelerator keys) or the menu item, but you
IM> want some different behavior with each.

OK, this makes sense, thanks. But I think we agree that the "accelerator"
behaviour should actually also happen if you use Alt-X,Y to open the menu
(where "X" and "Y" are the mnemonics of the menu and menu item
respectively), i.e. it only matters whether the keyboard or the mouse was
used for performing the action, doesn't it?

The annoying thing is that this is clearly related to what MSW already
does with WM_XXXUISTATE messages, which track exactly this (see
https://devblogs.microsoft.com/oldnewthing/20130516-00/?p=4343 for a good
explanation), but I'm not sure if we can somehow reuse this here: while
WM_QUERYUISTATE looks useful, I'm not sure if it's really what we need.

IM> I guess since the EVT_TOOL and EVT_MENU command events are the same, it
IM> would make sense to add a toolbar source, since the same handler could be
IM> linked to both events (and it is possible to want the same workflow
IM> described above with a toolbar button as well). That would simplify the
IM> user-level handling logic.
IM>
IM> So then the event sources would be:
IM> UNSPECIFIED
IM> MENU_CLICK
IM> TOOL_CLICK
IM> ACCEL_KEY

Actually, and contrary to what I wrote in the previous reply, before
understanding the context of this change, I now think it's not really
useful to distinguish between the menu and toolbar items here. For me the
possibilities should rather be something like UNKNOWN, KEYBOARD, MOUSE.

However, and especially if we can reuse WM_QUERYUISTATE, which doesn't
provide such detailed information, in wxMSW implementation, perhaps the
best API would actually be something like WasLastActionPerformedFromKbd()
in wxTLW.

Sorry for making this more complicated,
VZ

[Ian McInerney]

Vadim,

Is there an advantage to doing the detection in the wxTopLevelWindow class instead of the wxWindow class? The wxWindow class already has callbacks for the key press and button press events, so essentially all that would be needed is to have those callbacks set a flag to say if the last event processed was a keyboard or a mouse event and add a getter for it. Then we could also have the wxCommandEvent constructor automatically read that flag and set the event source (so then any command event will have the mouse/keyboard source information).

-Ian

[Vadim Zeitlin]

On Sun, 25 Aug 2019 17:01:48 -0700 (PDT) Ian McInerney wrote:

IM> Is there an advantage to doing the detection in the wxTopLevelWindow class
IM> instead of the wxWindow class?

If we need to have this information for menu events, it looks like it
would be enough to have it in wxTLW because menus are always attached to
one. But we could do it at wxWindow level too, this could be useful for
popup menus, I guess.

IM> The wxWindow class already has callbacks for the key press and button
IM> press events,

At least in wxMSW it doesn't. But I don't think we should use a handler
for this anyhow, I'd rather just add a global/static member variable that
we would update when generating these events. This should have minimal
overhead compared to invoking an extra event handler.

IM> so essentially all that would be needed is to have those callbacks set
IM> a flag to say if the last event processed was a keyboard or a mouse
IM> event and add a getter for it. Then we could also have the
IM> wxCommandEvent constructor automatically read that flag and set the
IM> event source (so then any command event will have the mouse/keyboard
IM> source information).

Yes, this looks like a plan. I still wonder if we could use
WM_QUERYUISTATE instead under MSW, but I'm not sure if this is compatible
with the non-default value of SPI_GETKEYBOARDCUES (OTOH this is something
that probably less than 1% of the users change anyhow, so maybe it's not
really important?).

Regards,
VZ

[Ian McInerney]

Vadim,

Having this for popup menus would also be useful (for the same reason outlined earlier), which was why I was originally starting with just modifying the wxMenu class.

When I said it already has the callbacks for the events, I meant that the wxWindow class already is on the event path for all of the key/mouse events (and seems to be the source of them in most cases). So it would be a matter of just adding the setting of the boolean variable to the appropriate functions, which I believe would be:

wxWindowMSW::MSWHandleMessage

wxWindowMac::OSXHandleKeyEvent

wxWindowMac::OnMouseEvent

wxWindowGTK::GTKProcessEvent

The issue would be if these handlers receive the events when the menu is open. I have been doing some experimentation on GTK, and it seems that the GTK keypress callback doesn't receive a keypress event when the menu is open. Instead, I have to add a keypress callback to the actual wxMenu object to get the key events. I don't know if this would be the case on other platforms though.

-Ian

--

Ian McInerney
mcia...@gmail.com

No electrons were harmed in the making of this message

[Vadim Zeitlin]

On Mon, 26 Aug 2019 19:14:23 +0200 Ian McInerney wrote:

IM> When I said it already has the callbacks for the events, I meant that the
IM> wxWindow class already is on the event path for all of the key/mouse events
IM> (and seems to be the source of them in most cases). So it would be a matter
IM> of just adding the setting of the boolean variable to the appropriate
IM> functions, which I believe would be:
IM> wxWindowMSW::MSWHandleMessage
IM> wxWindowMac::OSXHandleKeyEvent
IM> wxWindowMac::OnMouseEvent
IM> wxWindowGTK::GTKProcessEvent

You need to handle this even before it gets to the window, e.g. in
wxGUIEventLoop::PreProcessMessage() in wxMSW case, because accelerators are
not sent to the window as key events at all. For wxOSX the first wx methods
called seem to be wxOSX_mouseEvent and wxOSX_keyEvent and for wxGTK this
would be gtk_window_{key,button}_{press,release}_callback().

IM> The issue would be if these handlers receive the events when the menu is
IM> open. I have been doing some experimentation on GTK, and it seems that the
IM> GTK keypress callback doesn't receive a keypress event when the menu is
IM> open. Instead, I have to add a keypress callback to the actual wxMenu
IM> object to get the key events. I don't know if this would be the case on
IM> other platforms though.

In wxMSW wxKeyboardHook() should be called for absolutely all keyboard
events, but not for the mouse ones. And I'm not sure if installing another
hook just for this would be a good idea, to be honest.

Regards,
VZ