Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add Delete Method & Account for Firebase Error #14

Closed

Conversation

kulture-rob-snider
Copy link

@kulture-rob-snider kulture-rob-snider commented Aug 2, 2020

  • when firebase storage throws an error, return a transparent image
  • add in a delete method to Firebase image which evicts from the cache and deletes from the db
  • when gettig remote image, do a write instead of a get to avoid size limits

closes #11

@@ -135,8 +135,13 @@ class FirebaseImageCacheManager {
}

Future<Uint8List> remoteFileBytes(
FirebaseImageObject object, int maxSizeBytes) {
return object.reference.getData(maxSizeBytes);
FirebaseImageObject object, int maxSizeBytes) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now, maxSizeBytes has no purpose and should be removed.

But really, rather than changing remoteFileBytes, consider adding an alternative method without the restriction.

Alternatively, consider keeping the object.reference.getData(maxSizeBytes) call when maxSizeBytes is specified, and doing the new logic when it isn't.

@@ -111,6 +111,17 @@ class FirebaseImage extends ImageProvider<FirebaseImage> {
.instantiateImageCodec(await _fetchImage());
}

Future<bool> delete() async {
FirebaseImageCacheManager cacheManager = new FirebaseImageCacheManager(
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need the new keyword anymore.

@awhitford
Copy link
Contributor

Returning a transparent image should be an option (or a wrapper class), not necessarily enabled by default.

In my case, I show a blurHash, then replace it with the image. If the image failed to load, I'd rather see my properly sized blurHash, not a transparent image.

@awhitford
Copy link
Contributor

I feel like I should clarify that returning a transparent image will not address #11 because when the file does not exist, then the placeholder should not be replaced.

Also, the proposed code catches any Exception -- there are many reasons why the image may not load. If the image does not exist, then there is nothing to load, but the load could also fail due to I/O and bad network connectivity.

@kulture-rob-snider
Copy link
Author

Thanks for the comments! I'll look to do some refactoring!

@jbryanh
Copy link

jbryanh commented Nov 17, 2020

I've thought about this...in my case I have a great deal of images, and I really can't constantly feed for metadata reads. So returning the transparent image is great. I just code in a stack so that my placeholder image is below the transparent or found image. Thanks for the great resource guys!

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

Successfully merging this pull request may close these issues.

Gracefully handle File Not Exists
3 participants