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

Allow Overriding FindOwnerWindow #221

Merged
merged 5 commits into from
Sep 26, 2022

Conversation

RFBomb
Copy link
Contributor

@RFBomb RFBomb commented Sep 24, 2022

Description

Rename the static FindOwnerWindow method to FindOwner and introduce a protected virtual FindOwnerWindow method.

  • Fixes #(issue)

Currently, most of the service action can be modified by introducing different factories for the type locators or dialog factories. The exception to this rule is locating the owner window. This change fixes that.

In my application, I have many custom controls that are a combination of other controls. (For example, a control that houses around 10 comboboxes, about 6 buttons, a listbox, etc). This is controlled by what I deemed a 'ControlModel'. The 'ControlModel' has a property called 'Parent' which points to its parent ViewModel.

The tricky thing here is that the 'Parent' may not be on the registered view, the Parent's Parent may be though. So what I want to do is traverse up the 'Parent-Tree' until I find the registered view. Currently, this library does not allow for that.

So this PR exposes the method of finding the parent window and allows overriding just that functionality if needed. I figured this would be fine since the DialogService class is not sealed.

My usage would look something like this:

protected override Window FindOwnerWindow(INotifyPropertyChanged viewModel)
{
    if (viewModel is IViewModel vm)
    {
        try
        {
            return base.FindOwnerWindow(viewModel);
        }
        catch (ViewNotRegisteredException) when vm.ParentViewModel != null
        {
            return FindOwnerWindow(vm.ParentViewModel); // Recurse up the ParentViewModels until the registered view is found
        }
    } else
        return base.FindOwnerWindow(viewModel);        
}

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas

@FantasticFiasco FantasticFiasco self-assigned this Sep 26, 2022
@FantasticFiasco
Copy link
Owner

Hi @RFBomb!

Thanks for the contribution, I'll make a new release.

@FantasticFiasco FantasticFiasco merged commit 7a4419c into FantasticFiasco:master Sep 26, 2022
@FantasticFiasco
Copy link
Owner

A new NuGet package has now been published.

@RFBomb RFBomb deleted the AllowOverrides branch October 3, 2022 11:22
@RFBomb
Copy link
Contributor Author

RFBomb commented Oct 3, 2022

Awesome, thanks for the quick turnaround!

@RFBomb
Copy link
Contributor Author

RFBomb commented Oct 3, 2022

Hey,
I just realized that since this was updated to use .Net6.0-Windows, my app written in .Net5.0-Windows no longer works. My company is still on VS2019, and as such doesn't have access to .net6.

Since .NetFramework 462 is still listed as a target here, can netcoreapp3.1; be added as a target?

@RFBomb RFBomb restored the AllowOverrides branch October 3, 2022 17:35
@FantasticFiasco
Copy link
Owner

This codebase has removed support for .NET 5 due to its deprecation as of May 10, 2022, and .NET Core 3.1 was removed in preparation for its deprecation December 13, 2022. I'm hoping you'll be able to migrate.

@RFBomb
Copy link
Contributor Author

RFBomb commented Nov 7, 2022

Unfortunately I will be unable to migrate, as the company I develop for will not be upgrading to VisualStudio 2022 any time in the forseeable future. As such, I'll just have to have it forked until that time.

@RFBomb RFBomb deleted the AllowOverrides branch October 23, 2023 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants