Suggested modifications

Apr 22, 2011 at 4:16 PM

Hi,

I started using your menu a couple of days ago since it offers much more flexibility than the menu in the toolkit. Nice work. However, here are a couple of modifications I made and I wanted to bring them to your attention in case you want to implement them in the main version:

1. Disabling a PopupMenuItem should also disable its parent container that is generated by the ItemsControl. For instance, if the ItemsControl is a ListBox, the ListBoxItem must be disabled when the PopupMenuItem is disabled otherwise we can still select them. I added a ParentMenu property to the PopupMenuItem class and when the IsEnabled property changes, I get the container using the ParentMenu's ItemsControl.ItemContainerGenerator and enable or disable it accordingly;

2. In the PopupMenu constructor, I think you should not always create a ListBox (if I'm not mistaken, ItemsControl is always null at this point). Instead, I have modified the get accessor of the ItemsControl property to create the default ListBox only if it can't find a visual descendant of type ItemsControl;

3. Again in the PopupMenu constructor, where the anonymous method is registered for the Opening event, I modified the code to set the ItemsControl's DataContext to the trigger element's DataContext. In all my cases, when I right click something, I want the PopupMenuItems' datacontext to point to the same DataContext of the trigger element. Maybe there is another need that I don't see that the current behavior adresses. In this case, maybe a switch to allow both behaviors would be great.

Please let me know what you think.

Apr 22, 2011 at 6:25 PM

Another point: regarding the OnClick(bool) method of the PopupMenuItem, I happened to have a case where a command would disable the menu item, hence setting its CloseOnClick flag to false. Since the command is executed before the code to close the menuitem in the OnClick method, it caused the menu item to stay open. I think you should compute the result of the condition to close the menu item, raise the click event, execute the command and then close the menu item depending on the result computed before any change can happen as a result of the click event or command execution.

Coordinator
May 3, 2011 at 3:27 AM
Edited May 4, 2011 at 8:13 AM

Hi TheSowee,

Thanks for calling my attention to those issues and thumbs up for your well thought out opinions. 

I've tried to follow all your recommendations in the latest change set. So feel free to check it out and send me your feedback.

By the way I'm about to take the project out of the beta phase. So prior to it you've been added to the project as a developer in case you'd want to commit additional changes to the repository.

Cheers

Ziad

Coordinator
May 10, 2011 at 12:15 AM
Edited May 17, 2011 at 11:42 AM
Hi TheSowee,

Please check the latest release version 2.6.1 in which I believe all above issues have been successfully resolved.

Regards
Ziad