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

fix(image-loader): use tempDirectory on iOS #39

Merged
merged 3 commits into from
Apr 18, 2017
Merged

fix(image-loader): use tempDirectory on iOS #39

merged 3 commits into from
Apr 18, 2017

Conversation

swiftyone
Copy link
Contributor

The tempDirectory can be accessed from the WKWebView as well; no need
for base64!

The tempDirectory can be accessed from the WKWebView as well; no need
for base64!
@swiftyone
Copy link
Contributor Author

As the file reader is not used any more it should also fix issues #37 and #31

@ihadeed
Copy link
Member

ihadeed commented Apr 13, 2017

According to the File plugin documentation, the tempDirectory is meant for temporary files that might not persist across app restarts. I haven't played around with the tempDirectory enough to figure out how it works. Did you experiment with it to see if the cache persists over a few sessions at least?

What if we add an option to customize the iOS behavior, to make sure everybody is happy. Available options can be:

  • base64: returns a base64 image
  • tempDir: implements your solution
  • disable disable the caching completely and return the original URL

Not sure how the new Ionic WKWebView fork updates work, but I think they serve files via http:// protocol instead of file:///.

And I think the Base64 option will be helpful to develop with ionic run <platform> --livereload, so it might be a good idea to keep the functionality around.

@swiftyone
Copy link
Contributor Author

In my experience the tempDirectory is usually persistent, I guess it is only removed if the device runs out of available space.
But I agree, it could be a good idea to keep the base64 option, even though I cannot recommend using it on a device, at least not in production.

I will implement the option next week!
Cheers!

@ihadeed
Copy link
Member

ihadeed commented Apr 14, 2017

Thanks @swiftyone

Lorenz an Mey added 2 commits April 18, 2017 11:21
If 'uri' is used the files will be copied to the temporary directory if
the WKWebView engine is used. Files in this directory are not
persistent, but accessible from within the WebView.
If the files do not exist any longer they will automatically be copied
again!
@swiftyone
Copy link
Contributor Author

swiftyone commented Apr 18, 2017

Alright, have a look at it!
Now all the files are copied to the temporary directory, if needed. This way there should be no problems with the persistence of the directory…
The usage of the temporary directory is automatically chosen if the device uses the WKWebView and the option is set to 'uri', if the user does not want that approach 'base64' needs to be used – otherwise it will not work anyhow.

Also I did use readAsDataURL for the base64 option to avoid directly using the FileReader, which can lead to some problems (#31).

I did not(!) include the 'disable' option, as I was not completely sure, if this is really necessary; I don't really see the benefit of it.

@ihadeed ihadeed merged commit f29d630 into zyra:master Apr 18, 2017
@ihadeed
Copy link
Member

ihadeed commented Apr 18, 2017

Looks good, thanks @swiftyone !

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

Successfully merging this pull request may close these issues.

2 participants