View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0035219 | Patches | Patch | public | 2019-03-13 12:30 | 2021-01-10 00:16 |
Reporter | Benito van der Zander | Assigned To | |||
Priority | normal | Severity | minor | Reproducibility | have not tried |
Status | new | Resolution | reopened | ||
Product Version | 2.0 | ||||
Summary | 0035219: OnClick on opened submenu in popupmenu | ||||
Description | When a submenu in a popupmenu is opened, it needs to call onclick of the parent menuitem | ||||
Tags | No tags attached. | ||||
Fixed in Revision | r60679 | ||||
LazTarget | - | ||||
Widgetset | Win32/Win64 | ||||
Attached Files |
|
related to | 0037210 | resolved | Juha Manninen | OnClick on opened submenu in popupmenu |
|
popupmenu.patch (999 bytes)
Index: lcl/interfaces/win32/win32callback.inc =================================================================== --- lcl/interfaces/win32/win32callback.inc (Revision 60475) +++ lcl/interfaces/win32/win32callback.inc (Arbeitskopie) @@ -401,14 +401,18 @@ function TWindowProcHelper.GetPopMenuItemObject: TObject; var - MainMenuHandle: HMENU; + MenuHandle: HMENU; MenuInfo: MENUITEMINFO; begin MenuInfo.cbSize := MMenuItemInfoSize; MenuInfo.fMask := MIIM_DATA; - MainMenuHandle := GetMenuParent(HMENU(WParam), GetMenu(Window)); - if GetMenuItemInfo(MainMenuHandle, LOWORD(LParam), true, @MenuInfo) then + MenuHandle := 0; + if Assigned(WindowInfo^.PopupMenu) then + MenuHandle := GetMenuParent(HMENU(WParam), WindowInfo^.PopupMenu.Handle); + if MenuHandle = 0 then + MenuHandle := GetMenuParent(HMENU(WParam), GetMenu(Window)); + if GetMenuItemInfo(MenuHandle, LOWORD(LParam), true, @MenuInfo) then Result := TObject(MenuInfo.dwItemData) else Result := nil; |
|
Applied, thanks. |
|
I'm afraid this bugfix is incorrect. [u][b]Summary:[/b][/u] If you have a TPopupMenu with a TMenuItem (parent) with a [b]sub[/b] (child) TMenuItem, when you simply [b]hover[/b] on the parent TMenuItem: Expected behaviour: The Submenu Item should open. Actual behaviour: The Submenu Item opens, [b]AND[/b] it triggers the onClick event of the parent TMenuItem (if one exists) (if AutoCheck is false) [b]or[/b] toggles the 'checked' status of the parent TMenuItem (if AutoCheck is true) [u][b]Steps to reproduce (if AutoCheck is set to true):[/b][/u] 1. Create a TPopupMenu 2. Inside it: Create a Menu Item (parent) Set Checked := true; Set AutoCheck := true; (or see below if you set AutoCheck to false) 3. Inside that menu item: Create a **sub** Menu item (child) to the right. 4. Run the program, and **hover** over the parent menu. Problem: It toggles the 'checked' status of the parent MenuItem. [u][b]Alternative (if AutoCheck set to false):[/b][/u] 1. Create a TPopupMenu 2. Inside it: Create a Menu Item (parent) Set Checked := true; Set AutoCheck := false; 3. Inside that menu item: Create a **sub** Menu item (child) to the right. 4. Create an **OnClick** event handler for the parent Menu Item: [code=pascal]procedure TForm1.MenuItem1Click(Sender: TObject); begin showmessage('Clicked!'); end;[/code] 5. Run the program, and **hover** over the parent menu. Problem: It triggers the OnClick event. I'm running Lazarus 2.0.6 on Windows 10. |
|
Benito, the patch created a regression. Please see the related issue. |
|
I hope you reverted that patch because that it total BS. |
|
I reverted r60679. I am detaching myself. Somebody with Windows as a development OS, please continue with this. |
|
There is no need to change it, it works correctly as it is/was. And why would you want the OnClick event of the parent to fire when merely opening a submenu ? I am only going by what the poster left for everyone else to read. If that was me leaving that patch and it got applied I could be grinning, but I am not that kind of a person. |
|
> And why would you want the OnClick event of the parent to fire when merely opening a submenu ? That is just how menus work You can try it with a main menu rather than a popup menu, or read about it: https://stackoverflow.com/a/14076860/1501222 |
|
that isn't how it works here.. Please incorporate that in your own code if that is what you want. I don't want the parent OnClick to fire in my code when I merely open a sub menu. the submenu is there to allow you to make the choice from the user's stand point not automatically fire off a parent onclick for which is has a different purpose when the sub's open. You can elect to have a parent menu have different action for it's own onclick over a submenu... all of what you need can be done in user code, its not a block on your end and what you are suggesting will then limit the flexibility of the menus if not introduce bugs for other code.. The property request would be to have a property that would optionally trigger an event in the parent of a submenu, but not the onclick. This would have to be a specific event to this task.. For example OnChildMenuOpen. etc If you notice submenu's have their own onclicks, there is a reason for that |
|
@Jamie: > I don't want the parent OnClick to fire in my code when I merely open a sub menu. This is how it works in Delphi 7 (the only version I can test). Given this popup menu Item1->Sub Item 2 When the popupmenu pops up and I click/traverse Item1, it opens Sub (as expected) and it fires Item1's OnClick event. |
|
I know this is a relatively old issue but ... @jamie philbrook: > And why would you want the OnClick event of the parent to fire when merely opening a submenu ? Not that it matters, because if you do have an OnClick handler for any menu item it should fire (or not, if documented!) but here is an example where you would want it to trigger: You have a popup menu with an "Edit" item wich has a submenu with the normal Cut, Copy, Paste, etc. and you want to enable/disable those according to some conditions. Sure, you could do it in the menu's OnPopup event but you might not want to, for whatever reason (e.g. you're sharing that code with a TMainMenu), so your only option is to do it in the OnClick handler of the "Edit" item, but if that doesn't fire .... |
|
I really don't understand you, honestly.. You are trying to do something that can be done in code and you know it.. period.. A child has no reason to force its parent menu to trigger its onclick when the onclick never happen on the parent. If a menuitem has a child and that child gets clicked it will process the onclick handler of the child and if you wish and I have no idea why any one would want this, you can simply call the parent handler directly from any of the childs onclick handlers. This makes absolutely no sense at all because now you have onclicks taking place in the parent from various child menus, does that really make sense to you or any one ? Unless some one here is having issues trying to explain a different case then I am lost .. The trunk behaves exactly the way I would expect it to . No click events on the parent if it has children because the children is what is to be expected to be clicked on not the parent. I suppose you could perform a hack to make the parent clickable but that really makes little sense and it does not fallow the standard behavior.. Please don't hack the LCL just to please a 0.001% of the users when they can accomplish there needs in their own code. |
|
Problem is solvable using a TactionList on the menuitem of interest.. I never found a use for a TactionList until now.. At least saves the rest of us from dealing with unwanted events. |
|
The patch does not do any of that It triggers an onclick when the submenu is opened. Because clicking on an item with a submenu, corresponds to opening its submenu. Delphi's OnClick events have never been about clicking only. E.g. they also trigger when you press enter or use a keyboard shortcut. For menu items it appears to mean WM_COMMAND, which Microsoft documents as the item was selected > A child has no reason to force its parent menu to trigger its onclick when the onclick never happen on the parent. A (parent) menu does not have an onclick Only menu items have one |
|
"A (parent) menu does not have an onclick Only menu items have one" Exactly.. Not everything Delphi does is correct.. Please tell me the logic you find in having other Onclick Events to fire when its not the item that was clicked on? Windows sends a WM_SELECTITEM message to the parents because the string of menus are selected, this is not a onClick event.. You are asking everyone else to change their code to make you happy when its been proven that your thoughts about how it should work is counter productive as it is in Delphi.. If they DEVS wants to supply an option to turn that feature on/off then so be it, at least it will save the rest of us from your desires Why don't you supply a patch to have that feature in the Widget and Class code ? |
|
> Please tell me the logic you find in having other Onclick Events to fire when its not the item that was clicked on? With the patch, only one OnClick fires, no other OnClick Events > You are asking everyone else to change their code to make you happy when its been proven that your thoughts about how it should work is counter productive as it is in Delphi.. It is so they do not need to change their code when porting it from Delphi or to Linux |
|
My suggestions would be to rather add a OnSelect event instead of mangling the onclick event.. The widget can simple send a WM_MENUSELECT message to the LCL part of the code where it can then handle the message and check for an assigned OnSelect Event, and if so then call it with the current TmenuItem as the sender. This event shall not do anything else, no OnClicks and no toggling of the Check mark. Basically give the LCL part a DoSelect event. Each TmenuItem can have an added event "OnSelection".. But for this to go through management, other widgets would want to implement such event, otherwise it would be a windows only solution.. In the mean time, you can simply implement the OwnerDraw for the items. Draw the items yourself in your code and while you are there you can also process each item as you wish. This gives the same effect as you are looking for but with greater control. As for modifying the existing code to make every one happy its has to be talked about as to which way it should be done. I am more then willing to make the changes but this will be a two file patch, one for the widget and the other for the LCL. |
Date Modified | Username | Field | Change |
---|---|---|---|
2019-03-13 12:30 | Benito van der Zander | New Issue | |
2019-03-13 12:30 | Benito van der Zander | File Added: popupmenu.patch | |
2019-03-15 12:42 | Juha Manninen | Assigned To | => Juha Manninen |
2019-03-15 12:42 | Juha Manninen | Status | new => assigned |
2019-03-15 12:45 | Juha Manninen | Fixed in Revision | => r60679 |
2019-03-15 12:45 | Juha Manninen | LazTarget | => - |
2019-03-15 12:45 | Juha Manninen | Note Added: 0114836 | |
2019-03-15 12:45 | Juha Manninen | Status | assigned => resolved |
2019-03-15 12:45 | Juha Manninen | Resolution | open => fixed |
2020-06-13 04:20 | Richard Woolf | Note Added: 0123410 | |
2020-06-13 04:20 | Richard Woolf | File Added: Popup menu bug.jpg | |
2020-06-13 04:31 | Richard Woolf | Issue cloned: 0037210 | |
2020-06-13 04:31 | Richard Woolf | Relationship added | related to 0037210 |
2020-06-14 10:02 | Juha Manninen | Status | resolved => assigned |
2020-06-14 10:02 | Juha Manninen | Resolution | fixed => reopened |
2020-06-14 10:02 | Juha Manninen | Build | Lazarus 2.0.0 r55664M FPC 3.3.1 => Lazarus 2.0.0 r55664M FPC 3.3.1 |
2020-06-14 10:02 | Juha Manninen | Note Added: 0123428 | |
2020-07-05 08:58 | Juha Manninen | Status | assigned => feedback |
2020-07-05 16:16 | jamie philbrook | Note Added: 0123764 | |
2020-07-06 11:09 | Juha Manninen | Assigned To | Juha Manninen => |
2020-07-06 11:09 | Juha Manninen | Status | feedback => new |
2020-07-06 11:09 | Juha Manninen | Note Added: 0123771 | |
2020-07-06 12:29 | jamie philbrook | Note Added: 0123775 | |
2020-07-06 13:19 | Benito van der Zander | Note Added: 0123777 | |
2020-07-06 14:20 | jamie philbrook | Note Added: 0123779 | |
2020-07-09 22:02 | Bart Broersma | Note Added: 0123854 | |
2020-07-11 12:22 | Bart Broersma | Note Edited: 0123854 | View Revisions |
2021-01-03 11:52 | Luis Caballero | Note Added: 0128042 | |
2021-01-03 15:26 | jamie philbrook | Note Added: 0128047 | |
2021-01-04 03:02 | jamie philbrook | Note Added: 0128061 | |
2021-01-09 16:53 | Benito van der Zander | Note Added: 0128202 | |
2021-01-09 17:09 | jamie philbrook | Note Added: 0128204 | |
2021-01-09 23:31 | Benito van der Zander | Note Added: 0128227 | |
2021-01-10 00:14 | jamie philbrook | Note Added: 0128228 | |
2021-01-10 00:16 | jamie philbrook | Note Edited: 0128228 | View Revisions |