Skip to content

One page design for FileSavePicker.SuggestedSaveFilePath #5538

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

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

Conversation

DinahK-2SO
Copy link
Contributor

@DinahK-2SO DinahK-2SO commented Jun 18, 2025

One page design for FileSavePicker.SuggestedSaveFilePath

Code implementation: #5547

@DinahK-2SO DinahK-2SO marked this pull request as ready for review June 19, 2025 03:21
@benstevens48
Copy link

Why have a suggested file instead of folder? What if we want the initial file name to be blank, in case there is no good default name or we want to force the user to choose a name?

Copy link
Collaborator

@codendone codendone left a comment

Choose a reason for hiding this comment

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

Looks good to me. Per @benstevens48's question, if SuggestedSaveFile only points at a folder instead of a file in a folder, will that have the effect of setting the start folder with no default filename? Or is it required to include a suggested filename?

* **Pre-filled File Name:** The file name field in the dialog is pre-populated with the name of
the `SuggestedSaveFile`.

This takes precedence over the `FileSavePicker.SuggestedFileName` property if both are set.
Copy link
Contributor Author

@DinahK-2SO DinahK-2SO Jun 25, 2025

Choose a reason for hiding this comment

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

Thanks @codendone for reviewing both the spec and code! Here we explained that the Path of SuggestedSaveFile should take precedence over the SuggestedFileName #Closed

@DinahK-2SO
Copy link
Contributor Author

DinahK-2SO commented Jul 3, 2025

Looks good to me. Per @benstevens48's question, if SuggestedSaveFile only points at a folder instead of a file in a folder, will that have the effect of setting the start folder with no default filename? Or is it required to include a suggested filename?

Thanks @codendone for reviewing this design!

First, The SuggestedSaveFile of UWP FileSavePicker doesn't accept a StorageFolder object - the compiler would show this error

Cannot implicitly convert type 'Windows.Storage.StorageFolder' to 'Windows.Storage.StorageFile'

image

In the scope of our new APIs, I would strongly recommend the logic of TrySuggestedSaveFilePath to be simple as below:

TrySuggestedSaveFilePath takes whatever after the last slash to fill in the file name box,
as long as the part before last slash is a valid and existing folder.
And return false without changing any value if the input doesn't even contain a slash, or the folder doesn't exist.
Test cases here for clarification, will update the content in this design next.

The input would be largely depends on API consumer, but above logic is concrete - The method itself doesn't decide if the input path is a file path or folder path.

This is to reduce the complexity and avoid more undefined behaviors (e.g. should .git in C:\temp\MyProject\.git be treated as a filename or a folder name? what if the input string is C:\temp\1.txt but there's folder 1.txt under C:\temp? etc. )

For the questioned scenario, we can add another method in future, to set the suggested folder without file name, e.g. introducing a SuggestedFolder attribute and TrySetSuggestedFolder, and look for the folder of input path string, no matter the format.

@codendone
Copy link
Collaborator

TrySuggestedSaveFilePath takes whatever after the last slash to fill in the file name box, as long as the part before last slash is a valid and existing folder. And return false without changing any value if the input doesn't even contain a slash, or the folder doesn't exist.

I agree with that design.

This is to reduce the complexity and avoid more undefined behaviors (e.g. should .git in C:\temp\MyProject\.git be treated as a filename or a folder name? what if the input string is C:\temp\1.txt but there's folder 1.txt under C:\temp? etc. )

This relates to the question "if SuggestedSaveFile only points at a folder instead of a file in a folder, will that have the effect of setting the start folder with no default filename? Or is it required to include a suggested filename?".

From the implementation, I believe the current design is if C:\temp\MyProject\.git\ is specified with the trailing \, then a folder is specified and will be used (assuming it exists) and no filename will be set (allowing SuggestedSaveFile to be used, if that is set). I think that design is good.

But, as you said, what happens if C:\temp\MyProject\.git is set? I think the current implementation will use .git as the default filename even if that exists as a folder. That seems wrong. It may be necessary to check if the full specified path points to a folder and treat it as a folder with no filename.

Splitting out a separate SuggestedFolder attribute and TrySetSuggestedFolder seems unnecessary (and potentially confusing) if the handling of SuggestedSaveFilePath simply does that extra "the path is just a folder" check.

@DinahK-2SO
Copy link
Contributor Author

DinahK-2SO commented Jul 4, 2025

But, as you said, what happens if C:\temp\MyProject\.git is set? I think the current implementation will use .git as the default filename even if that exists as a folder. That seems wrong. It may be necessary to check if the full specified path points to a folder and treat it as a folder with no filename.

Thanks @codendone for raising this point! I also discussed this topic with @yeelam-gordon yesterday, and we concluded the current design could be more beneficial.

It's great to know that we have reached consensus on the first case:

  • C:\temp\MyProject\.git\ sets folder C:\temp\MyProject\.git, with empty file name;

The question is the second case:

  • C:\temp\MyProject\.git sets folder C:\temp\MyProject, with .git filled into file name, even when there's a folder C:\temp\MyProject\.git.

While it might seem counter-intuitive at first, this behavior is based on a very strict logic. It makes the API's behavior 100% predictable and gives developers clearer control over the code.

  • Via TrySetSuggestedSaveFilePath, the developer attempts to pass 2 suggestions to the end user: a location and a file name.

    With the current design, if the folder C:\temp\MyProject\.git exists, end user can see:

    1. the save dialog opens C:\temp\MyProject (passing the 1st suggestion).
    2. it is also suggested to save the new file as .git (the 2nd suggestion), although they cannot save as there's already a folder named .git

    In this case, the end user can decide:

    1. rename the folder .git to another name and save file
    2. change the new file's name
    3. or choose any other location with any other file name

    All based on 2 suggestions from the developer.

  • In contrast, if we check the folder existence of the input string and jump into the deeper folder, the API's behavior would become more complex and unpredictable. In above example, end user would see:

    1. the save file dialog opens C:\temp\MyProject\.git, which was not the intentioned folder of developer,
    2. no suggested file name - another information failed to pass to the end user.

    This would be like a second-guessing the developer's intent based on the unpredictable state of the end-user's file system, bringing an unreliable API's behavior

@DinahK-2SO DinahK-2SO changed the title One page design for FileSavePicker.SuggestedSaveFile One page design for FileSavePicker.SuggestedSaveFilePath Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants