Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds two WinAppSdk 1.8 sample applications to the main branch: CameraCaptureUI and StoragePickers. The changes include sample code, configuration files, deployment images, and project manifests for both C# and C++ implementations of the StoragePickers sample.
Key Changes
- Added StoragePickers sample with C# and C++ implementations
- Included deployment documentation images (deploy6.png, deploy7.png, deploy8.png)
- Added project configuration files for packaged and unpackaged deployment scenarios
Reviewed Changes
Copilot reviewed 83 out of 120 changed files in this pull request and generated 32 comments.
Show a summary per file
| File | Description |
|---|---|
| images/*.png | Deployment documentation images (binary files) |
| cs-sample/app.manifest | Application manifest for unpackaged scenarios |
| cs-sample/Properties/launchSettings.json | Launch profiles for packaged/unpackaged modes |
| cs-sample/Properties/PublishProfiles/*.pubxml | Publish configurations for different architectures |
| cs-sample/Package.appxmanifest | MSIX package manifest |
| cs-sample/MainWindow.xaml.cs | C# sample implementation |
| cs-sample/MainWindow.xaml | XAML UI definition |
| cpp-sample/pch.cpp | Precompiled header source |
| cpp-sample/MainWindow.idl | WinRT component definition |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <PublishProtocol>FileSystem</PublishProtocol> | ||
| <Platform>x64</Platform> | ||
| <RuntimeIdentifier>win-x64</RuntimeIdentifier> | ||
| <PublishDir>bin\\\win-x64\publish\</PublishDir> |
There was a problem hiding this comment.
The PublishDir path contains triple backslashes (\\\) which is likely unintentional. This should be either a single backslash or use the standard MSBuild path format with $(Configuration) and $(TargetFramework) variables like in win-x86.pubxml.
| <PublishProtocol>FileSystem</PublishProtocol> | ||
| <Platform>ARM64</Platform> | ||
| <RuntimeIdentifier>win-arm64</RuntimeIdentifier> | ||
| <PublishDir>bin\\\win-arm64\publish\win-arm64\</PublishDir> |
There was a problem hiding this comment.
The PublishDir path has two issues: it contains triple backslashes (\\\) and duplicates 'win-arm64' in the path. This should be corrected to match the pattern in win-x86.pubxml or use a consistent format.
| <Platform>ARM64</Platform> | ||
| <PublishDir>bin\\\win-x64\publish\win-x64\</PublishDir> |
There was a problem hiding this comment.
Platform is set to ARM64 but PublishDir references win-x64. This mismatch will cause the wrong architecture binaries to be published to the wrong location. Either the Platform should be x64 or the PublishDir should reference ARM64.
| </Dependencies> | ||
|
|
||
| <Resources> | ||
| <Resource Language="zh-Hans"/> <!--x-generate--> |
There was a problem hiding this comment.
The language resource is hardcoded to Chinese Simplified (zh-Hans) which may not be appropriate for a general sample. Consider using 'en-US' or 'x-generate' to support multiple languages, or add a comment explaining why this specific language was chosen.
| private string[] GetFileFilters() | ||
| { | ||
| string input = FileTypeFilterInput.Text?.Trim() ?? ""; | ||
| if (string.IsNullOrEmpty(input)) | ||
| return ["*"]; | ||
|
|
||
| return input.Split([','], StringSplitOptions.RemoveEmptyEntries) | ||
| .Select(s => s.Trim()) | ||
| .ToArray(); | ||
| } |
There was a problem hiding this comment.
The collection expression [\"*\"] syntax requires C# 12 or later. While this is likely fine for .NET 8, consider verifying that the project's language version supports this feature, or use the more traditional new[] { \"*\" } syntax for broader compatibility.
| { | ||
| public partial class Scenario2_CaptureVideo : Page | ||
| { | ||
| private MainPage rootPage; |
There was a problem hiding this comment.
Field 'rootPage' can be 'readonly'.
| catch (Exception ex) | ||
| { | ||
| LogResult($"Error in New FileOpenPicker: {ex.Message}"); | ||
| } |
There was a problem hiding this comment.
Generic catch clause.
| catch (Exception ex) | ||
| { | ||
| LogResult($"Error in New PickMultipleFilesAsync: {ex.Message}"); | ||
| } |
There was a problem hiding this comment.
Generic catch clause.
| catch (Exception ex) | ||
| { | ||
| LogResult($"Error in New FileTypeChoices: {ex.Message}"); | ||
| } |
There was a problem hiding this comment.
Generic catch clause.
| catch (Exception ex) | ||
| { | ||
| LogResult($"Error in New FolderPicker: {ex.Message}"); | ||
| } |
There was a problem hiding this comment.
Generic catch clause.
This PR adds 2 WinAppSdk1.8 samples to main branch: