-
-
Notifications
You must be signed in to change notification settings - Fork 442
[06x] UX: Rewrite FileDialog directory checks #3670
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
[06x] UX: Rewrite FileDialog directory checks #3670
Conversation
Previously, we assumed that Eto.EtoEnvironment.GetFolderPath would never return an empty string. It is however apparent that this is actually the case on some systems, seen on both Windows and Linux. Constructing URI's with an empty string throws an exception, which this commit attempts to avoid. It was not an option to simply change the Uri constructor, as Directory can only be set if it is a valid Uri, so instead the Save/OpenFileDialog Eto constructor has been moved to an extension method internal to the project.
|
Looks good. Has this been tested by users that had this problem? |
|
Seems like you could jus re-use OpenFileDialog for SaveFileDialog provided you offer another param for the title public static FileDialog OpenFileDialog(string title, string directory, FileFilter[] filters, string defaultTitle, bool multiSelect = false, bool saving = false)
{
var fileDialog = saving ? new SaveFileDialog() : new OpenFileDialog();
SetupFileDialog(fileDialog, title, directory, filters, defaultTitle);
fileDialog.MultiSelect = multiSelect;
return fileDialog;
}Also, i don't see the point of another title parameter if it's set everywhere. public static void AddFilterRange(this FileDialog fileDialog, FileFilter[] filters)
{
if (filters == null)
foreach (var filter in filters)
fileDialog.Filters.Add(filter);
}
//public static FileDialog OpenFileDialog<T>(string title, string directory, FileFilter[] filters, bool multiSelect = false)
//where T : FileDialog, new()
public static FileDialog OpenFileDialog(string title, string directory, FileFilter[] filters, bool multiSelect = false, bool saving = false)
{
// var fileDialog = new T();
var fileDialog = saving ? new SaveFileDialog() : new OpenFileDialog();
fileDialog.AddFilterRange(filters);
if (!string.IsNullOrEmpty(title))
fileDialog.Title = title;
if (!string.IsNullOrEmpty(directory))
fileDialog.Directory = new Uri(directory);
fileDialog.MultiSelect = multiSelect;
return fileDialog;
}also where is the actual opening part? return a bool instead by calling |
Not sure if I follow. I still want to return the correct type - is this possible without making a template class? Your changes return a |
|
Both inherit from |
Has been tested now: discord verification Will refactor this to use a template as discussed in DM's with Cubix. |
jamesbt365
left a comment
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.
Approving with good faith that this works, code looks fine at a glance.
|
Pretty sure we should be good to merge, absolutely worst case it's buggy and it'll be fixed/reverted before release anyway. |
|
Removing forward port tag because |
Fixes #3668 for 0.6.x
Previously, we assumed that
Eto.EtoEnvironment.GetFolderPathwould never return an empty string. It is however apparent that this is actually the case on some systems, seen on both Windows and Linux.Constructing URI's with an empty string throws an exception, which this commit attempts to avoid. It was not an option to simply wrap the
Uriconstructor, asDirectoryinSaveFileDialog/OpenFileDialogmust only be set if it is a valid Uri, so instead theSaveFileDialog/OpenFileDialogEto constructor has been moved to a wrapper method internal to the project.