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

image_cache #996

Merged
merged 11 commits into from
Dec 13, 2023
Merged

image_cache #996

merged 11 commits into from
Dec 13, 2023

Conversation

fullstackctp
Copy link
Contributor

For the ticket #967

@@ -132,7 +134,9 @@ class ImageState extends WidgetState<EnsembleImage> {
}

return CachedNetworkImage(
Copy link
Contributor

Choose a reason for hiding this comment

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

if (widget.controller.cache)
use Image.network
else
use CachedNetworkImage

@fullstackctp
Copy link
Contributor Author

I have made the required changes however, the problem persists in the web side. The native side requires you to do hot restart for the image.network to work again.
https://stackoverflow.com/questions/49892074/flutter-image-network-not-updated

In the flutter image_provider they have added this as a todo

// TODO(ianh): Find some way to honor cache headers to the extent that when the
// last reference to an image is released, we proactively evict the image from
// our cache if the headers describe the image as having expired at that point.

For now the fix that is provided is to basically add timestamp with the date of last-modified from the cache-headers to provide a difference url if the images changes in the url

@vusters
Copy link
Contributor

vusters commented Nov 20, 2023

That's fine. Please use the timestamp as you have had. Go ahead and merge that part in.

Copy link
Collaborator

@kmahmood74 kmahmood74 left a comment

Choose a reason for hiding this comment

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

This doesn't seem right to me. @vusters has to review this before it is approved.

lib/widget/image.dart Outdated Show resolved Hide resolved
final http.Response response = await http
.get(Uri.parse("${url}timeStamp=${DateTime.now().toString()}"));
DateTime lastModifiedDateTime =
parseHttpDate("${response.headers['last-modified']}");
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if the last-modified header is not present. You will end up sending "null" as the parameter.

How will that work?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@fullstackctp please see this comment as well

lib/widget/image.dart Outdated Show resolved Hide resolved
@@ -537,6 +537,10 @@
"type": "string",
"description": "A unique identifier for this widget"
},
"cache": {
"type": "boolean",
"description": "To put the image in the cache"
Copy link
Contributor

Choose a reason for hiding this comment

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

makek sure you add "... default true."

@vusters vusters merged commit 360f4fc into main Dec 13, 2023
2 checks passed
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.

4 participants