Skip to content
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

Add Directory and File Picker #3

Closed
wants to merge 14 commits into from

Conversation

ryanleecode
Copy link
Contributor

This adds a directory and file picker pluigin for desktop apps. Partially implements this issue go-flutter-desktop/go-flutter#64.

Tests for this plugin are provided and they include error cases. However, when I try to use the plugin intentionally providing an error case, it implodes. See go-flutter-desktop/go-flutter#132. Regular usage does work though.

@ryanleecode ryanleecode changed the title Directory and File Picker Add Directory and File Picker May 12, 2019
@GeertJohan
Copy link
Member

Thanks for posting this PR, I will look at it soon!

@GeertJohan GeertJohan self-assigned this May 12, 2019
Copy link
Member

@GeertJohan GeertJohan left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! I reviewed the code and left some comments to work with.
I haven't tested yet because I cannot seem to find the flutter plugin that accompanies this host-side implementation. Can you please link the correct Flutter/Dart plugin to use with this?

file_picker/.gitignore Outdated Show resolved Hide resolved
file_picker/README.md Outdated Show resolved Hide resolved
file_picker/README.md Outdated Show resolved Hide resolved
file_picker/go.mod Outdated Show resolved Hide resolved
file_picker/README.md Outdated Show resolved Hide resolved
file_picker/dialogs.go Outdated Show resolved Hide resolved
file_picker/plugin.go Outdated Show resolved Hide resolved
return err
}

dialogProvider := dialogProvider{}
Copy link
Member

Choose a reason for hiding this comment

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

The identifier shadows the type name.

file_picker/plugin_test.go Outdated Show resolved Hide resolved
@@ -10,6 +10,7 @@ Please report issues at the [go-flutter issue tracker](https://github.com/go-flu

- [shared_preferences](shared_preferences) - Provides a persistent store for simple data. ([flutter package](https://pub.dartlang.org/packages/shared_preferences))
- [path_provider](path_provider) - Finding commonly used locations on the filesystem. ([flutter package](https://pub.dartlang.org/packages/path_provider))
- [file_picker](file_picker) - Opens a dialog to pick file descriptors on the filesystem.
Copy link
Member

Choose a reason for hiding this comment

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

Which Flutter plugin does this plugin link to?

@ryanleecode
Copy link
Contributor Author

Theres no underlying flutter plugin. I just used the library from this issue. This means it wouldn't be supported for mobile devices.

@GeertJohan
Copy link
Member

Ahh, it would be great to have compatibility with the most-used plugin for Flutter. For example https://pub.dev/packages/file_picker

 - Renames title from dialog_picker to file_picker as it is the name of package
- Corrects the go import statement to the correct one.
- Removes the parameters on the FilePickerPlugin
- They aren't use by the plugin and are not mandatory to be set
@ryanleecode
Copy link
Contributor Author

ryanleecode commented May 14, 2019

How excatly would I link the mobile plugin with my desktop plugin? To use my plugin currently, I'm just calling invokeMethod directly on the method channel. The mobile plugin has its own methods like getTemporaryDirectory and getApplicationDocumentsDirectory, how do I make it so that when those methods are called it will use my desktop plugin?

Edit: I'm going to try just making it and see if it works like magic.

@GeertJohan
Copy link
Member

GeertJohan commented May 14, 2019

@drdgvhbh Check out the dart side of the file_picker plugin:
https://github.com/miguelpruivo/plugins_flutter_file_picker/blob/master/lib/file_picker.dart

It shows you what method calls are being done.
Just map those calls to this code, then it will work. It seems to use the Standard Method codec with standard messages, not json message. Also make sure to change the channel name to be compatible (just file_picker instead of plugins.flutter.io/file_picker).

@ryanleecode
Copy link
Contributor Author

Ahh that makes sense. So the file_picker plugin only seems to support choosing files and not directories. Should i modify this plugin to only pick files and make a separate one for directories while finding a corresponding mobile plugin (if it exists) for it?

@GeertJohan
Copy link
Member

For now that makes more sense yes. I'd like to add plugins to this repository that mirror popular existing plugins. If there's no plugin to pick files yet, then perhaps that feature shouldn't be in this repository. You could ofcourse open an issue with the maintainer of the original file_picker, to see if adding directory selection may be interesting for them.

@chunhunghan
Copy link
Contributor

chunhunghan commented Jun 25, 2019

I have added suggestion to file_picker plug-in
miguelpruivo/flutter_file_picker#99

@miguelpruivo
Copy link

It wouldn't be a bad idea to have it centralized in one place. Currently I've planned to add support for Desktop and Web for the file_picker, so the users can have a seamless experience. Feel free to PR if you implement it before me.

Thank you.

@GeertJohan
Copy link
Member

GeertJohan commented Jul 2, 2019

This could be a nice plugin on it's own. I think the plugins repository should mirror the "first class" plugins for flutter. (https://github.com/flutter/plugins/) I have recently added support for flutters

I have added initial support for flutter's image_picker, it can still be extended further and help is welcome on that front as well.

image_picker doesn't do everything you'd expect from a desktop file picker. So the plugin in this PR and what @chunhunghan is working in could still be very useful. I would like to suggest creating a separate repository for that. When that plugin becomes stable and also works as a dart plugin, it can be listed in the list.md file (which will later be integrated with hover tooling and perhaps a directory on hover.build to provide automatic compatibility listing)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants