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

feat: Add doNotActivate parameter to NavigateAsync method from Navigation Extensions #2397

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xperiandri
Copy link

GitHub Issue (If applicable): closes #2381

PR Type

What kind of change does this PR introduce?

  • Feature

What is the current behavior?

Window is always activated as soon as possible

What is the new behavior?

Window is activated either as soon as possible or right after the host is started (all hosted services completed)

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Docs have been added/updated which fit documentation template. (for bug fixes / features)
  • Unit Tests and/or UI Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Wasm UI Tests are not showing unexpected any differences. Validate PR Screenshots Compare Test Run results.
  • Contains NO breaking changes
  • Updated the Release Notes
  • Associated with an issue (GitHub or internal)

@jeromelaban jeromelaban requested a review from nickrandolph June 27, 2024 02:40
@nickrandolph
Copy link
Contributor

@xperiandri can you provide more context on the scenario you're trying to get to work?

@xperiandri
Copy link
Author

OpenIddict Client starts a hosted service to determine protocol activation, redirect it and close the app.
With you current logic the second instance of the app manages to activate the window before the app exited. It causes a window to blip and close.
My modification allows to prevent that if necessary

@nickrandolph
Copy link
Contributor

ok let me double check this next week - I thought we had a mechanism that would ensure some services are started before activation (eg localization from recollection).
If we do want to provide this, we'll possibly look at providing it as a registration on the DI, rather than as an argument to the app builder.
Thanks again for this suggestion!

@xperiandri
Copy link
Author

I need this behavior, please

@xperiandri
Copy link
Author

And I would prefer ability to just plug OpenIddict Client and it worked then declaring some additional services, implement some additional interfaces, etc.

@@ -22,5 +22,6 @@ internal interface IRootViewInitializer
/// <param name="window"></param>
/// <param name="element"></param>
/// <param name="loadingTask"></param>
void InitializeViewHost(Window window, FrameworkElement element, Task loadingTask);
/// <param name="doNotActivate"></param>
void InitializeViewHost(Window window, FrameworkElement element, Task loadingTask, bool doNotActivate = false);
Copy link
Member

Choose a reason for hiding this comment

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

That's odd using a double negative like that. That's confusing.

@nickrandolph
Copy link
Contributor

@xperiandri the interface I was thinking of is the IStartupService, which will effectively block activation until it returns. You can register this service in order to block activation.
I'd like to explore what we can do to make it eaiser to just "plug OpenIDdict client" and leverage the authentionservice.

@xperiandri
Copy link
Author

xperiandri commented Jul 16, 2024

Sounds like a good idea in general.
The only question is how to make https://github.com/openiddict/openiddict-core/blob/dev/src/OpenIddict.Client.SystemIntegration/OpenIddictClientSystemIntegrationActivationHandler.cs become IStartupService

Inherit from it, remove registration as IHostedService from DI and register as IStartupService?

Kevin sealed OpenIddictClientSystemIntegrationActivationHandler, probably we can ask to unseal

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.

[ApplicationBuilder] must not create Window until it ensures that the host started
3 participants