feat: Implement Launcher features on Skia#22395
Draft
morning4coffe-dev wants to merge 1 commit intomasterfrom
Draft
feat: Implement Launcher features on Skia#22395morning4coffe-dev wants to merge 1 commit intomasterfrom
Launcher features on Skia#22395morning4coffe-dev wants to merge 1 commit intomasterfrom
Conversation
…chFolderAsync` for Skia
github-actions
bot
added
the
area/skia ✏️
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements Launcher features for the Skia platform, specifically adding support for launching files and folders. The implementation provides the LaunchFileAsync, LaunchFolderAsync, and LaunchFolderPathAsync methods that were previously not implemented on Skia.
Changes:
- Implements file and folder launching functionality on Skia by adding methods to launch files and folders using the underlying URI launching infrastructure
- Updates generated code to exclude Skia from the NotImplemented attributes for the newly implemented methods
- Adds validation for file/folder existence and proper error logging
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Uno.UWP/System/Launcher.skia.cs | Implements LaunchFileAsync, LaunchFolderAsync, and LaunchFolderPathAsync methods with internal helper methods for file/folder path handling |
| src/Uno.UWP/Generated/3.0.0.0/Windows.System/Launcher.cs | Updates conditional compilation to mark Skia platform as implemented (changes __SKIA__ to false in #if directives) |
| return false; | ||
| } | ||
|
|
||
| var folderUri = new Uri(folderPath); |
There was a problem hiding this comment.
The URI is being created incorrectly from a folder path. The new Uri(folderPath) constructor expects an absolute URI string, but folderPath is likely a local file system path. This will fail for relative paths and may not work correctly for absolute paths without the "file://" scheme. Use the same approach as the file path handling - convert the path to a proper file URI using Path.GetFullPath() combined with the Uri constructor that accepts a file path.
Comment on lines
+30
to
+74
| public static IAsyncOperation<bool> LaunchFileAsync(IStorageFile file) | ||
| { | ||
| if (file == null) | ||
| { | ||
| throw new ArgumentNullException(nameof(file)); | ||
| } | ||
|
|
||
| return LaunchFileInternalAsync(file.Path).AsAsyncOperation(); | ||
| } | ||
|
|
||
| public static IAsyncOperation<bool> LaunchFileAsync(IStorageFile file, LauncherOptions options) | ||
| { | ||
| return LaunchFileAsync(file); | ||
| } | ||
|
|
||
| public static IAsyncOperation<bool> LaunchFolderPathAsync(string path) | ||
| { | ||
| if (string.IsNullOrEmpty(path)) | ||
| { | ||
| throw new ArgumentNullException(nameof(path)); | ||
| } | ||
|
|
||
| return LaunchFolderPathInternalAsync(path).AsAsyncOperation(); | ||
| } | ||
|
|
||
| public static IAsyncOperation<bool> LaunchFolderPathAsync(string path, FolderLauncherOptions options) | ||
| { | ||
| // options are currently not used, but we provide the overload for API compatibility | ||
| return LaunchFolderPathAsync(path); | ||
| } | ||
|
|
||
| public static IAsyncOperation<bool> LaunchFolderAsync(IStorageFolder folder) | ||
| { | ||
| if (folder == null) | ||
| { | ||
| throw new ArgumentNullException(nameof(folder)); | ||
| } | ||
|
|
||
| return LaunchFolderPathAsync(folder.Path); | ||
| } | ||
|
|
||
| public static IAsyncOperation<bool> LaunchFolderAsync(IStorageFolder folder, FolderLauncherOptions options) | ||
| { | ||
| return LaunchFolderAsync(folder); | ||
| } |
There was a problem hiding this comment.
The new LaunchFileAsync and LaunchFolderAsync methods lack test coverage in the runtime tests. While a manual test exists in the SamplesApp, there are no automated runtime tests for these new methods. Consider adding runtime tests similar to the existing Launcher tests in Given_Launcher.cs to validate null parameter handling, file existence checks, and successful launching scenarios.
| return false; | ||
| } | ||
|
|
||
| var fileUri = new Uri(filePath); |
There was a problem hiding this comment.
The URI is being created incorrectly from a file path. The new Uri(filePath) constructor expects an absolute URI string, but filePath is likely a local file path. This will fail for relative paths and may not work correctly for absolute paths without the "file://" scheme. Use new Uri(filePath, UriKind.Absolute) for absolute paths, or better yet, convert the path to a proper file URI using Path.GetFullPath() combined with the Uri constructor that accepts a file path.
Contributor
|
🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-22395/wasm-skia-net9/index.html |
Contributor
|
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-22395/docs/index.html |
Contributor
|
The build 191948 found UI Test snapshots differences: Details
|
Contributor
|
The build 191948 found UI Test snapshots differences: Details
|
Contributor
|
The build 191948 found UI Test snapshots differences: Details
|
Contributor
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GitHub Issue: closes #22308
PR Type:
PR Checklist ✅
Please check if your PR fulfills the following requirements:
Screenshots Compare Test Runresults.