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.

Sign up for GitHub

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

Jump to bottom

Added additional documentation and example code snippet. #2156

Merged
merged 9 commits into from May 22, 2021
Merged

Added additional documentation and example code snippet. #2156

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6808a8c
Added additional documentation and example code snippet.
Apr 11, 2021
12d2272
Update dialog/folder.go
lazyhacker Apr 21, 2021
93fa956
Merge pull request #1 from fyne-io/develop
lazyhacker Apr 21, 2021
1d4725e
Remove code snippet per review feedback.
lazyhacker Apr 23, 2021
b142ab7
Clarified wording per review comments.
lazyhacker Apr 24, 2021
9551c9b
Add comment about when the callback function is called and when it gets
lazyhacker Apr 24, 2021
9bf6bca
Added comment on when callback function is called and when it gets a …
lazyhacker Apr 24, 2021
1b461a6
Merge branch 'fyne-io:develop' into develop
lazyhacker May 15, 2021
a86eea6
Changed spacing between sentences to single. Fyne uses the single sp…
lazyhacker May 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 19 additions & 1 deletion dialog/folder.go
View file
Open in desktop
Expand Up @@ -19,9 +19,27 @@ func NewFolderOpen(callback func(fyne.ListableURI, error), parent fyne.Window) *
return dialog
}

// ShowFolderOpen creates and shows a file dialog allowing the user to choose a folder to open.
// ShowFolderOpen creates and shows a file dialog allowing the user to choose a
// folder to open. It takes a callback function that is called whenever the open
// or cancel buttons are tapped.
lazyhacker marked this conversation as resolved.
Show resolved Hide resolved
//
// The dialog will appear over the window specified.
//
// a := app.New()
// w := a.NewWindow("example")
// button := widget.NewButtonWithIcon("", theme.FolderIcon(),
// func() {
// dialog.ShowFolderOpen(
// func (uri fyne.ListableURI, err error) {
// // Cancel will pass a nil value.
// if uri == nil || err != nil {
// return
// }
// fmt.Println("URI", uri)
// }, w)
// }
// )
//
Copy link
Member

Choose a reason for hiding this comment

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

I really appreciate the effort, but you are not supposed to add code examples to the API docs in this way. See this link for an example https://blog.golang.org/examples on how it should be done for interactive examples.

However, we do have the fyne_demo application so I don't know if we really should add this kind of example in (someone else will have to comment on that) because adding it to one function would mean that we open up the possibility of adding to much more functions in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The interactive examples are normally able to be executed as a test and in the browser and I'm not sure how to get something like that to work in this case where it's rendering a GUI dialog box. That's why I just put in in a code snippet as an example similar to what Go's doc do for similar cases (e.g. https://golang.org/pkg/net/).

It's perfectly alright with me if this is not the right fit for the project so please feel free to reject the PR.

If the fyne_demo has the illustrative snippet of code, my suggestion would be to add a direct reference to it somehow in the doc to make it easy to find. A demo app's source code (especially one as complete as the demo) isn't as easily navigable IMHO so help to get to the right part would help.

Copy link
Member

Choose a reason for hiding this comment

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

Fyne code can be rendered in the playground in theory, but there is a 5sec timeout that I am sure means we can’t get the app compiled (you could check out the tools/playground package though).
With WASM aiming for 2.1 this may get much better :)

Copy link
Member

Choose a reason for hiding this comment

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

I am nervous about adding code examples like this (i.e. entire applications) because it includes so much outside the scope of this function. Linking to the fyne_demo project could be a nice way to do it, though those links may go stale as well.
I liked what was done with the clarification in the paragraph of text above the example, but feel that large examples like this would be better placed in the tutorials at https://developer.fyne.io.

// Since: 1.4
func ShowFolderOpen(callback func(fyne.ListableURI, error), parent fyne.Window) {
dialog := NewFolderOpen(callback, parent)
Expand Down