New ToolTipClosing, ToolTipOpening attached events and ToolTip.Opened, ToolTip.Closed

[maxkatz6]

What does the pull request do?

public class ToolTip
{
+     public static readonly RoutedEvent ToolTipOpeningEvent;
+     public static readonly RoutedEvent ToolTipClosingEvent;
+     public static void AddToolTipOpeningHandler(Control element, EventHandler<RoutedEventArgs> handler);
+     public static void RemoveToolTipOpeningHandler(Control element, EventHandler<RoutedEventArgs> handler);
+     public static void AddToolTipClosingHandler(Control element, EventHandler<RoutedEventArgs> handler);
+     public static void RemoveToolTipClosingHandler(Control element, EventHandler<RoutedEventArgs> handler);
}

Behavior is ported from WPF, but implementation is different, just like our ToolTip API is already quite different. Most of this decisions were made for consistency with the rest of the framework.

Common:

1. When ToolTipOpening is raised, it's possible to replace TipProperty, which will cause opening of another tip.
2. It's also possible to set ToolTipOpening Handled=true, which will essentially cancel tooltip opening.
3. It's not possible to cancel ToolTipClosing in any way.

Different:

1. Instead of adding attached events to the ToolTipService, I added them to ToolTip class. Following consistency with other attached ToolTip APIs.
2. Same about adding new members to the Control class - we never added anything ToolTip (or ContextMenu) related to the Control type itself - everything is attached.
3. Opened/Closed events are direct-routed in WPF, but CLR events in this PR. Following the same behavior with ContextMenu and Flyout events.

To raise Opening/Closing events, I used IsOpen.Coerse callback, which is also handy to cancel tooltip opening. In WPF these events were only usable via ToolTipService, and handled there. In general, in Avalonia tooltip is completely usable without ToolTipService, so these events should as well.

Checklist

• Added unit tests (if possible)?
• Added XML documentation to any related classes?
• Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation

Fixed issues

Fixes #15232

[avaloniaui-bot]

You can test this PR using the following package version. 11.2.999-cibuild0047624-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[BAndysc]

In case of custom drawn controls, tooltip content may depend on the position of the cursor thus it would be very useful to have a location of the pointer in ToolTipOpeningEvent that is causing the tooltip to show up if it is possible

[MrJul]

3. It's not possible to cancel ToolTipClosing in any way.

Is it a technical limitation? I can see uses in preventing tooltips from closing.
(Edit: for reference ContextMenu and Flyout have preventable Closing)

[maxkatz6]

@MrJul I am just following WPF behavior here. Technically, it can be done.

@BAndysc would be a bit tricky to pass position from ToolTipService to ToolTip.

[MrJul]

Should we unify the API between all popup-like controls here?

Having all necessary events (Opening, Opened, Closing, Closed) on Popup and re-trigger them on ContextMenu, ToolTip and Flyout. Currently it feels a bit random, and the signature sometimes differ between controls (e.g. CancelEventArgs vs standard event args).

[MrJul]

I'll let @grokys comment on this, but I'm a bit worried of having such side effects in coercion methods, potentially leading to reentrancy or other hard to debug problems. Coercion methods are called in the middle of setting an EffectiveValue, and I don't think this code currently handle reentrancy. (For example, PropertyChanged isn't called until all the internal effective value bookkeeping is done to avoid issues.)

[maxkatz6]

Yeah, didn't think about reentrancy. It's technically possible to change IsOpen value inside.

[TomEdwardsEnscape]

It's also possible for the events to be raised when the value isn't changing. Aside from any internal coercion calls, the user can do this at any time:

myControl.CoerceValue(ToolTip.IsOpenProperty);

ToolTip.IsOpenChanged is the right place for this code. It's a bit weird that IsOpen will be true when ToolTipOpening is raised, but the event name makes it very clear what's going on.

[robloo]

Should we unify the API between all popup-like controls here?

Having all necessary events (Opening, Opened, Closing, Closed) on Popup and re-trigger them on ContextMenu, ToolTip and Flyout. Currently it feels a bit random, and the signature sometimes differ between controls (e.g. CancelEventArgs vs standard event args).

I'm a big fan of unifying APIs everywhere it's possible. This is a great place to do it IMO.

Also note that Expander is related to this discussion and while the naming is different (Expanding/Collapsing/etc.) usage and API patterns should be the same as the rest.

[billhenn]

@maxkatz6 The update works well. I was able to dynamically update a tooltip before it displayed.

I agree that having a consistent API for the Opening, Opened, Closing, and Closed events in various places would be ideal.

I'm also usually a fan of using something like e.Cancel to prevent a default behavior. e.Handled is technically there to mean stop routing an event further. The way WPF uses it for ToolTipOpening events is sort of a gray area, since they hijack e.Handled to say whether to cancel the default functionality. One could probably argue either way on that usage.

[robloo]

@billhenn @maxkatz6

I'm also usually a fan of using something like e.Cancel to prevent a default behavior. e.Handled is technically there to mean stop routing an event further. The way WPF uses it for ToolTipOpening events is sort of a gray area, since they hijack e.Handled to say whether to cancel the default functionality. One could probably argue either way on that usage.

Note that this exact topic was discussed for Expander here: #9979 (comment). The agreement was to add CancelRoutedEventArgs to go along with the existing CancelEventArgs establishing Avalonia's convention. We differ here from WPF but it's better for the same reasons you state. All controls should be using this convention -- tooltip as well.

Just keep in mind Avalonia has these cancel event args already and 3rd party libraries should ideally use them as well.

[robloo]

As discussed new events here should follow existing conventions and use CancelRoutedEventArgs. This follows Expander which itself was based on what Flyout did (although flyout isn't API symmetric and doesn't allow cancelling an open).

[jmacato]

@cla-avalonia recheck

[cla-avalonia]

• All contributors have signed the CLA.

[maxkatz6]

@cla-avalonia agree

[maxkatz6]

Updated PR:

1. Reimplemented attached events to not rely on Coerce callback
2. Changed Opening args type from RoutedEventArgs to CancelRoutedEventArgs
3. Removed Opened and Closed events. These weren't part of the initial issue, and can be re-added in the future. It's likely involve breaking changes in ContextMenu or Flyout controls, as these have different events. Also, Popup itself wasn't really intended to be cancellable.

Note, Closing event is still not cancellable. If necessary, it can be implemented later.

[avaloniaui-bot]

You can test this PR using the following package version. 11.2.999-cibuild0048346-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[billhenn]

@maxkatz6 Thanks, I tried the PR build and it does what we needed for the tooltip opening notification.

[avaloniaui-bot]

You can test this PR using the following package version. 11.2.999-cibuild0048447-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]