Mobile (Android) save dialog #2077
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andydotxyz
requested changes
Mar 11, 2021
There was a problem hiding this comment.
Thanks for this. I think we need to land the PR to mobile dep first otherwise the generated dex file is rather arbitrary.
Additionally there is a commented out type assertion for the Writer interface inside internal/driver/gomobile that should be enabled to ensure test that this will advertise as writeable.
howeyc
commented
Mar 11, 2021
andydotxyz
requested changes
Mar 12, 2021
| @@ -34,6 +32,19 @@ func (m *mobileFileRepo) CanRead(u fyne.URI) (bool, error) { | |||
| func (m *mobileFileRepo) Destroy(string) { | |||
| } | |||
|
|
|||
| func (m *mobileFileRepo) CanWrite(u fyne.URI) (bool, error) { | |||
| return true, nil // TODO check a file can be read | |||
There was a problem hiding this comment.
I think this is a copy-paste issue in comment
|
The dex here is what I did by taking latest GoNative java and running a manual edited gendex to use that file. I'm not sure how to do using vendor or however it's usually done. |
|
Just run "go mod vendor" and commit the module/vendor changes. |
|
This pull request does have the updated dex.go file as a result of running gendex. You can see it in the change file list. Just an FYI in case you are waiting on me. |
|
Apologies if I’m wrong, but the date stamps make it look like the generated dex was created before mime parameters were added. Does it not need to be updated again? |
|
That's my "fault" for updating the dex.go as I was making changes I guess. I manually generated with up-to-date java file as I was making commits. So for instance you'll see the dex.go file has changes along with the other changes in the mime commit. |
|
Goodness, no, nothing wrong here except I missed that dex change, sorry. Will test again in the morning and we should be good. |
|
Running on my Android phone it seems to crash. Do I need to add anything to the apk for this new code to work? |
|
To actually test the code better in fyne_demo I have pushed an update to For reference the change is this: diff --git a/cmd/fyne_demo/tutorials/dialog.go b/cmd/fyne_demo/tutorials/dialog.go
index 54fd28d1..a829d014 100644
--- a/cmd/fyne_demo/tutorials/dialog.go
+++ b/cmd/fyne_demo/tutorials/dialog.go
@@ -67,7 +67,7 @@ func dialogScreen(win fyne.Window) fyne.CanvasObject {
return
}
- fileSaved(writer)
+ fileSaved(writer, win)
}, win)
}),
widget.NewButton("Folder Open", func() {
@@ -141,13 +141,18 @@ func imageOpened(f fyne.URIReadCloser) {
showImage(f)
}
-func fileSaved(f fyne.URIWriteCloser) {
+func fileSaved(f fyne.URIWriteCloser, w fyne.Window) {
if f == nil {
log.Println("Cancelled")
return
}
- log.Println("Save to...", f.URI())
+ defer f.Close()
+ _, err := f.Write([]byte("Written by Fyne demo\n"))
+ if err != nil {
+ dialog.ShowError(err, w)
+ }
+ log.Println("Saved to...", f.URI())
}
func loadImage(f fyne.URIReadCloser) *canvas.Image { |
|
This probably relates to the URI (I was writing to downloads folder) being a "content://" which I guess may create challenges? |
dex.go was updated on earlier commit, so running gendex.go again does not show a change
|
I think it had something to do with how I was sending bytes through go/c/jni. |
andydotxyz
requested changes
Mar 17, 2021
internal/driver/gomobile/android.c
Outdated
| jmethodID write = find_method(env, streamClass, "write", "([BII)V"); | ||
|
|
||
| jbyteArray data = (*env)->NewByteArray(env, len); | ||
| jboolean isCopy; |
There was a problem hiding this comment.
Unfortunately some of these tabs are spaces, so messes up the indenting
| @@ -17,6 +17,8 @@ github.com/fyne-io/glfw/v3.3/glfw v0.0.0-20201123143003-f2279069162d h1:WfVxpuVm | |||
| github.com/fyne-io/glfw/v3.3/glfw v0.0.0-20201123143003-f2279069162d/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= | |||
| github.com/fyne-io/mobile v0.1.3-0.20210308120140-0925491569e9 h1:VNKrDC+nmsomQ8llShnBd8X8WFAr66eEwJxX5sap1oE= | |||
| github.com/fyne-io/mobile v0.1.3-0.20210308120140-0925491569e9/go.mod h1:/kOrWrZB6sasLbEy2JIvr4arEzQTXBTZGb3Y96yWbHY= | |||
| github.com/fyne-io/mobile v0.1.3-0.20210312180903-f9a21000f5dc h1:A5hFL3tVUfFHhVpjmLGPs8SdVIcSkTRpFMm1Vsio5+k= | |||
There was a problem hiding this comment.
I think this is a duplicate - can you run "go mod tidy" again?
|
One other note, we ask if you can avoid force-pushince once a review has been started so we can avoid re-reviewing each time. Thnks for this work, it's great - just a few minor things to polish it off. |
|
Yeah, I think I didn't do the rebase quite right or something. |
andydotxyz
approved these changes
Mar 17, 2021
I probably should have suggested merging instead of rebasing. Either way this is in now - thanks so much :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Adds file save support for android. The dex file is generated using the GoNativeActivity.java from my pull-request against the 'mobile' repository.
Fixes #2076
I don't have a Mac so I have no idea how IOS would work. There are just empty stubs there.
Checklist:
Where applicable: