Public event for view extensions to indicate a close action on the extensions sidebar.

[reddyashish]

Purpose

This PR is for the task: https://jira.autodesk.com/browse/DYN-3214

@saintentropy requested an improvement to have a public event that would be raised when the view extension is closed from the right hand side bar. In the current environment, we have a static internal event CloseExtension on the DynamoView that would be raised when the view extension is closed. But a public event on the ViewLoadedParams would be useful to directly perform some action on the ViewExtension when it is closed. This public event ViewExtensionClosed is similar to the CloseExtension event on DynamoView.

The other option would be to convert the internal CloseExtension event into public and let the users use it. But Craig mentioned that it would be easy to find and use if it is on the ViewLoadedParams.
Let me know what the team thinks about both these options and will creating an extra event on the ViewLoadedParams cause any downsides?

Declarations

Check these if you believe they are true

• The codebase is in a better state after this PR
• Is documented according to the standards
• The level of testing this PR includes is appropriate
• User facing strings, if any, are extracted into *.resx files
• All tests pass using the self-service CI.
• Snapshot of UI changes, if any.
• Changes to the API follow Semantic Versioning and are documented in the API Changes document.
• This PR modifies some build requirements and the readme is updated

Reviewers

@QilongTang @mmisol @mjkkirschner @aparajit-pratap
.

[QilongTang]

@reddyashish Looks clean and do you think it would be possible to add a unit test for that in one of the view extension tests?

[mmisol]

Agree we should have a test.

Also, I think there is a design issue with this approach. As an extension author I wouldn't want to get ViewExtensionClosed events for extensions that are not my own. Can we come up with a design that triggers the event to the right extension instead?

[mjkkirschner]

to @mmisol's point - perhpas a hook rather than event makes more sense - for example, the Loaded or Startup methods that extension authors implement are already idiomatic for methods that are only called for the developer's own extension.

[mmisol]

@mjkkirschner I agree that would be the cleanest approach. However I don't think we can change IViewExtension because it would be a breaking change, right? Sometimes, I feel interfaces are a versioning pain...

[mjkkirschner]

@mmisol we can add a new interface and merge them together at 3.x

MyViewExtension : IViewExtension, IViewExtensionCloseHook

we would need to document this of course.

[mmisol]

@mjkkirschner While that would work, I think it might be better to have a base class, kinda like we have with the start configuration for DynamoModel. That way we wouldn't need to add an IViewExtension3, if eventually needed.

MyViewExtension : IViewExtension, BaseViewExtension

And whenever we are on .NET core just use this :) https://docs.microsoft.com/en-us/dotnet/csharp/tutorials/default-interface-methods-versions

[mjkkirschner]

@mmisol - yes I am so ready for default implementations.

hmm, just for some thoughts
https://stackoverflow.com/questions/56867/interface-vs-base-class
though it would solve some maintenance headaches for us, I could imagine we could evolve ViewExtensions/Extensions to allow a mixin pattern which could be quite nice.

I think the interface pattern is actually less risky for API breaks because we can't accidentally change any implementation details out from under devs by making some change to the base class.

I guess we could be strict and enforce that all members are abstract for this base class?

[mmisol]

@mjkkirschner It could be abstract but its members can't be, otherwise we would be in the same circumstances. It should have dummy/empty implementations, i.e. the defaults.

Here is a link to the example base class I was talking about:
https://github.com/DynamoDS/Dynamo/blob/master/src/DynamoCore/Models/DynamoModel.cs#L512

Here is a link to how we use it in the constructor of DynamoModel:
https://github.com/DynamoDS/Dynamo/blob/master/src/DynamoCore/Models/DynamoModel.cs#L563

[reddyashish]

Closed this as I had to pull in latest changes from master and opened a new PR: #11260 to address the task.