Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open/close menu events are confusing #4443

Open
grokys opened this issue Aug 6, 2020 · 2 comments
Open

Open/close menu events are confusing #4443

grokys opened this issue Aug 6, 2020 · 2 comments
Assignees
Labels

Comments

@grokys
Copy link
Member

grokys commented Aug 6, 2020

The following events are available on Menu/ContextMenu:

  • MenuBase.MenuOpened
  • ContextMenu.ContextMenuOpening
  • ContextMenu.ContextMenuClosing

I see a few issues here:

  1. The MenuOpened event is inherited by ContextMenu meaning that its open event has the Menu prefix but its opening event has the ContextMenu prefix
  2. Why is a cancelable Closing event needed? I've never seen a context menu that can't be closed, and having one sounds like a problem
  3. Why are the context menu events not routed events?
  4. There should probably be an Opening event on Menu too (though it shouldn't be cancelable)

I suggest the following;

  • MenuBase has Opened and Closed
  • ContextMenu has a cancelable Opening
  • Menu has a non-cancelable Opening
  • The closing event is removed from ContextMenu

@WojciechKrysiak you implemented ContextMenuOpening/Closing - what do you think?

@grokys grokys added the feature label Aug 6, 2020
@WojciechKrysiak
Copy link
Contributor

I have no preference either way - the only reason I implemented it like that was to mirror WPF naming and behavior, as you can mark the event as handled through its args. This has the same effect as cancelling it.

As for why it wasn't done as a routed event - I can't remember, but most likely lack of skill back then.

I agree that unifying the naming is a good aim, but it would introduce additional work for people porting from WPF.

@maxkatz6
Copy link
Member

Also, ToolTip events #15493

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants