-
Notifications
You must be signed in to change notification settings - Fork 329
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
Adding initial spec for Reunion Picker API. #99
Conversation
Is this intending to be a wrap for the existing picker experiences, or will this be a new File/Folder Picking UI and UX which could be built in Xaml, and used by all lifecycles of WinUI and Reunion apps? #Resolved |
This API will reuse some code from the existing experience. This will for the most part be a rewrite and hence will benefit from being built in Xaml. #Resolved |
Great to hear! File Picking dialogs are some of the most commonly seen UI (like the UAC prompt), and having them be modern and reflect the modern Shell and Fluent design will go a long way to making the whole OS feel more consistent. I would like to see most things that are currently possible with the Win32 dialogs carried over, (with the relevant permissions dialogs). The removal of outdated or undesirable extensibility and features should be considered, as Reunion and WinUI 3 are both opt ins, and so there is an expectation of code changes for the most part - but keeping existing code working should be a goal as much as possible. #Resolved |
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.
Some preliminary feedback.
Also, I just wanted to make sure that I can get a filesystem path out of the StorageItem
(at least in a full-trust process) so I can use it with whatever API I need. The UWP storage subsystem is infamously slow and difficult to use (cf. #8). Thanks!
One other thought: I’m torn about the namespace
My personal namespace design principles lean heavily towards option 3. If it was my decision, every Reunion API would have a root namespace something like |
943b5e3
to
cee7d60
Compare
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.
A great start! @ShawnSteele - can you help guide updates?
specs/Picker-API_spec.md
Outdated
@@ -0,0 +1,115 @@ | |||
# Background | |||
|
|||
The current FilePicker API for UWP has several limitations. Some of the top limitations |
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.
How closely should we be following the .NET file open dialog?
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.
Should we encapsulate the core "picking" experience (the list of folders / files) into a stand-alone control so that developers can build their own picker UI for when they need additional features? For example, Notepad customizes the standard Win32 dialog to add an "Encoding" drop-down; Photoshop adds a bunch of advanced options; etc. The control would of course only work within the constraints of whatever permissions the app had (eg, just Pictures library) but relieves the developer of having to do all the messy filesystem work and trying to copy the look and feel of the "real" dialog.
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.
For example, Win32 apps can embed an Explorer control or use the ancient DlgDirListComboBox
function.
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.
Could we start off with enabling the Win32 file/folder picker dialog from UWP and then make improvements to provide a modern UI.
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.
@zenbird the dialog needs to run out-of-proc in the broker in order to access the files and then grant them to the UWP process. So the existing dialogs can be used, but there still needs to be some code in the middle to jump over to the broker and back again. #Resolved
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.
Should we encapsulate the core "picking" experience (the list of folders / files) into a stand-alone control so that developers can build their own picker UI for when they need additional features? For example, Notepad customizes the standard Win32 dialog to add an "Encoding" drop-down; Photoshop adds a bunch of advanced options; etc. The control would of course only work within the constraints of whatever permissions the app had (eg, just Pictures library) but relieves the developer of having to do all the messy filesystem work and trying to copy the look and feel of the "real" dialog.
The existing File Picker UIs will co-exist with the Reunion ones - so I don't think there is a huge need to support rendering of GDI elements.
Any new WinUI XAML based dialog, could support custom content, and the UIs get built with WinUI.
In fact I proposed that the main content region could display XAML content, as well as HTML, with OneDrive, Google Drive and Photos as examples.
The Photos example, being a WinUI app, could provide a picker UI that renders in the dialog.
As for the customisations made by Notepad and Adobe - I believe these use existing Win32 APIs for adding controls. So at least in the mockup designs I have worked on, I visualised these as a collapsible panel containing WinUI controls.
microsoft/microsoft-ui-xaml#2854
Then there is the question of whether this API makes its way to Xbox, Surface Hub and Hololens. I would initially suggest that the APIs should match all platforms, but with a different UI/UX to suit the platform - and any overrides such as file system restrictions
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.
Correct. V0.5 of the picker will use the existing win32 dialogs until we figure out the details of the intermediate process that will "broker" the permissions.
In reply to: 488307896 [](ancestors = 488307896)
specs/Picker-API_spec.md
Outdated
|
||
The goal of this API is to address the gaps in the existing File/Folder picker APIs. The API surface is similar to the existing | ||
Picker APIs in Windows SDK with additional methods to support File + Folder picker and multiple folder picker. The API removes the | ||
deprecated methods from Windows SDK. |
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.
When returned to an app, I assume the app has all the same access to these objects as if they'd been picked by the platform API instead? Future access, available through CreateFileFromApp
/FindFirstFileExFromApp
, etc. #Resolved
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.
That's not the case now for the pickers and the behavior should remain the same. E.g. The items returned from the picker are available temporarily and are not available by PATH or any other means unless added to the MRU or FutureAccessList. In particular the implementation must NOT internally add items to either list as the contents of those lists are owned by the application - not the system. Though we may want to take the opportunity here to include the user intent that access is supposed to only be one time to and block adding the item to the MRU/FAL @ptorr-msft may have some thoughts on that as he's talked about that before. #Resolved
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.
Though we may want to take the opportunity here to include the user intent that access is supposed to only be one time to and block adding the item to the MRU/FAL
This in turn begs the question: If you are blocked from adding a path selected via the picker to the MRU/FAL, then how are you supposed to populate those lists in the first place? (Especially in an AppContainer app, where the only way to get access to arbitrary paths is to use the picker.) This is a bizarre limitation, especially since the platform pickers don’t do this. If we go through with this idea, then apps will lose functionality by moving to Reunion. This does not sound like a good idea 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.
Since we're blue-skying here... the ultimate goal is for the duration of the consent to be specified by the user -- "Until I close the app" or "Always" or "Only when the app is in the foreground" etc. -- and also the scope of the grant (read-only vs. read/write) to be under their control. The apps should not be required to use FAL in order to maintain access once we do this; it should "just work."
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.
I agree. The picker should return a vector if IStorageItem with no guarantees on the underlying object.
In reply to: 458438812 [](ancestors = 458438812)
Will this specification include the notion of WinUI/UWP apps acting as file providers, rendering UI within this new File Dialog? The conversation here microsoft/microsoft-ui-xaml#2854 brought up apps like Camera which can appear within the Win32 file dialog, but which do not work very well today. #Resolved |
specs/Picker-API_spec.md
Outdated
|
||
The goal of this API is to address the gaps in the existing File/Folder picker APIs. The API surface is similar to the existing | ||
Picker APIs in Windows SDK with additional methods to support File + Folder picker and multiple folder picker. The API removes the | ||
deprecated methods from Windows SDK. |
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.
That's not the case now for the pickers and the behavior should remain the same. E.g. The items returned from the picker are available temporarily and are not available by PATH or any other means unless added to the MRU or FutureAccessList. In particular the implementation must NOT internally add items to either list as the contents of those lists are owned by the application - not the system. Though we may want to take the opportunity here to include the user intent that access is supposed to only be one time to and block adding the item to the MRU/FAL @ptorr-msft may have some thoughts on that as he's talked about that before. #Resolved
String CancelButtonText { set; } | ||
Windows.Foundation.Collections.IMap<String, String> FilterAndDescription{ get; } | ||
PickerViewMode ViewMode; | ||
StorageItemPickerKinds PickerKinds; |
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.
PickerKinds [](start = 35, length = 11)
ItemKinds #Resolved
We were discussing implementing those custom items with a future File Picker Dialog. |
6a47b62
to
4eafb66
Compare
Currently this is not in scope. We can look to address this as part of future work. In reply to: 659771383 [](ancestors = 659771383) |
Can we consider the case for using the |
so this is opt-in for apps? :( #Resolved this really shouldn't be up to apps to choose. >:( #Resolved |
This is not going to replace the existing FilePicker api. Apps can choose to use this API instead of the existing FilePicker. In reply to: 700438667 [](ancestors = 700438667) |
Will this API apply to platforms like Xbox where file storage is handled differently? |
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.
Next up, @ShawnSteele
specs/Picker-API_spec.md
Outdated
|
||
**3. Unable to specify an arbitrary start location for the picker.** | ||
|
||
The existing File/Folder pickers do not allow the developer to specify an arbitrary start location (even if the app has access). Currently, the start location is limited to a set of predefined enum values. |
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 line-wrap at 100 columns. VSCode has various Prettifier plugins that'll do that for you. #Resolved
specs/Picker-API_spec.md
Outdated
APIs in the Windows SDK. The Reunion Picker API will supercede the existing APIs and add additional methods | ||
for the functionality: File and folder picker, Multiple folder picker. | ||
|
||
- Windows.Storage.Pickers.FileOpenPicker (https://docs.microsoft.com/en-us/uwp/api/Windows.Storage.Pickers.FileOpenPicker) |
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.
Make these real links, like [nice text](http://link/here)
#Resolved
if(0 < storageItems.Count) | ||
{ | ||
} | ||
``` |
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.
Markdown usually likes additional blank lines after headers and triple-ticks. If it's rendering don't worry about it. #Resolved
specs/Picker-API_spec.md
Outdated
```c# | ||
var picker = new StorageItemPicker(); | ||
picker.FiltersAndDescriptions.Insert("Images", "*.jpg, *.png"); | ||
picker.StartLocation = startLocation; |
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.
Expand sample to show the type of startLocation
#Resolved
specs/Picker-API_spec.md
Outdated
Picker APIs in Windows SDK with additional methods to support File + Folder picker and multiple folder picker. The Project Reunion APIs will supercede the Windows APIs. | ||
|
||
# Examples | ||
## Show file and folder picker |
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.
More descriptive title - "Pick specific images or a folder to open" #Resolved
specs/Picker-API_spec.md
Outdated
String CommitButtonText; | ||
String CancelButtonText; | ||
Windows.Foundation.Collections.IPropertySet FiltersAndDescriptions{ get; }; | ||
ViewMode PickerViewMode; |
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.
Should be PickerViewMode DefaultViewMode;
instead ... the user can change the view mode during the pick operation, so either Default...
or Initial...
to say that it's the app's desired initial state, but not a requirement. #Resolved
specs/Picker-API_spec.md
Outdated
# API Notes | ||
# API Details | ||
```c# | ||
namespace Microsoft.Reunion.Storage.Pickers |
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.
Not sure this is the right namespace yet. #Resolved
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.
Discussions to date have agreed namespaces shouldn't include 'Reunion' (or ProjectReunion) in their name, and recommended variants of existing APIs should use variants of existing namespaces but Microsoft. instead of Windows. e.g. Microsoft.Storage.Pickers
@jonwis docs\Coding-Guidelines.md makes no mention of namespace guidance. We should add it (in a new WinRT section)? #Resolved
specs/Picker-API_spec.md
Outdated
```c# | ||
namespace Microsoft.Reunion.Storage.Pickers | ||
{ | ||
enum PickerViewMode |
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.
Should probably be StorageItemPickerViewMode
? #Resolved
specs/Picker-API_spec.md
Outdated
String SuggestedSaveFile; | ||
String DefaultFileExtension; | ||
Boolean MultiSelect; | ||
Int64 Result{ get; } |
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.
What is this? #Resolved
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.
This holds the dialog result(Canceled, Access_Denied etc.). This can be removed in favor of packaging the result with IAsyncOperation
In reply to: 499730295 [](ancestors = 499730295)
specs/Picker-API_spec.md
Outdated
StorageItemPicker(); | ||
|
||
/// Properties | ||
String StartLocation; |
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.
This should probably be StartLocationFolderPath, since it's a path. What happens if the app does not have access to this path? Is there a prompt for the user to approve picking from here to start with? Or is the act of picking implicit approval? #Resolved
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.
IMO, this API app should accept an arbitrary folder path when running in an AppContainer, but not allow any other operations on that folder or its contents except through the usual methods.
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.
Can I specify a StartLocation via a StorageFolder e.g.
var sip = new StorageItemPicker();
sip.Start = Windows.Storage.KnownFolders.PicturesLibrary;
I can see merit in both
String StartFolderPath;
StorageFolder StartFolderLocation;
and .ShowPickerAsync()
throws an exception if both are specified
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.
Woudn't you be able to just use the Path API on the StorageFolder? E.g.:
var sip = new StorageItemPicker();
sip.StartFolderPath = Windows.Storage.KnownFolders.PicturesLibrary.Path;
The documentation for Path states "The full path of the current folder in the file system, if the path is available", so perhaps that's a scenario which would require one can specify a StorageFolder as the StartLocation (no full path available)?
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.
StorageFolders
that represent libraries like the PicturesLibrary
don't necessarily have paths, since they are shell constructs and not filesystem directories. You could get the 'best' directory from SHGetKnownFolderPath
though (.NET has an equivalent).
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.
Yes, as I have mentioned in the original thead here, using the .NET equivalent could look like:
var sip = new StorageItemPicker()
{
StartFolderPath = Environment.GetFolderPath(Environment.SpecialFolder.MyPictures)
};
so at least in that case, we would be fine without shipping a specific StorageFolder StartFolderLocation
API.
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.
IMO, this API app should accept an arbitrary folder path when running in an AppContainer, but not allow any other operations on that folder or its contents except through the usual methods.
Yes, the point of the picker is to get files you don't have access to. If the user doesn't like the location, they can change it.
Relying on a single, fixed list of locations (current Windows.
API) or relying on the app already having access vastly reduces the functionality. For example, if I already have access to C:\Foo\bar.txt
I might want the user to pick another file from C:\Foo\
even though I don't have access. (I believe the current API remembers the last start location, but that doesn't help if the user picked another file in the meantime...)/
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.
The new pickers should be easy to nest into user controls or dialogs, and should allow simple access from MVVM viewmodels. Don't repeat WPF's mistakes.
This can be enabled via a property similar to Win32 IFileDialog interface. In reply to: 698086497 [](ancestors = 698086497) |
specs/Picker-API_spec.md
Outdated
Windows.Foundation.Collections.IPropertySet FiltersAndDescriptions{ get; }; | ||
ViewMode PickerViewMode; | ||
StorageItemPickerKinds PickerKinds; | ||
String SuggestedSaveFile; |
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.
Suggested
? Do we use that terminology? Default
seems more natural
File
sounds like an object, but this is just a filename. String DefaultSaveFilename
? #Resolved
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.
Suggested
mirrors the SuggestedStartLocation
, although most WinRT APIs use the term Prefer
or Request
when the app is giving a hint that the platform can ignore. .NET just calls it FileName
(note the different capitalization as well... which is quite dated since now the world recognizes "filename" as a single word in the English language).
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.
specs/Picker-API_spec.md
Outdated
picker.FiltersAndDescriptions.Insert("Images", "*.jpg, *.png"); | ||
picker.StartLocation = startLocation; | ||
picker.PickerKinds = StorageItemPickerKinds.Folder | StorageItemPickerKinds.FileOpen; | ||
// multi-select will be automatically set |
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.
Multi-select is the default? Is that what applications expect? It defaults to false
in the .NET pickers. #Resolved
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.
MultiSelect is being automatically set since the PickerKind is both files and folders. Multiselect is false by default.
In reply to: 500443235 [](ancestors = 500443235)
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.
That doesn't logically follow though... it's legitimate to want to pick a single file or a single folder. Also, having one property change value based on another property's value is against guidelines. For example, the following two bits of code should be equivalent:
picker.PickerKinds = Folder | FileOpen;
picker.Multiselect = false;
and
picker.Multiselect = false;
picker.PickerKinds = Folder | FileOpen;
But they are not equivalent; the first one works as expected, but the second one does not. #Resolved
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.
Updated to false by default. No automatic setting of the property.
In reply to: 500446491 [](ancestors = 500446491,500443235)
a6aed2f
to
0b86e7d
Compare
specs/Picker-API_spec.md
Outdated
} | ||
``` | ||
|
||
## Pick folders |
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.
s [](start = 14, length = 1)
Pick folder no "s" at the end. Picking a single folder. #Resolved
Adding StorageItemPickerResult to return the result of the operation. Added more code to the samples. Addressed feedback comments
@zenbird what happened to this PR? |
Yea, what is the status of the updated XAML based File Pickers? |
This is the high level specification for the File/Folder picker in Reunion. This initial draft includes the API of the proposed File/Folder picker.