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

Added multi window support for WinUI 3 #4819

Merged
merged 16 commits into from
Mar 19, 2024

Conversation

Joost-Jens-Luminis
Copy link
Contributor

@Joost-Jens-Luminis Joost-Jens-Luminis commented Feb 16, 2024

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

I have added support for navigating to a view that opens in a new window. Also supports navigation inside the new window. The same viewmodel can therefore be used to navigate to from multiple different windows. (You do get a new viewmodel instance each time of course)

⤵️ What is the current behavior?

There is no multi window support

🆕 What is the new behavior (if this is a feature change)?

We will now support multiple windows.

💥 Does this PR introduce a breaking change?

No.

🐛 Recommendations for testing

The Playground app for Windows is updated with samples. I recommend testing more advanced navigation scenarios.

📝 Links to relevant issues/docs

🤔 Checklist before submitting

@Joost-Jens-Luminis
Copy link
Contributor Author

@dotnet-policy-service agree

@Joost-Jens-Luminis
Copy link
Contributor Author

Joost-Jens-Luminis commented Feb 19, 2024 via email

Copy link
Member

@Cheesebaron Cheesebaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please do the following:

  • use file scoped namespaces on new files
  • add some more description on how this works in general, since there are quite a few moving parts

MvvmCross/Platforms/WinUi/Views/MvxApplication.cs Outdated Show resolved Hide resolved
MvvmCross/MvvmCross.csproj Outdated Show resolved Hide resolved
MvvmCross/MvvmCross.csproj Outdated Show resolved Hide resolved
MvvmCross/Navigation/MvxNavigationService.cs Outdated Show resolved Hide resolved
MvvmCross/Platforms/WinUi/Presenters/Models/INeedWindow.cs Outdated Show resolved Hide resolved
@Joost-Jens-Luminis
Copy link
Contributor Author

Joost-Jens-Luminis commented Feb 23, 2024

As requested a further description on how this works.

  1. A new attribute has been added MvxNewWindowPresentationAttribute.cs has been added which can be put above views. If detected, the ViewPresenter will open a new window.

  2. All navigate calls that have to support to be executed in a Window other then the main Window have to pass themselves as the Source. (Note: It is not required that they pass themselves, but is the most logical option usually).

  3. An attempt is then made to find the source in any of the open windows, (before this was _rootFrame). If no window is found the mainWindow is used instead.

  4. The newly created ViewModel (from the request) is then registered with the Window, so if it wants to call Navigate we can find it.

Would it help if I replace the MvxWindowsViewPresenter with the new MvxMultiWindowViewPresenter.cs

As a further note: I have updated the Playground with the new functionality.

@Joost-Jens-Luminis
Copy link
Contributor Author

I realised over the weekend I forgot to add an explanation over the IMvxNeedWIndow.
The IMvxNeedWindow is used to give the page access to the Window it is in. This allows the page, and by extension the viewmodel via Binding, to set and change the Title and possibly other things. (We only needed it for the title :) )

@Cheesebaron
Copy link
Member

Sorry for the delay. I need to find a Windows PC to test this one, I will try to get it reviewed this weekend.

@Joost-Jens-Luminis
Copy link
Contributor Author

Joost-Jens-Luminis commented Mar 15, 2024

Have you been able to check it out @Cheesebaron ? Is there anything you want me to do still on this or do you have any questions/issues found?
If it helps, we are using this in production (it's in a slightly different form, but we would migrate to this if it gets in the library)

@Cheesebaron
Copy link
Member

Sorry, was out traveling.

Will get to this one today :)

@Cheesebaron Cheesebaron merged commit 58ba92d into MvvmCross:develop Mar 19, 2024
6 checks passed
@Cheesebaron Cheesebaron added the t/feature Feature request type label Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t/feature Feature request type
Development

Successfully merging this pull request may close these issues.

None yet

2 participants