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

Updating the .image property of a LightboxImage instance after Lightbox has downloaded an image via URL #115

Closed
wants to merge 13 commits into from

Conversation

vitalii-tym
Copy link
Contributor

@vitalii-tym vitalii-tym commented Jun 19, 2017

This change allows apps to access the images inside LightboxImage even if those images were downloaded from an external URL provided by the app.
Can be useful, for example, when an app needs to provide sharing functionality.
See example of such a usage in #98

…e fetched by the Lightbox itself via given URL. This will enable apps to be able to access and use images, downloaded by Lightbox.
@vitalii-tym vitalii-tym mentioned this pull request Jun 19, 2017
@onmyway133
Copy link
Contributor

@vitalii-tym Hi, thanks for your PR. This is good addition. What do you think about when user opens and closes Lightbox in a short time, so that it still hasn't finished downloading all the images?

@vitalii-tym
Copy link
Contributor Author

vitalii-tym commented Jun 19, 2017

If the image was not yet downloaded, the .image property is still going to be nil.
The apps using Lightbox should mind that .image is optional and safely unwrap it.
In my example at #98 tapping such a Share button would simply do nothing.
Ideally the Delete button (which is used for Share functionality in the example) is supposed to be disabled while image is still being loaded by Lightbox, but its state is always enabled.

… an image fetched by the Lightbox itself via given URL. This will enable apps to be able to access and use images, downloaded by Lightbox."

This reverts commit fa6c90b.
…eived image in case the preview has been dismissed before image is loaded
@vitalii-tym
Copy link
Contributor Author

@onmyway133 I took a second look and found that there was indeed a strong capture of self. Fixed now. Thanks for the heads up.

@vitalii-tym
Copy link
Contributor Author

sorry, too much commits in here now. Made another PR with only one concrete commit here #122

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