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

Expanding on App Install URI Protocol Support - Winget Package URIs #2733

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

AgneLukoseviciute
Copy link
Contributor

@AgneLukoseviciute AgneLukoseviciute commented Apr 24, 2024

Summary of the pull request

With this PR, if you win+R: ms-devhome:add-apps-to-cart?WingetURIs="x-ms-winget://winget/Git.Git","x-ms-winget://winget/Microsoft.VisualStudioCode", "x-ms-winget://winget/Microsoft.PowerToys" [updated 5/22]
You will be taken to this view of Dev Home:
image

This also adds a message dialog for when there is a machine configuration already in progress: [updated 5/22]
image

References and relevant issues

#2428

Detailed description of the pull request / Additional comments

This is mostly a refactoring of the code added in: #2642 to support the winget URIs scenario which allows for specifying multiple packages to select and unlike the "search" option this does not modify the search input box in the AppManagementViewModel.

Validation steps performed

Manually tested through win+R both the search and the new URIs query params.

PR checklist

.FirstOrDefault();
_log.Information("Starting add-apps-to-cart activation");
_navigationService.NavigateTo(typeof(SetupFlowViewModel).FullName!);
_setupFlowViewModel.StartAppManagementFlow(queryType == ActivationQueryType.Search ? identifiers[0] : null);
Copy link
Contributor

Choose a reason for hiding this comment

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

why null if ActivationQueryType isn't search?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's because we only want to do the query prefilling into the search box piece in the search scenario.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

similar to the other comment, lmk if you think using a bool prefillSearch might help with clarity here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know if null is passed anywhere else into StartAppManagementFlow? MainPageViewModel.StartAppManagementFlow throws if the string is null when trying to log the query.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the only place we call StartAppManagementFlow -- it was added with the previous PR: https://github.com/microsoft/devhome/pull/2642/files

With the change to not log the query string, StartAppManagementFlow should not throw if null (tested locally to confirm)

@AgneLukoseviciute AgneLukoseviciute marked this pull request as draft April 25, 2024 20:29
@AgneLukoseviciute
Copy link
Contributor Author

AgneLukoseviciute commented Apr 25, 2024

After discussing this feature with @AmelBawa-msft, I will be refactoring this PR slightly to take in and utilize WingetPackageURIs instead of WingetIDs. This way we'll be enabling package sources other than winget with minimal extra effort and this can later benefit from any WingetPackageUri feature additions. Converting to draft until I get this change in.

@AgneLukoseviciute AgneLukoseviciute marked this pull request as ready for review May 1, 2024 20:59
@AgneLukoseviciute AgneLukoseviciute changed the title Expanding on App Install URI Protocol Support - Query by wingetIds Expanding on App Install URI Protocol Support - Winget Package URIs May 1, 2024
@krschau krschau requested a review from MattHyman May 2, 2024 20:22
@AmelBawa-msft AmelBawa-msft added the Needs-Second Pull request that needs another approval label May 22, 2024
Copy link
Contributor

@krschau krschau left a comment

Choose a reason for hiding this comment

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

Approving pending WindowEx modification.

src/Services/AppInstallActivationHandler.cs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Second Pull request that needs another approval
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[URI Protocol] Activate Application Install flow with specific applications already selected
4 participants