Add hover and leave callbacks for linkprovider

[marvinthepa]

Fixes #3056.

I guess this needs another breaking change when LinkMatchers are removed, to replace the ILinkMatcherOptions argument to the WebLinksAddon constructor with something not referring to LinkMatchers..

[Tyriar]

I think it would be better if we has link provider-specific options:

export interface ILinkProviderOptions {
  hover?(event: MouseEvent, text: string): void;
  leave?(event: MouseEvent, text: string): void;
}

...

private _options: ILinkMatcherOptions | ILinkProviderOptions = {}

[marvinthepa]

private _options: ILinkMatcherOptions | ILinkProviderOptions = {}

Do you want to allow both types as arguments on WebLinksAddon?
I am afraid my knowledge of typescript is not sufficient to get that to compile.

Or do you want to allow both as argument to WebLinksProvider?

As a suggestion, I will push an alternative that I could get working:
Converting from ILinkMatcherOptions to ILinkProviderOptions in WebLinksAddon.activate.
If you do not like that I am afraid I will need a little help.

[Tyriar]

I don't think TS would fuss about this because all properties on both interfaces are optional, so essentially any object matches the interface, it should however provide nice intellisense when something is using the API.

[Tyriar]

Ah I see the problem you were hitting now, I'll push a fix using a cast as it should be safe to do that since the interfaces only have optional props.

[marvinthepa]

I don't think TS would fuss about this because all properties on both interfaces are optional, so essentially any object matches the interface, it should however provide nice intellisense when something is using the API.

It did fuss about it in my experience 🙁.

E.g.:

  constructor(
    private _handler: (event: MouseEvent, uri: string) => void = handleLink,
    private _options: ILinkMatcherOptions | ILinkProviderOptions = {},
    private _useLinkProvider: boolean = false
  ) {
    this._options.matchIndex = 1; //  Property 'matchIndex' does not exist on type 'ILinkProviderOptions'.ts(2339)
  }

I will try to solve this in a different way.

[Tyriar]

@marvinthepa yeah that's what I ended up seeing, you can check the change I just pushed for an alright workaround

[marvinthepa]

When linkmatchers are removed, this can go and the ILinkMatcherOptions argument can be replaced by ILinkProviderOptions..

[Tyriar]

@marvinthepa normally using as is bad, but it doesn't matter much here since the objects are definitely compatible.

[Tyriar]

I moved this here to avoid setting it when the object gets used for a link provider

[marvinthepa]

I knew there was something about TS that I didn't know.

However, this will break clients that pass their callbacks as tooltipCallback and leaveCallback (instead of hover and leave, and then switch _useLinkProvider to true. Shouldn't there be at least a warning in this case?

[Tyriar]

I think it's fine like this since link matchers should be going away soon.

TS wasn't as clever as I thought it was by not narrowing the options type when you enter in hover, which makes sense now that I think about it:

If people are confused about the API in the meantime, they can go into the types to view the definition and/or look at the object type in intellisense:

Passing the object as ILinkProviderOptions makes it act nicely:

I personally don't think it's worth adding warnings since link matchers are deprecated and it's such a simple addon API.