-
Notifications
You must be signed in to change notification settings - Fork 351
Public Spec for Microsoft.Windows.Storage.Pickers #5155
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
Conversation
Is the general principle in WindowsAppSDK to continue using the StorageFile and StorageFolder types? I mean if writing in C#, using the .Net IO functions is more convenient, in which case it's preferable just to have file paths and not have the overhead of the StorageFile/Folder instances. However, I have to admit, I'm not full up to date with all the possible security models that can be used with WindowsAppSDK, so maybe there's a reason for using StorageFile and StorageFolder instead of just returning the file path? |
a new picker API that must have these two bare minimum features:
EDIT: longstanding issues that will be finally resolved : #88, #2504, #3536, microsoft/microsoft-ui-xaml#9557 |
Hello @benstevens48 , The new API returns At the current stage, once we have the StorageFile, we can use StorageFile.Path for subsequent operations (as demonstrated in the examples). In future, we may consider adding more methods and return types. |
@DinahK-2SO Thanks for the reply. That makes sense. I'm just concerned about the ongoing usage of StorageFile in WIndowsAppSDK since it always had very poor performance in UWP (I mean just creating 1 StorageFile could consume 100kB of RAM and loading many StorageFiles was hundreds of times slower than using Win32/.Net APIs). Although I haven't tested in in WIndowsAppSDK , so maybe it behaves differently. Probably in a file picker it doesn't matter too much, although in some cases the user may pick 1000s of files (like all photos in a large folder of photos etc), so it could start to become an issue. |
Can you provide a variant which returns the file as a string e.g. Properly implementing that would require more than skin deep differences but ahort term that could be implemented as
with the deeper implementation optimized to return the string w/o any |
@DinahK-2SO Performance can be a concern if you have the path/file as a string. The cost to create a StorageFile/Folder object can be non-trivial if your sole purpose is to rip the string out of the returned object and free the object. Is the sole purpose of this spec is to provide WinAppSDK alternatives supporting elevation to Windows.* APIs that don't? Seems it's maybe that and less given what's spec'd is a subset of Windows.* pickers (no PickMultipleFilesAsync, CreateForUser, etc). It's hard assess the spec without an understanding of the desired endgame. Is that longer-term clear to you but intentionally deferred for now, 'cause, reasons? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- specs/StoragePickers/FileSavePicker.md: Evaluated as low risk
Comments suppressed due to low confidence (1)
specs/StoragePickers/FileOpenPicker.md:65
- The method name
PickSingleFilesAsync
is incorrect. It should be renamed toPickSingleFileAsync
.
## FileOpenPicker.PickSingleFilesAsync
da1e3f7
to
6e3e097
Compare
6e3e097
to
cb97ad7
Compare
Excuse me for sending messages among the elders. But can you clarify and consider the following questions/requests? 1.Why you are using WindowId instead of hwnd? later you get hwnd like this:
So why don't you get hwnd directly? 2.Why are there different properties for extensions? FileOpenPicker-> FileTypeFilter Why not use FileTypeChoices for both? 3.When we use Windows.Storage.Picker, we must set FileTypeFilter or FileTypeChoices, otherwise picker will not run. and get exception:
I hope the new picker doesn't have this problem. 4.We can set details for extension in FileSavePicker like this:
but we can not add details and multiple extensions for FileOpenPicker:
|
Hi @ghost1372 , thanks for raising these questions and sharing your insights! This is a long answer list following a long question list, so I'll address each part separately.
The reason for using using Windows.Storage.Pickers;
var hwnd = WinRT.Interop.WindowNative.GetWindowHandle(this);
var picker = new FileOpenPicker();
WinRT.Interop.InitializeWithWindow.Initialize(picker, hwnd);
var file = await picker.PickerSingleFileAsync();
string path = file.Path; In the new using Microsoft.Windows.Storage.Pickers;
var picker = new FileOpenPicker(this.AppWindow.Id);
var result = await picker.PickerSingleFileAsync();
string path = result.Path; We can see the new approach reduces more boilerplate code and makes the API easier to use. By the way, you might be referring an old commit. If you're looking for the progress on coding, kindly follow our dev branch: user/xianghong/storage-pickers-develop
Also for the question 4: It's an inspiring request to add
Thanks for bringing up this topic! First, the answer for For the When the file picked by the end user doesn't exist,
Please refer to the previous answer 2. |
Thank you @DinahK-2SO |
Iteration 5 looks good to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces the public specification for the Microsoft.Windows.Storage.Pickers API and adds detailed markdown documents for the new classes and enums supporting file and folder picking in elevated desktop applications.
- Added a comprehensive overview and definition of the API in Microsoft.Windows.Storage.Pickers.md
- Introduced individual spec documents for FileOpenPicker, FileSavePicker, FolderPicker, and their associated result classes
- Defined enum specifications for PickerViewMode and PickerLocationId with background context and examples
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
specs/StoragePickers/Microsoft.Windows.Storage.Pickers.md | New API overview and definition of core classes and enums |
specs/StoragePickers/PickerViewMode.md | Specification and examples for PickerViewMode enum |
specs/StoragePickers/FileOpenPicker.md | Detailed class specification with code examples for FileOpenPicker |
specs/StoragePickers/PickerLocationId.md | Specification and usage context for PickerLocationId enum |
specs/StoragePickers/FolderPicker.md | Detailed class specification with code examples for FolderPicker |
specs/StoragePickers/PickFolderResult.md | Specification for PickFolderResult class and conversion examples |
specs/StoragePickers/FileSavePicker.md | Detailed class specification with code examples for FileSavePicker |
specs/StoragePickers/PickFileResult.md | Specification for PickFileResult class including usage examples |
Comments suppressed due to low confidence (1)
specs/StoragePickers/PickFileResult.md:21
- Typo detected in the header: "compatability" should be corrected to "compatibility".
## Backward compatability
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
/azp run |
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
/azp run |
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please clarify do we need to indicate it is for "experimental only" or not need.
Also, for Microsoft doc, do we need to do it ourselves or not? I assume the doc reviewer is need.
@DinahK-2SO clarify in the docs if it works within AppContainers + win32-app-isolation or not? |
After going through this and the testing with experimental. It appears that this is only for WinUI 3 in elevated Admin privelages. It not only fails in LowIL it also returns a path, which is also not allowed in LowIL. In trying to move from UWP I need to have LowIL because Windows.Storage.Search performance is 20x slower in MediumIL - Is this something that might be fixed, or should I just stay on UWP ? |
The public Spec for
Microsoft.Windows.Storage.Pickers
Preview: Microsoft.Windows.Storage.Pickers.md
The code is also open for review. Find PR: #5240